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

js: add support for keydown events #678

Merged
merged 2 commits into from
Aug 16, 2019
Merged

Conversation

leonklingele
Copy link
Contributor

@leonklingele leonklingele commented Aug 8, 2019

This will modify the player behavior even if the player element is unfocused.

Based on the YouTube key bindings, allow to

  • toggle playback with space and 'k' key
  • increase and decrease player volume with up / down arrow key
  • mute and unmute player with 'm' key
  • jump forwards and backwards by 5 seconds with right / left arrow key
  • jump forwards and backwards by 10 seconds with 'l' / 'j' key
  • set video progress with number keys 0–9
  • toggle captions with 'c' key
  • toggle fullscreen mode with 'f' key
  • play next video with 'N' key
  • increase and decrease playback speed with '>' / '<' key

Also, remove a JavaScript dependency (videojs.hotkeys.min.js) which is no longer required.

@leonklingele
Copy link
Contributor Author

Fixed an issue where changing the captions via the captions menu item would interfere with changing it through the 'c' keyboard shortcut.

@omarroth
Copy link
Contributor

omarroth commented Aug 9, 2019

Very nice, would appear to close #501 and #667.

Since this PR is adding support for hotkeys page-wide it should be possible to remove L47-L101 in player.js.

For Unhandled key down events, is it reasonable to ignore these or forward them to the page if the player doesn't handle them?

@omarroth
Copy link
Contributor

omarroth commented Aug 9, 2019

Looks like you can also do:

diff --git a/assets/js/watch.js b/assets/js/watch.js
index 0f3e812..c8dd729 100644
--- a/assets/js/watch.js
+++ b/assets/js/watch.js
@@ -415,20 +415 @@ if (video_data.play_next) {
-        var url = new URL('https://example.com/watch?v=' + video_data.next_video);
-
-        if (video_data.params.autoplay || video_data.params.continue_autoplay) {
-            url.searchParams.set('autoplay', '1');
-        }
-
-        if (video_data.params.listen !== video_data.preferences.listen) {
-            url.searchParams.set('listen', video_data.params.listen);
-        }
-
-        if (video_data.params.speed !== video_data.preferences.speed) {
-            url.searchParams.set('speed', video_data.params.speed);
-        }
-
-        if (video_data.params.local !== video_data.preferences.local) {
-            url.searchParams.set('local', video_data.params.local);
-        }
-
-        url.searchParams.set('continue', '1');
-        location.assign(url.pathname + url.search);
+        next_video();

@leonklingele
Copy link
Contributor Author

leonklingele commented Aug 9, 2019

Applied your patch, changed a few lets to consts, updated function increase_playback_rate to rely on the playback rates available from the preferences, removed js dependency videojs.hotkeys.min.js which might interfere with our own hotkeys.
Removing the dependency also removes support for controlling the player volume by scrolling over it (enableHoverScroll ).

PTAL @omarroth

assets/js/player.js Outdated Show resolved Hide resolved
@omarroth
Copy link
Contributor

omarroth commented Aug 9, 2019

I would still like hover scroll to be supported. You might take a look at the relevant code videojs-hotkeys for how it could be implemented.

Left a couple comments. Quite excited to merge this.

@leonklingele
Copy link
Contributor Author

What's the benefit of having support for scroll-based volume-adjustments compared to the good old slider which can be adjusted by moving the mouse?

@omarroth
Copy link
Contributor

It makes it possible to change volume without switching focus to another window. It's mostly a QoL feature, however since both Invidious and YouTube currently support it I would like for it to still be available.

@leonklingele
Copy link
Contributor Author

Brought back support for volume-scrolling.

PTAL @omarroth

@leonklingele
Copy link
Contributor Author

Rebased to latest master, CI should pass again.

This will modify the player behavior even if the player element is unfocused.

Based on the YouTube key bindings, allow to

- toggle playback with space and 'k' key
- increase and decrease player volume with up / down arrow key
- mute and unmute player with 'm' key
- jump forwards and backwards by 5 seconds with right / left arrow key
- jump forwards and backwards by 10 seconds with 'l' / 'j'  key
- set video progress with number keys 0–9
- toggle captions with 'c' key
- toggle fullscreen mode with 'f' key
- play next video with 'N' key
- increase and decrease playback speed with '>' / '<' key
Support for controlling the player volume by scrolling over it is
still retained by copying over the relevant code part from the
aforementioned library.
@leonklingele
Copy link
Contributor Author

CI is failing due to a bug in the Crystal build toolchain: https://travis-ci.org/omarroth/invidious/jobs/572082045#L279-L286
See also crystal-lang/crystal#7558.

@omarroth could you please trigger a CI rebuild?

@omarroth
Copy link
Contributor

LGTM 👍

@omarroth omarroth merged commit e6b4e12 into iv-org:master Aug 16, 2019
greentornado added a commit to greentornado/invidious that referenced this pull request Aug 22, 2019
* upstream/master:
  Add 'playlistThumbnail' to playlist objects
  Use accurate sub count when available
  Refactor search extractor
  Fix allowed_regions for globally blocked videos
  js: add support to detect alt, meta and control key in keydown handler (iv-org#704)
  Fix playlist_thumbnail extractor
  js: add support for keydown events (iv-org#678)
  Change font family to better native selection (iv-org#679)
  Fix season playlists
  Add prefers-color-scheme support (iv-org#601)
@github-actions
Copy link

This pull request has been automatically locked since there has not been any activity in it in the last 30 days. If you want to tell us about needed or wanted changes or if problems related to this code are discovered, feel free to open an issue or a new pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants