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

Replace videojs-hotkeys with builtin hotkeys function #250

Merged
merged 1 commit into from
Oct 2, 2023
Merged

Conversation

masaball
Copy link
Contributor

Videojs-hotkeys was not interfacing correctly between Avalon and embedded Ramp. This could be an issue encountered by other implementers. Rather than cobble together a fix on the Avalon side and documenting that for others, it felt better to try to change the approach in Ramp so that hopefully workarounds would not be necessary.

To that end, recent versions of VideoJS have added a builtin hotkey ability so we are going to try switching to that. This currently requires building the hotkeys out from scratch because the defaults do not include all of the functionality we require and adding any customization disables the defaults. It may be worthwhile exploring contributing "seek" and "volume" hotkey components back upstream to be added to the VideoJS default behaviors but that is a future discussion.

Co-authored-by: Dananji Withana <dwithana@iu.edu>
@Dananji
Copy link
Collaborator

Dananji commented Sep 29, 2023

This looks great 💯

Comment on lines +292 to +297
this.currentTime(this.currentTime() + 5);
}

// Left arrow seeks 5 seconds back
if (event.which === 37) {
this.currentTime(this.currentTime() - 5);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a check here for the duration of the media in case the currentTime after the 5 second adjustment is either greater than the duration or less than 0 or the alternate start time of the media?
Or is it getting handled on the VideoJS end of things?

Copy link
Contributor Author

@masaball masaball Sep 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like it is getting handled on the VideoJS end of things. Trying to seek back past the start time of a video/segment gets set to the start time. Seeking past the end of a video/segment either triggers the autoplay of the next section or, if there are no more sections, triggers the "Transcripts undefined" error we've recently been seeing which is unrelated to this specific seeking behavior.

Neither case seems to triggers any other error messages or weird behavior.

@masaball masaball merged commit d482f3d into main Oct 2, 2023
@masaball masaball deleted the hotkeys branch October 2, 2023 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants