Skip to content

Commit

Permalink
Merge pull request #685 from samvera-labs/bug-emptymanifest
Browse files Browse the repository at this point in the history
Regression bug: display error for empty Manifest w/o crashing
  • Loading branch information
Dananji authored Oct 25, 2024
2 parents c866d88 + 0e10257 commit 5d3df62
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 24 deletions.
18 changes: 18 additions & 0 deletions src/components/MediaPlayer/MediaPlayer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ 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';
import emptyManifest from '@TestData/empty-manifest';
import * as hooks from '@Services/ramp-hooks';

describe('MediaPlayer component', () => {
Expand Down Expand Up @@ -465,6 +466,23 @@ describe('MediaPlayer component', () => {
}));
return switchPlayerMock;
};

test('for empty Manifest', () => {
const PlayerWithManifest = withManifestAndPlayerProvider(MediaPlayer, {
initialManifestState: { ...manifestState(emptyManifest) },
initialPlayerState: {},
});
render(
<ErrorBoundary>
<PlayerWithManifest />
</ErrorBoundary>
);
expect(screen.queryByTestId('inaccessible-message-display')).toBeInTheDocument();
expect(screen.getByTestId('inaccessible-message-content').textContent)
.toEqual('No media resource(s). Please check your Manifest.');
expect(screen.queryByTestId('inaccessible-message-buttons')).not.toBeInTheDocument();
});

test('with HTML from placeholderCanvas for an empty canvas', () => {
// Stub loading HTMLMediaElement for jsdom
window.HTMLMediaElement.prototype.load = () => { };
Expand Down
41 changes: 22 additions & 19 deletions src/components/MediaPlayer/VideoJS/VideoJSPlayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -777,25 +777,28 @@ function VideoJSPlayer({
<p className="ramp--media-player_inaccessible-message-content" data-testid="inaccessible-message-content"
dangerouslySetInnerHTML={{ __html: placeholderText }}>
</p>
<div className="ramp--media-player_inaccessible-message-buttons">
{canvasIndex >= 1 &&
<button aria-label="Go back to previous item"
onClick={() => handlePrevNextClick(canvasIndex - 1)}
onKeyDown={(e) => handlePrevNextKeydown(e, canvasIndex - 1, 'previousBtn')}
data-testid="inaccessible-previous-button">
<SectionButtonIcon flip={true} /> Previous
</button>
}
{canvasIndex != lastCanvasIndex &&
<button aria-label="Go to next item"
onClick={() => handlePrevNextClick(canvasIndex + 1)}
onKeyDown={(e) => handlePrevNextKeydown(e, canvasIndex + 1, 'nextBtn')}
data-testid="inaccessible-next-button">
Next <SectionButtonIcon />
</button>
}
</div>
{canvasIndex != lastCanvasIndex &&
{lastCanvasIndex > 0 &&
<div className="ramp--media-player_inaccessible-message-buttons"
data-testid="inaccessible-message-buttons">
{canvasIndex >= 1 &&
<button aria-label="Go back to previous item"
onClick={() => handlePrevNextClick(canvasIndex - 1)}
onKeyDown={(e) => handlePrevNextKeydown(e, canvasIndex - 1, 'previousBtn')}
data-testid="inaccessible-previous-button">
<SectionButtonIcon flip={true} /> Previous
</button>
}
{canvasIndex != lastCanvasIndex &&
<button aria-label="Go to next item"
onClick={() => handlePrevNextClick(canvasIndex + 1)}
onKeyDown={(e) => handlePrevNextKeydown(e, canvasIndex + 1, 'nextBtn')}
data-testid="inaccessible-next-button">
Next <SectionButtonIcon />
</button>
}
</div>
}
{canvasIndex != lastCanvasIndex && lastCanvasIndex > 0 &&
<p data-testid="inaccessible-message-timer"
className={cx(
'ramp--media-player_inaccessible-message-timer',
Expand Down
3 changes: 1 addition & 2 deletions src/services/iiif-parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,9 @@ export function getMediaInfo({ manifest, canvasIndex, startTime, srcIndex = 0, i
let canvas = null;
let sources, tracks = [];
let info = {
canvas: null,
sources: [],
tracks: [],
canvasTargets: []
canvasTargets: [],
};

// return empty object when canvasIndex is undefined
Expand Down
2 changes: 0 additions & 2 deletions src/services/iiif-parser.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ describe('iiif-parser', () => {
})
).toEqual({
error: 'Error fetching content',
canvas: null,
sources: [],
tracks: [],
canvasTargets: []
Expand All @@ -187,7 +186,6 @@ describe('iiif-parser', () => {
sources: [],
tracks: [],
poster: 'No media resource(s). Please check your Manifest.',
canvas: null,
canvasTargets: [],
});
});
Expand Down
3 changes: 2 additions & 1 deletion src/services/ramp-hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,8 @@ export const useSetupPlayer = ({
});

const currentCanvas = allCanvases.find((c) => c.canvasIndex === canvasId);
if (!currentCanvas.isEmpty) {
// When Manifest is empty currentCanvas is null
if (currentCanvas && !currentCanvas.isEmpty) {
// Manifest is taken from manifest state, and is a basic object at this point
// lacking the getLabel() function so we manually retrieve the first label.
let manifestLabel = manifest.label ? Object.values(manifest.label)[0][0] : '';
Expand Down

0 comments on commit 5d3df62

Please sign in to comment.