Skip to content

feat(Player Shortcuts): Add the shortcut control functionality to player controls - #594

Merged
KhooLy merged 11 commits into
NuvioMedia:Devfrom
AdityasahuX07:patch-7
Sep 6, 2026
Merged

feat(Player Shortcuts): Add the shortcut control functionality to player controls#594
KhooLy merged 11 commits into
NuvioMedia:Devfrom
AdityasahuX07:patch-7

Conversation

@AdityasahuX07

@AdityasahuX07 AdityasahuX07 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR vastly expands the keyboard accessibility and shortcut functionality of the desktop video player, mirroring power-user features found in modern media players and smart TV interfaces. It implements over a dozen new hotkeys, adds intelligent smart-focus logic for side panels (episodes, sources, subtitles, audio, speed), introduces instant-apply navigation for specific modals, and fixes several focus clipping and asynchronous state-sync bugs.

PR type

  • Reproducible bug fix
  • UI glitch/bug fix
  • Behavior bug/regression fix
  • Small maintenance only, with no UI or behavior change
  • Docs accuracy fix
  • Translation/localization only
  • Approved larger or directional change

Why

Desktop users lacked a comprehensive set of hotkeys to control playback, UI panels, and media settings seamlessly without reaching for the mouse. Navigating complex nested menus (like subtitles or episodes) purely via keyboard was cumbersome because focus didn't smartly track the active item, tabs required manual tabbing, and rapid shortcut inputs suffered from backend state-sync lag. This PR solves these issues to provide a fluid, fully keyboard-drivable desktop player experience.

Desktop scope

This PR is scoped purely to the desktop video player UI (controls.js, controls.css), running inside the desktop webview overlay. It does not affect mobile or core backend playback logic.

Issue or approval

Fixes #109, Fixes #170, Fixes #172, Fixes #207, Fixes #456

UI / behavior impact

  • No UI change
  • No behavior change
  • UI changed only to fix a documented glitch/bug
  • Behavior changed only to fix a documented bug/regression
  • UI change has explicit maintainer approval
  • Behavior change has explicit maintainer approval

Policy check

  • I have read and understood CONTRIBUTING.md.
  • This PR is small, focused, and limited to one problem.
  • This PR is scoped to the desktop app, desktop packaging, desktop documentation, or shared code required for desktop behavior.
  • This PR is not cosmetic-only.
  • Any UI change fixes a linked glitch/bug and includes visual proof, or this PR has no UI change.
  • Any behavior change fixes a linked bug/regression or has explicit approval, or this PR has no behavior change.
  • This PR does not bundle unrelated refactors, cleanups, formatting, or drive-by changes.
  • This PR does not add dependencies, architecture changes, migrations, or product-direction changes without explicit approval.
  • I listed the testing performed below.

UI polish, cosmetic-only changes, minor behavior tweaks, and unapproved product changes will be closed without review.

Scope boundaries

This PR focuses exclusively on keyboard event bindings, focus management, and local state debouncing for the player UI webview. It does not touch Kotlin-side media session routing, platform-specific media keys, or restructure the core DOM layout.

Testing

