From e0bee12fd294e54432e918b84b63bd693673b92a Mon Sep 17 00:00:00 2001 From: Dananji Withana Date: Tue, 28 May 2024 10:26:54 -0400 Subject: [PATCH] Enable/disable captions icon as needed when switching between canvases (#505) * Enable/disable captions icon as needed when switching between canvases * Code review: fix unnecessary condition when calculating duration --- src/components/MediaPlayer/MediaPlayer.js | 1 + .../MediaPlayer/MediaPlayer.test.js | 100 ++++++++++++-- .../MediaPlayer/VideoJS/VideoJSPlayer.js | 24 +++- .../StructuredNavigation.test.js | 2 +- src/components/Transcript/Transcript.test.js | 2 +- src/services/iiif-parser.test.js | 25 ++-- src/test_data/playlist.js | 122 ++++++++++++++++-- 7 files changed, 231 insertions(+), 45 deletions(-) diff --git a/src/components/MediaPlayer/MediaPlayer.js b/src/components/MediaPlayer/MediaPlayer.js index 2f0ab5db..a344d4da 100644 --- a/src/components/MediaPlayer/MediaPlayer.js +++ b/src/components/MediaPlayer/MediaPlayer.js @@ -431,6 +431,7 @@ const MediaPlayer = ({ > { - let originalError; + let originalError, originalLogger; beforeEach(() => { originalError = console.error; console.error = jest.fn(); + originalLogger = console.log; + console.log = jest.fn(); }); afterAll(() => { console.error = originalError; + console.log = originalLogger; }); describe('with a regular audio Manifest', () => { @@ -92,7 +96,7 @@ describe('MediaPlayer component', () => { describe('with props', () => { describe('enableFileDownload', () => { - test('with default value: `false`', () => { + test('with default value: `false` does not render file download icon', () => { const PlayerWithManifest = withManifestAndPlayerProvider(MediaPlayer, { initialManifestState: { ...manifestState, manifest: videoManifest, canvasIndex: 0 }, initialPlayerState: {}, @@ -106,7 +110,7 @@ describe('MediaPlayer component', () => { expect(screen.queryByTestId('videojs-file-download')).not.toBeInTheDocument(); }); - test('set to `true`', async () => { + test('set to `true` renders file download icon', async () => { const PlayerWithManifest = withManifestAndPlayerProvider(MediaPlayer, { initialManifestState: { ...manifestState, manifest: videoManifest, canvasIndex: 0 }, initialPlayerState: {}, @@ -122,7 +126,7 @@ describe('MediaPlayer component', () => { }); describe('enablePIP', () => { - test('with default value: `false`', async () => { + test('with default value: `false` does not render pip icon', async () => { const PlayerWithManifest = withManifestAndPlayerProvider(MediaPlayer, { initialManifestState: { ...manifestState, manifest: videoManifest, canvasIndex: 0 }, initialPlayerState: {}, @@ -134,7 +138,7 @@ describe('MediaPlayer component', () => { )); expect(screen.queryByTitle('Picture-in-Picture')).not.toBeInTheDocument(); }); - test('set to `true`', async () => { + test('set to `true` renders pip icon', async () => { const PlayerWithManifest = withManifestAndPlayerProvider(MediaPlayer, { initialManifestState: { ...manifestState, manifest: videoManifest, canvasIndex: 0 }, initialPlayerState: {}, @@ -150,7 +154,7 @@ describe('MediaPlayer component', () => { }); describe('enablePlaybackRate', () => { - test('with default value: `false`', async () => { + test('with default value: `false` does not render playback rate icon', async () => { const PlayerWithManifest = withManifestAndPlayerProvider(MediaPlayer, { initialManifestState: { ...manifestState, manifest: videoManifest, canvasIndex: 0 }, initialPlayerState: {}, @@ -162,7 +166,7 @@ describe('MediaPlayer component', () => { )); expect(screen.queryByTitle('Playback Rate')).not.toBeInTheDocument(); }); - test('set to `true`', async () => { + test('set to `true` renders playback rate icon', async () => { const PlayerWithManifest = withManifestAndPlayerProvider(MediaPlayer, { initialManifestState: { ...manifestState, manifest: videoManifest, canvasIndex: 0 }, initialPlayerState: {}, @@ -178,8 +182,8 @@ describe('MediaPlayer component', () => { }); }); - describe('previous/next section buttons', () => { - test('does not render with a single Canvas Manifest', () => { + describe('does not render previous/next section buttons', () => { + test('with a single Canvas Manifest', () => { const PlayerWithManifest = withManifestAndPlayerProvider(MediaPlayer, { initialManifestState: { ...manifestState, manifest: audioManifest, canvasIndex: 0 }, initialPlayerState: {}, @@ -193,7 +197,7 @@ describe('MediaPlayer component', () => { expect(screen.queryByTestId('videojs-previous-button')).not.toBeInTheDocument(); }); - test('does not render with a single Canvas with multiple sources', () => { + test('with a single Canvas with multiple sources', () => { const PlayerWithManifest = withManifestAndPlayerProvider(MediaPlayer, { initialManifestState: { ...manifestState, manifest: audioManifest, canvasIndex: 0 }, initialPlayerState: {}, @@ -207,7 +211,10 @@ describe('MediaPlayer component', () => { expect(screen.queryByTestId('videojs-previous-button')).not.toBeInTheDocument(); }); - test('renders with a multi-Canvas regualr Manifest', async () => { + }); + + describe('renders previous/next section buttons', () => { + test('with a multi-Canvas regualr Manifest', async () => { const PlayerWithManifest = withManifestAndPlayerProvider(MediaPlayer, { initialManifestState: { ...manifestState, manifest: videoManifest, canvasIndex: 0 }, initialPlayerState: {}, @@ -221,7 +228,7 @@ describe('MediaPlayer component', () => { expect(screen.queryByTestId('videojs-previous-button')).toBeInTheDocument(); }); - test('renders with a multi-Canvas playlist Manifest', async () => { + test('with a multi-Canvas playlist Manifest', async () => { const PlayerWithManifest = withManifestAndPlayerProvider(MediaPlayer, { initialManifestState: { manifest: playlistManifest, @@ -240,6 +247,75 @@ describe('MediaPlayer component', () => { }); }); + describe('renders captions button', () => { + test('with a video canvas with supplementing annotations', async () => { + const PlayerWithManifest = withManifestAndPlayerProvider(MediaPlayer, { + initialManifestState: { ...manifestState, manifest: videoManifest, canvasIndex: 0 }, + initialPlayerState: {}, + }); + render( + + + + ); + expect( + screen.queryAllByTestId('videojs-video-element').length + ).toBeGreaterThan(0); + await waitFor(() => { + expect(screen.queryByTitle('Captions')).toBeInTheDocument(); + }); + }); + + test('with a video canvas with supplementing annotations in playlist context', async () => { + const PlayerWithManifest = withManifestAndPlayerProvider(MediaPlayer, { + initialManifestState: { ...manifestState, manifest: playlistManifest, canvasIndex: 3 }, + initialPlayerState: {}, + }); + render( + + + + ); + expect( + screen.queryAllByTestId('videojs-video-element').length + ).toBeGreaterThan(0); + await waitFor(() => { + expect(screen.queryByTitle('Captions')).toBeInTheDocument(); + }); + }); + }); + + describe('does not render captions button', () => { + test('with an audio canvas', () => { + const PlayerWithManifest = withManifestAndPlayerProvider(MediaPlayer, { + initialManifestState: { ...manifestState, manifest: audioManifest, canvasIndex: 0 }, + initialPlayerState: {}, + }); + render( + + + + ); + expect(screen.queryByTitle('Captions')).not.toBeInTheDocument(); + }); + + test('with a video canvas w/o supplementing annotations', () => { + const PlayerWithManifest = withManifestAndPlayerProvider(MediaPlayer, { + initialManifestState: { ...manifestState, manifest: noCaptionManifest, canvasIndex: 0 }, + initialPlayerState: {}, + }); + render( + + + + ); + expect( + screen.queryAllByTestId('videojs-video-element').length + ).toBeGreaterThan(0); + expect(screen.queryByTitle('Captions')).not.toBeInTheDocument(); + }); + }); + describe('track scrubber button', () => { test('does not render with a regular Manifest without structure timespans', () => { const PlayerWithManifest = withManifestAndPlayerProvider(MediaPlayer, { diff --git a/src/components/MediaPlayer/VideoJS/VideoJSPlayer.js b/src/components/MediaPlayer/VideoJS/VideoJSPlayer.js index 9fad8995..919eddf1 100644 --- a/src/components/MediaPlayer/VideoJS/VideoJSPlayer.js +++ b/src/components/MediaPlayer/VideoJS/VideoJSPlayer.js @@ -30,6 +30,7 @@ import VideoJSTrackScrubber from './components/js/VideoJSTrackScrubber'; function VideoJSPlayer({ isVideo, + hasMultipleCanvases, isPlaylist, trackScrubberRef, scrubberTooltipRef, @@ -248,9 +249,11 @@ function VideoJSPlayer({ while (i--) { player.removeRemoteTextTrack(oldTracks[i]); } - tracks.forEach(function (track) { - player.addRemoteTextTrack(track, false); - }); + if (tracks?.length > 0 && isVideo) { + tracks.forEach(function (track) { + player.addRemoteTextTrack(track, false); + }); + } /* Update player control bar for; @@ -262,6 +265,10 @@ function VideoJSPlayer({ */ if (player.getChild('controlBar') != null && !canvasIsEmpty) { const controlBar = player.getChild('controlBar'); + // Index of the duration display in the player's control bar + const durationIndex = controlBar.children() + .findIndex((c) => c.name_ == 'DurationDisplay') || + (hasMultipleCanvases ? 6 : 4); /* Track-scrubber button: remove if the Manifest is not a playlist manifest or the current Canvas doesn't have structure items. Or add back in if it's @@ -271,8 +278,6 @@ function VideoJSPlayer({ controlBar.removeChild('videoJSTrackScrubber'); } else if (!controlBar.getChild('videoJSTrackScrubber')) { // Add track-scrubber button after duration display if it is not available - const durationIndex = controlBar.children() - .findIndex((c) => c.name_ == 'DurationDisplay') || 6; controlBar.addChild( 'videoJSTrackScrubber', { trackScrubberRef, timeToolRef: scrubberTooltipRef }, @@ -280,6 +285,14 @@ function VideoJSPlayer({ ); } + if (tracks?.length > 0 && isVideo && !controlBar.getChild('subsCapsButton')) { + let subsCapBtn = controlBar.addChild( + 'subsCapsButton', {}, durationIndex + 1 + ); + // Add CSS to mark captions-on + subsCapBtn.children_[0].addClass('captions-on'); + } + /* Change player's appearance when switching between audio and video canvases. For audio: player height is reduced and big play button is removed @@ -836,6 +849,7 @@ function VideoJSPlayer({ VideoJSPlayer.propTypes = { isVideo: PropTypes.bool, + hasMultipleCanvases: PropTypes.bool, isPlaylist: PropTypes.bool, trackScrubberRef: PropTypes.object, scrubberTooltipRef: PropTypes.object, diff --git a/src/components/StructuredNavigation/StructuredNavigation.test.js b/src/components/StructuredNavigation/StructuredNavigation.test.js index 64bd0101..10f13fe2 100644 --- a/src/components/StructuredNavigation/StructuredNavigation.test.js +++ b/src/components/StructuredNavigation/StructuredNavigation.test.js @@ -324,7 +324,7 @@ describe('StructuredNavigation component', () => { }); test('renders all playlist items', () => { - expect(screen.queryAllByTestId('list-item')).toHaveLength(3); + expect(screen.queryAllByTestId('list-item')).toHaveLength(4); expect(screen.queryAllByTestId('list-item')[1]).toHaveTextContent('Playlist Item 1'); }); diff --git a/src/components/Transcript/Transcript.test.js b/src/components/Transcript/Transcript.test.js index a0c59316..d8a02972 100644 --- a/src/components/Transcript/Transcript.test.js +++ b/src/components/Transcript/Transcript.test.js @@ -498,7 +498,7 @@ describe('Transcript component', () => { await act(() => Promise.resolve()); await waitFor(() => { - expect(parseTranscriptMock).toHaveBeenCalledTimes(1); + expect(parseTranscriptMock).toHaveBeenCalled(); expect(screen.queryByTestId('transcript_content_-2')).toBeInTheDocument(); expect(screen.queryByTestId('no-transcript')).toBeInTheDocument(); expect(screen.getByTestId('no-transcript')).toHaveTextContent( diff --git a/src/services/iiif-parser.test.js b/src/services/iiif-parser.test.js index a7613202..00ca03e9 100644 --- a/src/services/iiif-parser.test.js +++ b/src/services/iiif-parser.test.js @@ -36,7 +36,7 @@ describe('iiif-parser', () => { describe('with a playlist manifest', () => { it('returns summary for all canvases', () => { const canvases = iiifParser.canvasesInManifest(playlistManifest); - expect(canvases).toHaveLength(3); + expect(canvases).toHaveLength(4); // Empty Canvas => for inaccessible items expect(canvases[0]).toHaveProperty('summary'); expect(canvases[0].summary).toEqual('Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua'); @@ -50,7 +50,7 @@ describe('iiif-parser', () => { test('returns positional homepage for all canvases', () => { const canvases = iiifParser.canvasesInManifest(playlistManifest); - expect(canvases).toHaveLength(3); + expect(canvases).toHaveLength(4); // Empty Canvas => for inaccessible items expect(canvases[0]).toHaveProperty('homepage'); expect(canvases[0].homepage).toEqual('https://example.com/playlists/1?position=1'); @@ -502,7 +502,7 @@ describe('iiif-parser', () => { it('canvas with metadata returns a list of key, value pairs', () => { const { manifestMetadata, canvasMetadata } = iiifParser.getMetadata(playlistManifest, true); expect(manifestMetadata.length).toBeGreaterThan(0); - expect(canvasMetadata.length).toEqual(3); + expect(canvasMetadata.length).toEqual(4); expect(canvasMetadata[1].metadata[0]).toEqual({ label: "Title", value: "Second Playlist Item" }); expect(canvasMetadata[1]).toHaveProperty('rights'); expect(canvasMetadata[1].rights[0]).toEqual( @@ -510,18 +510,18 @@ describe('iiif-parser', () => { label: "Attribution", value: "Creative commons CC BY-SA 3.0" }); - // console.log is called twice for the 2 canvases without metadata - expect(console.log).toBeCalledTimes(2); + // console.log is called twice for the 3 canvases without metadata + expect(console.log).toBeCalledTimes(3); }); it('canvas without metadata returns []', () => { const { manifestMetadata, canvasMetadata } = iiifParser.getMetadata(playlistManifest, true); expect(manifestMetadata.length).toBeGreaterThan(0); - expect(canvasMetadata.length).toEqual(3); + expect(canvasMetadata.length).toEqual(4); expect(canvasMetadata[0].metadata).toEqual([]); - // console.log is called twice for the 2 canvases without metadata - expect(console.log).toBeCalledTimes(2); + // console.log is called twice for the 3 canvases without metadata + expect(console.log).toBeCalledTimes(3); }); }); @@ -682,11 +682,10 @@ describe('iiif-parser', () => { expect(markRoot).toBeFalsy(); }); - it('returns canvas summary with structure for playist manifests', () => { - const { structures, timespans, markRoot } = iiifParser.getStructureRanges(playlistManifest, true); - expect(structures).toHaveLength(3); - expect(timespans).toHaveLength(3); - expect(markRoot).toBeFalsy(); + it('returns canvas summary with structure for playlist manifests', () => { + const { structures, timespans } = iiifParser.getStructureRanges(playlistManifest, true); + expect(structures).toHaveLength(4); + expect(timespans).toHaveLength(4); const firstStructCanvas = structures[1]; expect(firstStructCanvas.label).toEqual('Playlist Item 1'); diff --git a/src/test_data/playlist.js b/src/test_data/playlist.js index 1e43b2c2..0e445578 100644 --- a/src/test_data/playlist.js +++ b/src/test_data/playlist.js @@ -267,17 +267,6 @@ export default { width: 480, duration: 572.0 }, - { - id: 'http://example.com/lunchroom_manners/low/lunchroom_manners_256kb.mp4#t=0,572.0', - type: 'Video', - format: 'video/mp4', - label: { - en: ['Low'], - }, - height: 360, - width: 480, - duration: 572.0 - }, ], }, ], @@ -317,6 +306,102 @@ export default { } ], }, + { + id: 'http://example.com/playlists/1/canvas/4', + type: 'Canvas', + duration: 662.037, + label: { + en: ["Volleyball for boys"] + }, + summary: { + none: ["Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod \ + tempor incididunt ut labore et dolore magna aliqua"] + }, + homepage: [ + { + id: 'https://example.com/playlists/1?position=4', + type: 'Text', + label: { none: ['View in Repository'] }, + format: 'text/html' + } + ], + placeholderCanvas: { + id: 'http://example.com/playlists/1/canvas/4/placeholder', + type: "Canvas", + width: 640, + height: 360, + items: [ + { + id: 'http://example.com/playlists/1/canvas/4/placeholder/1', + type: "AnnotationPage", + items: [ + { + id: 'http://example.com/playlists/1/canvas/4/placeholder/1-image', + type: "Annotation", + motivation: "painting", + body: { + id: 'http://example.com/volleyball-for-boys-poster.jpg', + type: "Image", + format: "image/jpeg", + width: 640, + height: 360 + }, + target: 'http://example.com/playlists/1/canvas/4/placeholder' + } + ] + } + ] + }, + items: [ + { + id: 'http://example.com/playlists/1/canvas/4/page', + type: 'AnnotationPage', + duration: 662.037, + items: [ + { + id: 'http://example.com/playlists/1/canvas/4/page/annotation', + type: 'Annotation', + motivation: 'painting', + body: { + id: 'http://example.com/volleyball/high/volleyball-for-boys.m3u8#t=35.0,40.0', + type: 'Video', + format: 'application/x-mpegURL', + label: { + en: ['High'], + }, + height: 360, + width: 480, + duration: 662.037, + }, + target: 'http://example.com/playlists/1/canvas/4', + }, + ], + }, + ], + annotations: [ + { + id: 'http://example.com/playlists/1/canvas/4/page/2', + type: 'AnnotationPage', + items: [ + { + id: 'http://example.com/playlists/1/canvas/4/annotation/1', + type: 'Annotation', + motivation: 'supplementing', + body: { + id: 'http://example.com/playlists/1/volleyball-for-boys.vtt', + type: 'Text', + format: 'text/vtt', + label: { + en: ['Captions in WebVTT format'], + }, + language: 'en', + }, + target: 'http://example.com/playlists/1/canvas/4' + } + ] + } + ] + } ], structures: [ { @@ -344,7 +429,7 @@ export default { items: [ { type: "Canvas", - id: 'http://example.com/playlists/1/canvas/2#t=0,' + id: 'http://example.com/playlists/1/canvas/2#t=0,32.0' } ] }, @@ -355,7 +440,18 @@ export default { items: [ { type: "Canvas", - id: 'http://example.com/playlists/1/canvas/3#t=0,' + id: 'http://example.com/playlists/1/canvas/3#t=0,572.0' + } + ] + }, + { + id: 'http://example.com/playlists/1/range/4', + type: 'Range', + label: { en: ['Playlist Item 3'] }, + items: [ + { + type: "Canvas", + id: 'http://example.com/playlists/1/canvas/4#t=35.0,40.0' } ] },