Skip to content

Commit

Permalink
fix: default transcript language should match browser default (#1160)
Browse files Browse the repository at this point in the history
Co-authored-by: Maham Akif <[email protected]>
  • Loading branch information
mahamakifdar19 and Maham Akif committed Aug 21, 2024
1 parent fa8a2dc commit 9e9ef9b
Show file tree
Hide file tree
Showing 9 changed files with 66 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ const imageURL = 'https://test-domain.com/test-image/id.png';
const hlsUrl = 'https://test-domain.com/test-prefix/id.m3u8';
const ytUrl = 'https://www.youtube.com/watch?v=oHg5SJYRHA0';

jest.mock('@edx/frontend-platform/i18n', () => ({
...jest.requireActual('@edx/frontend-platform/i18n'),
getLocale: () => 'en',
}));

describe('Course Preview Tests', () => {
it('Renders preview image and not the video when video URL is not given.', () => {
const { container, getByAltText } = renderWithRouter(<CoursePreview previewImage={imageURL} />);
Expand Down
1 change: 1 addition & 0 deletions src/components/microlearning/styles/VideoDetailPage.scss
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

.video-player-container-with-transcript {
display: flex;
padding-bottom: 55px;
}

.video-js-wrapper {
Expand Down
3 changes: 3 additions & 0 deletions src/components/video/VideoJS.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import PropTypes from 'prop-types';
import 'videojs-youtube';
import videojs from 'video.js';
import 'video.js/dist/video-js.css';
import { getLocale } from '@edx/frontend-platform/i18n';
import { PLAYBACK_RATES } from './data/constants';
import { usePlayerOptions, useTranscripts } from './data';

Expand All @@ -14,10 +15,12 @@ require('videojs-vjstranscribe');
const VideoJS = ({ options, onReady, customOptions }) => {
const videoRef = useRef(null);
const playerRef = useRef(null);
const siteLanguage = getLocale();

const transcripts = useTranscripts({
player: playerRef.current,
customOptions,
siteLanguage,
});

const playerOptions = usePlayerOptions({
Expand Down
22 changes: 13 additions & 9 deletions src/components/video/data/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@ import { useEffect, useState } from 'react';
import { logError } from '@edx/frontend-platform/logging';

import { fetchAndAddTranscripts } from './service';
import { sortTextTracks } from './utils';

export function useTranscripts({ player, customOptions }) {
export function useTranscripts({ player, customOptions, siteLanguage }) {
const shouldUseTranscripts = !!(customOptions?.showTranscripts && customOptions?.transcriptUrls);
const [isLoading, setIsLoading] = useState(shouldUseTranscripts);
const [textTracks, setTextTracks] = useState([]);
const [textTracks, setTextTracks] = useState({});
const [transcriptUrl, setTranscriptUrl] = useState(null);

useEffect(() => {
Expand All @@ -15,12 +16,15 @@ export function useTranscripts({ player, customOptions }) {
if (shouldUseTranscripts) {
try {
const result = await fetchAndAddTranscripts(customOptions.transcriptUrls, player);
setTextTracks(result);
// We are only catering to English transcripts for now as we don't have the option to change
// the transcript language yet.
if (result.en) {
setTranscriptUrl(result.en);
}

// Sort the text tracks to prioritize the site language at the top of the list.
// Currently, video.js selects the top language from the list of transcripts.
const sortedResult = sortTextTracks(result, siteLanguage);
setTextTracks(sortedResult);

// Default to site language, fallback to English
const preferredTranscript = sortedResult[siteLanguage] || sortedResult.en;
setTranscriptUrl(preferredTranscript);
} catch (error) {
logError(`Error fetching transcripts for player: ${error}`);
} finally {
Expand All @@ -29,7 +33,7 @@ export function useTranscripts({ player, customOptions }) {
}
};
fetchFn();
}, [customOptions?.transcriptUrls, player, shouldUseTranscripts]);
}, [customOptions?.transcriptUrls, player, shouldUseTranscripts, siteLanguage]);

return {
textTracks,
Expand Down
4 changes: 2 additions & 2 deletions src/components/video/data/tests/hooks.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe('useTranscripts', () => {

expect(logError).toHaveBeenCalledWith(`Error fetching transcripts for player: Error: ${errorMessage}`);
expect(result.current.isLoading).toBe(false);
expect(result.current.textTracks).toEqual([]);
expect(result.current.textTracks).toEqual({});
expect(result.current.transcriptUrl).toBeNull();
});

Expand All @@ -64,7 +64,7 @@ describe('useTranscripts', () => {
customOptions: customOptionsWithoutTranscripts,
}));

expect(result.current.textTracks).toEqual([]);
expect(result.current.textTracks).toEqual({});
expect(result.current.transcriptUrl).toBeNull();
});
});
20 changes: 19 additions & 1 deletion src/components/video/data/tests/utils.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { convertToWebVtt, createWebVttFile } from '../utils';
import { convertToWebVtt, createWebVttFile, sortTextTracks } from '../utils';

describe('Video utils tests', () => {
it('should convert transcript data to WebVTT format correctly', () => {
Expand Down Expand Up @@ -40,4 +40,22 @@ describe('Video utils tests', () => {
expect(blob.type).toBe('text/vtt');
expect(blob.size).toBe(mockWebVttContent.length);
});
it('should sort text tracks with site language first and others alphabetically', () => {
const mockTracks = {
en: 'https://test-domain.com/transcript-en.txt',
ar: 'https://test-domain.com/transcript-ar.txt',
fr: 'https://test-domain.com/transcript-fr.txt',
};

const siteLanguage = 'fr';

const expectedSortedTracks = {
fr: 'https://test-domain.com/transcript-fr.txt',
ar: 'https://test-domain.com/transcript-ar.txt',
en: 'https://test-domain.com/transcript-en.txt',
};

const result = sortTextTracks(mockTracks, siteLanguage);
expect(result).toEqual(expectedSortedTracks);
});
});
13 changes: 13 additions & 0 deletions src/components/video/data/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,16 @@ export const createWebVttFile = (webVttContent) => {
const blob = new Blob([webVttContent], { type: 'text/vtt' });
return URL.createObjectURL(blob);
};

export const sortTextTracks = (tracks, siteLanguage) => {
const sortedKeys = Object.keys(tracks).sort((a, b) => {
if (a === siteLanguage) { return -1; }
if (b === siteLanguage) { return 1; }
return a.localeCompare(b);
});

return sortedKeys.reduce((acc, key) => {
acc[key] = tracks[key];
return acc;
}, {});
};
5 changes: 5 additions & 0 deletions src/components/video/tests/VideoJS.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ jest.mock('../data', () => ({
usePlayerOptions: jest.fn(),
}));

jest.mock('@edx/frontend-platform/i18n', () => ({
...jest.requireActual('@edx/frontend-platform/i18n'),
getLocale: () => 'en',
}));

const hlsUrl = 'https://test-domain.com/test-prefix/id.m3u8';
const ytUrl = 'https://www.youtube.com/watch?v=oHg5SJYRHA0';

Expand Down
5 changes: 5 additions & 0 deletions src/components/video/tests/VideoPlayer.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ const hlsUrl = 'https://test-domain.com/test-prefix/id.m3u8';
const ytUrl = 'https://www.youtube.com/watch?v=oHg5SJYRHA0';
const mp3Url = 'https://example.com/audio.mp3';

jest.mock('@edx/frontend-platform/i18n', () => ({
...jest.requireActual('@edx/frontend-platform/i18n'),
getLocale: () => 'en',
}));

describe('Video Player component', () => {
it('Renders Video Player components correctly for HLS videos.', async () => {
const { container } = renderWithRouter(<VideoPlayer videoURL={hlsUrl} />);
Expand Down

0 comments on commit 9e9ef9b

Please sign in to comment.