Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable/disable captions icon as needed when switching between canvases #505

Merged
merged 2 commits into from
May 28, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/components/MediaPlayer/MediaPlayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,7 @@ const MediaPlayer = ({
>
<VideoJSPlayer
isVideo={isVideo}
hasMultipleCanvases={isMultiCanvased}
isPlaylist={playlist.isPlaylist}
trackScrubberRef={trackScrubberRef}
scrubberTooltipRef={timeToolRef}
Expand Down
100 changes: 88 additions & 12 deletions src/components/MediaPlayer/MediaPlayer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,25 @@ import MediaPlayer from './MediaPlayer';
import { ErrorBoundary } from 'react-error-boundary';
import audioManifest from '@TestData/transcript-canvas';
import videoManifest from '@TestData/lunchroom-manners';
import noCaptionManifest from '@TestData/multiple-canvas-auto-advance';
import emptyCanvasManifest from '@TestData/transcript-annotation';
import playlistManifest from '@TestData/playlist';

let manifestState = {
playlist: { isPlaylist: false, markers: [], isEditing: false }
};
describe('MediaPlayer component', () => {
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', () => {
Expand Down Expand Up @@ -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: {},
Expand All @@ -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: {},
Expand All @@ -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: {},
Expand All @@ -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: {},
Expand All @@ -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: {},
Expand All @@ -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: {},
Expand All @@ -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: {},
Expand All @@ -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: {},
Expand All @@ -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: {},
Expand All @@ -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,
Expand All @@ -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(
<ErrorBoundary>
<PlayerWithManifest />
</ErrorBoundary>
);
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(
<ErrorBoundary>
<PlayerWithManifest />
</ErrorBoundary>
);
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(
<ErrorBoundary>
<PlayerWithManifest />
</ErrorBoundary>
);
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(
<ErrorBoundary>
<PlayerWithManifest />
</ErrorBoundary>
);
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, {
Expand Down
24 changes: 19 additions & 5 deletions src/components/MediaPlayer/VideoJS/VideoJSPlayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import VideoJSTrackScrubber from './components/js/VideoJSTrackScrubber';

function VideoJSPlayer({
isVideo,
hasMultipleCanvases,
isPlaylist,
trackScrubberRef,
scrubberTooltipRef,
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -271,15 +278,21 @@ 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 },
durationIndex + 1
);
}

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
Expand Down Expand Up @@ -836,6 +849,7 @@ function VideoJSPlayer({

VideoJSPlayer.propTypes = {
isVideo: PropTypes.bool,
hasMultipleCanvases: PropTypes.bool,
isPlaylist: PropTypes.bool,
trackScrubberRef: PropTypes.object,
scrubberTooltipRef: PropTypes.object,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});

Expand Down
2 changes: 1 addition & 1 deletion src/components/Transcript/Transcript.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
8 changes: 5 additions & 3 deletions src/services/iiif-parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -706,9 +706,11 @@ export function getStructureRanges(manifest, isPlaylist = false) {
} else {
hasRoot = true;
// Total duration for all resources in the Manifest
manifestDuration = canvasesInfo.reduce(
(duration, canvas) => duration + canvas.range.end, 0
);
if (!isPlaylist) {
Dananji marked this conversation as resolved.
Show resolved Hide resolved
manifestDuration = canvasesInfo.reduce(
(duration, canvas) => duration + canvas.range.end, 0
);
}
structures.push(parseItem(rootNode, rootNode, cIndex));
}
}
Expand Down
25 changes: 12 additions & 13 deletions src/services/iiif-parser.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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');
Expand Down Expand Up @@ -502,26 +502,26 @@ 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(
{
label: "Attribution",
value: "<span>Creative commons <a href=\"https://creativecommons.org/licenses/by-sa/3.0\">CC BY-SA 3.0</a></span>"
});
// 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);
});
});

Expand Down Expand Up @@ -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');
Expand Down
Loading