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

improve keyboard shortcuts #5717

Merged
merged 3 commits into from
Mar 22, 2021
Merged

improve keyboard shortcuts #5717

merged 3 commits into from
Mar 22, 2021

Conversation

rafael-xmr
Copy link
Collaborator

@rafael-xmr rafael-xmr commented Mar 19, 2021

PR Checklist

Please check all that apply to this PR using "x":

  • I have checked that this PR is not a duplicate of an existing PR (open, closed or merged)
  • I have checked that this PR does not introduce a breaking change
  • This PR introduces breaking changes and I have provided a detailed explanation below

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting)
  • Refactoring (no functional changes)
  • Documentation changes
  • Other - Please describe:

Fixes

Issue Number:

What is the current behavior?

Keyboard shortcuts are limited

What is the new behavior?

Changed left and right arrow keys to move 5 in time, added J and L to move 10 and K for pause. Also added up and down keybinds for 5% volume change, now mimicking other platforms keyboard shortcuts

Other information

Copy link
Contributor

@DispatchCommit DispatchCommit left a comment

Choose a reason for hiding this comment

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

Bless, looks great, nice quality of life improvement! :)

I pushed a fix for some of the linting errors - should pass test now.

if tests pass, it's good to merge.

@infinite-persistence
Copy link
Contributor

infinite-persistence commented Mar 20, 2021

  • Up/Down would also scroll the page.
    • I wanted to do up/down for volume in the past, but the current approach would also scroll page down. I think we are listening to the event at too high up of a component.

@rafael-xmr
Copy link
Collaborator Author

  • Up/Down would also scroll the page.

    • I wanted to do up/down for volume in the past, but the current approach would also scroll page down. I think we are listening to the event at too high up of a component.

Yeah I noticed that, but it works great on fullscreen mode!

@neb-b neb-b merged commit 954966a into lbryio:master Mar 22, 2021
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.

4 participants