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

Fix track scrubber bugs in Videojs re-setup work #484

Merged
merged 1 commit into from
Apr 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
31 changes: 15 additions & 16 deletions src/components/MediaPlayer/VideoJS/VideoJSPlayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,15 +215,18 @@ function VideoJSPlayer({
player: player,
type: 'updatePlayer',
});
} else if (playerRef.current && options.sources?.length == 0) {
}
}, [options.sources, videoJSRef]);

React.useEffect(() => {
if (playerRef.current) {
// For empty Canvas pause the player if it's playing
if (isPlayingRef.current) { playerRef.current.pause(); }
// Disable hotkeys for avoid playback on the underlying player
document.removeEventListener('keydown', playerHotKeys);
// Set the player's aspect ratio to video
playerRef.current.audioOnlyMode(false);
playerRef.current.canvasIsEmpty = true;
}
}, [options.sources, videoJSRef]);
}, [canvasIsEmpty]);

/**
* Build track HTML for Video.js player on initial page load
Expand All @@ -249,6 +252,7 @@ function VideoJSPlayer({
player.srcIndex = srcIndex;
player.targets = targets;
player.duration(canvasDuration);
player.canvasIsEmpty = canvasIsEmptyRef.current;

// Update textTracks in the player
var oldTracks = player.remoteTextTracks();
Expand Down Expand Up @@ -313,15 +317,13 @@ function VideoJSPlayer({
for both audio and video.
*/
if (!IS_MOBILE) {
const volumeIndex = controlBar.children()
.findIndex((c) => c.name_ == 'VolumePanel');
controlBar.removeChild('volumePanel');
if (!isVideo) {
controlBar.addChild('volumePanel', { inline: true },
player.getChild('controlBar').children().length - 3
);
controlBar.addChild('volumePanel', { inline: true }, volumeIndex);
} else {
controlBar.addChild('volumePanel', { inline: false },
player.getChild('controlBar').children().length - 3
);
controlBar.addChild('volumePanel', { inline: false }, volumeIndex);
}
/*
Trigger ready event to reset the volume slider in the refreshed
Expand Down Expand Up @@ -358,12 +360,9 @@ function VideoJSPlayer({
* @param {Object} player Video.js player instance
*/
const playerLoadedMetadata = (player) => {
player.on('loadedmetadata', () => {
player.one('loadedmetadata', () => {
videojs.log('Player loadedmetadata');

// Enable hotkeys eventlistener after inaccessible items
document.addEventListener('keydown', playerHotKeys);

player.duration(canvasDurationRef.current);

isEndedRef.current ? player.currentTime(0) : player.currentTime(currentTime);
Expand Down Expand Up @@ -516,7 +515,7 @@ function VideoJSPlayer({
elements on the page
*/
document.addEventListener('keydown', (event) => {
playerHotKeys(event, player);
playerHotKeys(event, player, canvasIsEmptyRef.current);
});
};

Expand Down Expand Up @@ -888,7 +887,7 @@ function VideoJSPlayer({
>
</video>
</div>
{((hasStructure || playlist.isPlaylist) && !canvasIsEmptyRef.current) &&
{(hasStructure || playlist.isPlaylist) &&
(<div className="vjs-track-scrubber-container hidden" ref={trackScrubberRef} id="track_scrubber">
<p className="vjs-time track-currenttime" role="presentation"></p>
<span type="range" aria-label="Track scrubber" role="slider" tabIndex={0}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,8 @@ function ProgressBar({
}
timeToolRef.current.style.left =
leftWidth - timeToolRef.current.offsetWidth / 2 + 'px';

handleTimeUpdate(initTimeRef.current);
}, [player.src(), targets]);

// Update progress bar with timeupdate in the player
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@ function TrackScrubberButton({ player, trackScrubberRef, timeToolRef, isPlaylist
playerEventListener = setInterval(() => {
timeUpdateHandler();
}, 100);

}, [player.src(), player.srcIndex]);
if (player.canvasIsEmpty) { setZoomedOut(true); }
}, [player.src(), player.srcIndex, player.canvasIsEmpty]);

/**
* Keydown event handler for the track button on the player controls,
Expand All @@ -132,6 +132,7 @@ function TrackScrubberButton({ player, trackScrubberRef, timeToolRef, isPlaylist
if (e.which === 32 || e.which === 13) {
e.preventDefault();
handleTrackScrubberClick();
e.stopPropagation();
}
};

Expand Down
8 changes: 5 additions & 3 deletions src/services/utility-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -543,9 +543,10 @@ export function autoScroll(currentItem, containerRef, toTop = false) {
* Bind default hotkeys for VideoJS player
* @param {Object} event keydown event
* @param {String} id player instance ID in VideoJS
* @param {Boolean} canvasIsEmpty flag to indicate empty Canvas
* @returns
*/
export function playerHotKeys(event, player) {
export function playerHotKeys(event, player, canvasIsEmpty) {
let playerInst = player?.player();

let inputs = ['input', 'textarea'];
Expand All @@ -564,14 +565,15 @@ export function playerHotKeys(event, player) {
- focus is on a navigation tab AND the key pressed is one of left/right arrow keys
this specific combination of keys with a focused navigation tab is avoided to allow
keyboard navigation between tabbed UI components, instead of triggering player hotkeys
- when key combinations are not in use with a key associated with hotkeys
- key combinations are not in use with a key associated with hotkeys
- current Canvas is empty
*/
if (
(activeElement &&
(inputs.indexOf(activeElement.tagName.toLowerCase()) !== -1 ||
(activeElement.role === "tab" && (pressedKey === 37 || pressedKey === 39))) &&
!focusedWithinPlayer)
|| isCombKeyPress
|| isCombKeyPress || canvasIsEmpty
) {
return;
} else if (playerInst === null || playerInst === undefined) {
Expand Down