Manual Testing via Desktop App (Windows/Mac):

  • Panel Toggles: Verified a (Audio), s (Subtitles), e (Episodes), q (Sources), and ` (Speed) seamlessly open and close their respective modals.
  • Smart Focus: Opened Episodes/Sources/Subtitles panels and verified that pressing Arrow Up/Down instantly finds the currently playing item and snaps focus relative to it.
  • Instant Apply Navigation: Opened Audio/Speed panels and verified that Arrow Up/Down instantly selects and applies the highlighted track without needing to press Enter.
  • Tab Switching: Verified Left/Right arrows switch Season and Stream Provider tabs when navigating the Episodes/Sources modals.
  • Rapid Input Sync: Hammered the < and > speed shortcuts and verified the local pendingSpeedIndex successfully debounces the inputs and correctly hits 2x without dropping presses due to asynchronous backend lag.
  • Mute Sync: Toggled m and verified the volume slider immediately syncs and displays the standard desktop volume toast pill (e.g. 0% and 100%).
  • Additional Hotkeys Tested: Enter (Skip intro), Shift+N (Next episode), v (Toggle subtitle), b (Cycle audio), g/h (Sub delay), o/p/i (Sub opacity), f (Fullscreen), and Right Click (Play/Pause).
  • Visuals: Verified the focus ring on list items (.track-row) no longer clips at the left/right edges of the scroll container.

Screenshots / Video

Not a UI change

Breaking changes

None.

Linked issues

Fixes #109, Fixes #170, Fixes #172, Fixes #207, Fixes #456

@AdityasahuX07 AdityasahuX07 changed the title Add the shortcut control functionality to player controls feat(Player Shortcuts): Add the shortcut control functionality to player controls Sep 2, 2026
@KhooLy

KhooLy commented Sep 3, 2026

Copy link
Copy Markdown
Member

Looks good overall, but please keep the current left-click play/pause behavior and Escape-to-exit-fullscreen behavior. This PR currently removes the left-click toggle and makes Escape always go back.

@AdityasahuX07

AdityasahuX07 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Looks good overall, but please keep the current left-click play/pause behavior and Escape-to-exit-fullscreen behavior. This PR currently removes the left-click toggle and makes Escape always go back.

  • I switch left click play/pause to right click because its left clicks primary use to select and muscle memory is making it unintentionally pauses this play pause while try to hide the OSC with left click. Also still works on click right click to play/ pause.

  • Escape-to-exit-Fullscreen change to exit player intentionally changed because in whole app what globally used esc as back so when player it must close instead of exit Fullscreen, for exit Fullscreen there are 2 option already available via F11 or F

@AdityasahuX07

AdityasahuX07 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author
  • I switch left click play/pause to right click because its left clicks primary use to select and muscle memory is making it unintentionally pauses this play pause while try to hide the OSC with left click. Also still works on click right click to play/ pause.
  • Escape-to-exit-Fullscreen change to exit player intentionally changed because in whole what globally used esc as back so when player it must close instead of exit Fullscreen, for exit Fullscreen there are 2 option already available via F11 or F

@KhooLy do still want to revert it. after my explanation why it changed.

@KhooLy

KhooLy commented Sep 4, 2026

Copy link
Copy Markdown
Member

Please rebase onto current Dev and keep the existing left-click play/pause and Escape-to-exit-fullscreen behaviors. Those are currently regressed by this PR.

@AdityasahuX07

Copy link
Copy Markdown
Contributor Author

Please rebase onto current Dev and keep the existing left-click play/pause and Escape-to-exit-fullscreen behaviors. Those are currently regressed by this PR.

I can revert the play/pause to left click but esc to go back should be as my PR is correct

esc is correct behavior because when to exit fullscreen F and F11 can be used but to close the player window what is use there is no option in keyboad to close player via keyboard

  • right now you want this me to make-
    exit fullscreen - esc, F, F11 [total no. of ways 3]
    to close player - none [total no. of ways 0]

@KhooLy

KhooLy commented Sep 4, 2026

Copy link
Copy Markdown
Member

Yes, please revert both. Escape is intentionally layered: it closes a modal first, exits fullscreen second, and goes back from the player when already windowed. So the player is still fully closable from the keyboard — from fullscreen, pressing Esc twice exits fullscreen and then leaves the player. F/F11 are additional fullscreen shortcuts, not a replacement for the standard Esc behavior. Please also keep left-click play/pause as requested.

@AdityasahuX07

Copy link
Copy Markdown
Contributor Author

Yes, please revert both. Escape is intentionally layered: it closes a modal first, exits fullscreen second, and goes back from the player when already windowed. So the player is still fully closable from the keyboard — from fullscreen, pressing Esc twice exits fullscreen and then leaves the player. F/F11 are additional fullscreen shortcuts, not a replacement for the standard Esc behavior. Please also keep left-click play/pause as requested.

But my real Q. is what is the option to close the player via keyboard instead then.

@ssjlun

ssjlun commented Sep 4, 2026

Copy link
Copy Markdown

You should make exiting the player be the backspace key that way it doesn't conflict with esc and fullscreen

@KhooLy

KhooLy commented Sep 4, 2026

Copy link
Copy Markdown
Member

The keyboard option to leave the player is still Escape when the player is windowed. In fullscreen, Escape first exits fullscreen, then a second Escape leaves the player. I don't want to add Backspace as a separate player-back shortcut just to bypass that layered behavior.

@ssjlun

ssjlun commented Sep 4, 2026

Copy link
Copy Markdown

But then how do you expect people with HTPC who use only the keyboard to efficiently use keys? Every time you'd want to exit the player you have to click esc and leave fullscreen first? That seems counterintuitive

@KhooLy

KhooLy commented Sep 4, 2026

Copy link
Copy Markdown
Member

Nuvio Desktop isn't designed around HTPC-only keyboard navigation, and pressing Escape twice from fullscreen is not a significant usability burden. I don't want to change the standard Escape behavior for that edge case.

@AdityasahuX07

Copy link
Copy Markdown
Contributor Author

But then how do you expect people with HTPC who use only the keyboard to efficiently use keys? Every time you'd want to exit the player you have to click esc and leave fullscreen first? That seems counterintuitive

this is the reason why I made the esc to go back as it is correct.

@AdityasahuX07

Copy link
Copy Markdown
Contributor Author

Nuvio Desktop isn't designed around HTPC-only keyboard navigation, and pressing Escape twice from fullscreen is not a significant usability burden. I don't want to change the standard Escape behavior for that edge case.

I got it you always use the nuvio app in windowed mode and watch the video player in Fullscreen esc return to windowed is correct according to u but I use nuvio app in fullscreen not just player so esc means just close player is correct to me.

so the people who use app in fullscreen not not esc to exit fullscreen because after close the player they have to toggle to fullscreen again form the windowed and visa versa for people who use app in windowed mode.

I have new solution that please both kind of user.

  • if video player opened form app in windowed mode then esc exit the fullscreen.
  • if video player opened form app in fullscreen then esc just close the player instead of exiting the Fullscreen.

@KhooLy what do u think?

@KhooLy

KhooLy commented Sep 4, 2026

Copy link
Copy Markdown
Member

I understand the distinction now, but I don't want to add launch-state-dependent Escape behavior to this PR. Please keep the existing Escape behavior here. If preserving app fullscreen when leaving the player is a real issue, let's handle that separately as a dedicated fullscreen-state fix.

Refactor speed boost handling and clear timers for improved performance.
@AdityasahuX07

Copy link
Copy Markdown
Contributor Author

I understand the distinction now, but I don't want to add launch-state-dependent Escape behavior to this PR. Please keep the existing Escape behavior here. If preserving app fullscreen when leaving the player is a real issue, let's handle that separately as a dedicated fullscreen-state fix.

reverted both sucessfully.

@KhooLy

KhooLy commented Sep 5, 2026

Copy link
Copy Markdown
Member

The requested left-click and Escape behavior are fixed now, thanks. One remaining issue: plain F no longer toggles fullscreen, even though this PR says it fixes #456 and lists F as tested. Please restore the plain F shortcut without changing the current Escape behavior. Also please remove unrelated Fixes references for issues this PR does not actually implement.

@AdityasahuX07

AdityasahuX07 commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

The requested left-click and Escape behavior are fixed now, thanks. One remaining issue: plain F no longer toggles fullscreen, even though this PR says it fixes #456 and lists F as tested. Please restore the plain F shortcut without changing the current Escape behavior. Also please remove unrelated Fixes references for issues this PR does not actually implement.

this PR indeed fixed issue #456 (just test again this "F" working on my debug build)

this not fixes #91 that I removed form PR description.

Recording.2026-09-05.181408.mp4

@KhooLy

KhooLy commented Sep 5, 2026

Copy link
Copy Markdown
Member

The video may be from a local/debug build, but the current pushed head still has no plain KeyF handler, it only handles F11 and Ctrl+Cmd+F. Please make sure the working F change is actually pushed to the PR. Also, #91 was removed, but there are still other unrelated Fixes references such as #311 and #451. (I tested "F" and didnt work.)

@AdityasahuX07

Copy link
Copy Markdown
Contributor Author

The video may be from a local/debug build, but the current pushed head still has no plain KeyF handler, it only handles F11 and Ctrl+Cmd+F. Please make sure the working F change is actually pushed to the PR. Also, #91 was removed, but there are still other unrelated Fixes references such as #311 and #451. (I tested "F" and didnt work.)

this working on Windows the mac issue fixes via PR #608

@AdityasahuX07

Copy link
Copy Markdown
Contributor Author

The video may be from a local/debug build, but the current pushed head still has no plain KeyF handler, it only handles F11 and Ctrl+Cmd+F. Please make sure the working F change is actually pushed to the PR. Also, #91 was removed, but there are still other unrelated Fixes references such as #311 and #451. (I tested "F" and didnt work.)

#311 shortcut to change playback is added (not used X C Z but added batter YouTube style Shift+< or > added and for reset /)
#451 Play/Pause → toggle pause, Stop → stop playback / exit player, Next episode is added

@KhooLy

KhooLy commented Sep 5, 2026

Copy link
Copy Markdown
Member

#311 and #451 still should not be marked as fixed.

#311 specifically asks for X/C/Z bindings, 0.1x speed steps, and Z toggling between 1.0x and the last non-default speed. This PR adds different shortcuts, so it is related, but does not fully fix that issue.

#451 is specifically about hardware media keys such as Play/Pause, Stop, Next and Previous. Regular keyboard shortcuts inside controls.js are not the same thing.

Also I'm not sure how "F" key works for you, it doesnt work for me. i didnt see anything related to that in code too.

@AdityasahuX07

Copy link
Copy Markdown
Contributor Author

#311 and #451 still should not be marked as fixed.

#311 specifically asks for X/C/Z bindings, 0.1x speed steps, and Z toggling between 1.0x and the last non-default speed. This PR adds different shortcuts, so it is related, but does not fully fix that issue.

#451 is specifically about hardware media keys such as Play/Pause, Stop, Next and Previous. Regular keyboard shortcuts inside controls.js are not the same thing.

Also I'm not sure how "F" key works for you, it doesnt work for me. i didnt see anything related to that in code too.

removed #311 and #451

F key is working as I show the evidence video.

@KhooLy

KhooLy commented Sep 5, 2026

Copy link
Copy Markdown
Member

Thanks for the updates. I did another full pass over the current code and there are still a few keyboard navigation regressions I’d like fixed before merging. Please restore the text-entry guard so shortcuts don’t interfere with inputs, scope the arrow-key focus navigation to the active modal instead of moving through every focusable element in the whole player, and make Enter trigger Skip Intro only when the player/root has focus so it doesn’t override Enter on focused buttons. The source-list virtualization and F behavior can be handled separately if needed. Once those focus/input issues are fixed, I’m fine with merging this.

@AdityasahuX07

Copy link
Copy Markdown
Contributor Author

Thanks for the updates. I did another full pass over the current code and there are still a few keyboard navigation regressions I’d like fixed before merging. Please restore the text-entry guard so shortcuts don’t interfere with inputs, scope the arrow-key focus navigation to the active modal instead of moving through every focusable element in the whole player, and make Enter trigger Skip Intro only when the player/root has focus so it doesn’t override Enter on focused buttons. The source-list virtualization and F behavior can be handled separately if needed. Once those focus/input issues are fixed, I’m fine with merging this.

that intentional to move focus for hands free use,

@KhooLy

KhooLy commented Sep 5, 2026

Copy link
Copy Markdown
Member

I understand that moving focus is intentional, but the issue is the scope, not the idea itself. Right now the arrow handler can take over arrow keys even when a text input, slider, or unrelated player control has focus, so it changes existing seek/volume/input behavior. You can keep the hands-free focus navigation, just limit it to the active modal and keep the text-entry guard so normal controls still behave normally. The Enter/Skip Intro conflict is separate and should also be guarded so Enter still activates a focused button.

@AdityasahuX07

Copy link
Copy Markdown
Contributor Author

I understand that moving focus is intentional, but the issue is the scope, not the idea itself. Right now the arrow handler can take over arrow keys even when a text input, slider, or unrelated player control has focus, so it changes existing seek/volume/input behavior. You can keep the hands-free focus navigation, just limit it to the active modal and keep the text-entry guard so normal controls still behave normally. The Enter/Skip Intro conflict is separate and should also be guarded so Enter still activates a focused button.

No, this not affect to other key binding such as volume and seek the navigation arrow keys is just activate when the audio, subtitle, source episode and playback speed panel is visible

Enter only triggers skip when the skip intro/recap/outro prompt is actually showing on screen. At all other times, Enter does nothing.

@AdityasahuX07

Copy link
Copy Markdown
Contributor Author

@KhooLy all tested works as expected now its ready for merge.

@KhooLy

KhooLy commented Sep 5, 2026

Copy link
Copy Markdown
Member

Thanks, most of the issues are fixed now. I checked the latest commit and there are just two things left: the generic arrow-key focus handler still has no activeModal guard, so it can still override seek/volume when a normal player control has focus, and Enter still overrides a focused button whenever the skip prompt is visible. Please scope the generic arrow navigation to an active modal and don’t intercept Enter when another interactive control has focus. After that I’m good to merge.

@KhooLy

KhooLy commented Sep 5, 2026

Copy link
Copy Markdown
Member

One small localization thing: please don’t hardcode Muted, Volume, Volume up, or Volume down in controls.js. We already have localized compose_player_muted and compose_player_volume_level strings, and the actual volume percentage is available, so just pass those localized labels into the player state and show the percentage instead of using the English Volume up/down fallbacks.

Refactor event handling for arrow keys and Enter key to check active element conditions.
@AdityasahuX07

Copy link
Copy Markdown
Contributor Author

Thanks, most of the issues are fixed now. I checked the latest commit and there are just two things left: the generic arrow-key focus handler still has no activeModal guard, so it can still override seek/volume when a normal player control has focus, and Enter still overrides a focused button whenever the skip prompt is visible. Please scope the generic arrow navigation to an active modal and don’t intercept Enter when another interactive control has focus. After that I’m good to merge.

Done.

@AdityasahuX07

Copy link
Copy Markdown
Contributor Author

One small localization thing: please don’t hardcode Muted, Volume, Volume up, or Volume down in controls.js. We already have localized compose_player_muted and compose_player_volume_level strings, and the actual volume percentage is available, so just pass those localized labels into the player state and show the percentage instead of using the English Volume up/down fallbacks.

Done.

@KhooLy

KhooLy commented Sep 5, 2026

Copy link
Copy Markdown
Member

The keyboard fixes look good now, but the localization change isn’t actually wired up yet. controls.js reads state.mutedLabel and state.volumeLevelLabelFormat, but this PR still only changes controls.js and those values are never passed from Kotlin, so it will always fall back to the hardcoded English Muted / Volume %s. Please pass compose_player_muted and compose_player_volume_level through PlayerControlsState/the player JSON state as well. Also please remove english hardcoded fallbacks.

@AdityasahuX07

Copy link
Copy Markdown
Contributor Author

The keyboard fixes look good now, but the localization change isn’t actually wired up yet. controls.js reads state.mutedLabel and state.volumeLevelLabelFormat, but this PR still only changes controls.js and those values are never passed from Kotlin, so it will always fall back to the hardcoded English Muted / Volume %s. Please pass compose_player_muted and compose_player_volume_level through PlayerControlsState/the player JSON state as well. Also please remove english hardcoded fallbacks.

check now

@KhooLy

KhooLy commented Sep 5, 2026

Copy link
Copy Markdown
Member

Code looks good now. One last cleanup before merging: please remove Fixes #263 and Fixes #486 from the PR description, since those behaviors already existed in the base branch and this PR isn’t what actually implements them. After that, I’m good with merging.

@AdityasahuX07

Copy link
Copy Markdown
Contributor Author

Code looks good now. One last cleanup before merging: please remove Fixes #263 and Fixes #486 from the PR description, since those behaviors already existed in the base branch and this PR isn’t what actually implements them. After that, I’m good with merging.

Done but please close them as completed.

@KhooLy

KhooLy commented Sep 6, 2026

Copy link
Copy Markdown
Member

Two small things left: your newly added plain F shortcut should respect the text-input guard, and modal arrow navigation should query focusable elements inside the active modal instead of the whole document.

Refine fullscreen toggle conditions and modal focus handling.
@AdityasahuX07

Copy link
Copy Markdown
Contributor Author

Two small things left: your newly added plain F shortcut should respect the text-input guard, and modal arrow navigation should query focusable elements inside the active modal instead of the whole document.

Done.

@KhooLy
KhooLy merged commit 21aabee into NuvioMedia:Dev Sep 6, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants