Skip to content

Commit

Permalink
Fixes from code review
Browse files Browse the repository at this point in the history
  • Loading branch information
Dananji committed Jun 24, 2024
1 parent 18d8b22 commit 99678a7
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 15 deletions.
16 changes: 6 additions & 10 deletions src/components/MediaPlayer/MediaPlayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -286,18 +286,15 @@ const MediaPlayer = ({
* Switch player when navigating across canvases
* @param {Number} index canvas index to be loaded into the player
* @param {Boolean} fromStart flag to indicate set player start time to zero or not
* @param {Boolean} checkAutoAdvance flag to indicate to check value of autoadvance in state
* @param {String} focusElement element to be focused within the player when using
* next or previous buttons with keyboard
*/
const switchPlayer = (index, fromStart, focusElement = '', checkAutoAdvance = false) => {
const switchPlayer = (index, fromStart, focusElement = '') => {
if (canvasIndexRef.current != index && index <= lastCanvasIndexRef.current) {
if (!checkAutoAdvance || (checkAutoAdvance && autoAdvanceRef.current)) {
manifestDispatch({
canvasIndex: index,
type: 'switchCanvas',
});
}
manifestDispatch({
canvasIndex: index,
type: 'switchCanvas',
});
initCanvas(index, fromStart);
playerDispatch({ element: focusElement, type: 'setPlayerFocusElement' });
}
Expand Down Expand Up @@ -352,8 +349,7 @@ const MediaPlayer = ({
srcIndex,
targets,
currentTime: currentTime || 0,
nextItemClicked,
switchPlayer
nextItemClicked
},
videoJSCurrentTime: { srcIndex, targets, currentTime: currentTime || 0 },
},
Expand Down
4 changes: 2 additions & 2 deletions src/components/MediaPlayer/MediaPlayer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,7 @@ describe('MediaPlayer component', () => {
expect(screen.queryByTestId('inaccessible-message-display')).toBeInTheDocument();
expect(screen.getByText('You do not have permission to playback this item.')).toBeInTheDocument();
expect(screen.queryByTestId('inaccessible-message-timer')).toBeInTheDocument();
expect(screen.getByTestId('inaccessible-message-timer')).toHaveClass('disabled');
expect(screen.getByTestId('inaccessible-message-timer')).toHaveClass('hidden');
expect(screen.queryByTestId('inaccessible-next-button')).toBeInTheDocument();
});

Expand All @@ -525,7 +525,7 @@ describe('MediaPlayer component', () => {
expect(screen.queryByTestId('inaccessible-message-display')).toBeInTheDocument();
expect(screen.getByText('You do not have permission to playback this item.')).toBeInTheDocument();
expect(screen.queryByTestId('inaccessible-message-timer')).toBeInTheDocument();
expect(screen.getByTestId('inaccessible-message-timer')).toHaveClass('disabled');
expect(screen.getByTestId('inaccessible-message-timer')).toHaveClass('hidden');
expect(screen.queryByTestId('inaccessible-next-button')).toBeInTheDocument();
fireEvent.click(screen.getByTestId('inaccessible-next-button'));
// Loads video player for the next item in the list
Expand Down
2 changes: 1 addition & 1 deletion src/components/MediaPlayer/VideoJS/VideoJSPlayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -880,7 +880,7 @@ function VideoJSPlayer({
dangerouslySetInnerHTML={{ __html: placeholderText }}>
</p>
<p data-testid="inaccessible-message-timer"
className={`ramp--media-player_inaccessible-message-timer ${autoAdvanceRef.current ? '' : 'disabled'}`}>
className={`ramp--media-player_inaccessible-message-timer ${autoAdvanceRef.current ? '' : 'hidden'}`}>
{`Next item in ${messageTime} second${messageTime === 1 ? '' : 's'}`}
</p>
<div className="ramp--media-player_inaccessible-message-buttons">
Expand Down
4 changes: 2 additions & 2 deletions src/components/MediaPlayer/VideoJS/VideoJSPlayer.scss
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,6 @@
color: inherit;
}

.ramp--media-player_inaccessible-message-timer.disabled {
color: $primaryDark;
.ramp--media-player_inaccessible-message-timer.hidden {
visibility: hidden
}

0 comments on commit 99678a7

Please sign in to comment.