Skip to content

Fix focus loss caused by sliders affecting keyboard shortcuts (follow-up to #1650)#1694

Merged
Arnei merged 1 commit intoopencast:developfrom
ferishili:eth-issue-20-repair-focus
Apr 15, 2026
Merged

Fix focus loss caused by sliders affecting keyboard shortcuts (follow-up to #1650)#1694
Arnei merged 1 commit intoopencast:developfrom
ferishili:eth-issue-20-repair-focus

Conversation

@ferishili
Copy link
Copy Markdown
Contributor

@ferishili ferishili commented Apr 8, 2026

This PR is a follow-up/fix for the new keyboard shortcuts introduced in #1650.

The issue occurred because the sliders (zoom and volume) were taking focus during interaction and not returning it afterward to the main element. As a result, keyboard shortcuts stopped working once users adjusted those sliders.

Solution

This PR introduces a MainFocusContext that allows components to restore focus back to the main workspace after slider interaction completes.

Specifically:

  • A focus context is added and provided at the main content level
  • Components using sliders consume this context
  • Slider onChangeCommitted events trigger restoring focus to the main workspace
  • Keyboard shortcuts remain functional after slider usage
  • The MainFocusContext can be used also in other places later on!

Result

After adjusting zoom or volume sliders:

  • focus returns to the main element automatically
  • keyboard shortcuts continue working as expected
  • behavior stays consistent with the intended shortcut workflow

UPDATE

This PR had a revert on the above mentioned solution and now got newer changes: #1694 (comment)

@ferishili ferishili requested a review from Arnei April 8, 2026 11:56
@ferishili ferishili self-assigned this Apr 8, 2026
@ferishili ferishili added type:bug Something isn't working type:enhancement New feature or request labels Apr 8, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 2026

This pull request is deployed at test.editor.opencast.org/1694/2026-04-10_12-36-21/ .
It might take a few minutes for it to become available.

@Arnei
Copy link
Copy Markdown
Member

Arnei commented Apr 9, 2026

I can reproduce the issue you've described and this PR does fix it. However, I am not quite happy with the solution.

  • This solution moves the focus to the start of the view. If you are navigating by keyboard, this sudden jump in focus is very disorienting.
  • This breaks keyboard controls on the slider. For example, I could adjust the slider by pressing "<-" or "->" on my keyboard. Now, the slider loses focus after a single keystroke.

Do you think there might be another solution? Since the issue seems to lie with key events being caught by the sliders, maybe the sliders could let the key events they don't need propagate? E.g. keep behaviour for "<-" and "->", but let "Space" propagate to trigger the video play/pause?

@ferishili ferishili force-pushed the eth-issue-20-repair-focus branch from f4fa8d8 to 3f73aac Compare April 10, 2026 12:35
@ferishili
Copy link
Copy Markdown
Contributor Author

@Arnei, thanks for the review and for catching that!

I've updated the implementation to use the enableOnFormTags option provided by react-hotkeys-hook. To make this work precisely, I've made the following changes:

  • Role Injection: I explicitly added role="slider" to the Sliders in both locations. This allows the hotkey logic to identify them specifically.
  • Centralized Configuration: To maintain reusability and keep the code cleaner, I've bundled these options into the KEYMAP... constant. Now, any call to useHotkeys can pull the correct options from a single source of truth.

Please take another look. Since this touched the configuration for all hotkeys, a quick sanity test on the general hotkey behavior everywhere would be much appreciated!

Copy link
Copy Markdown
Member

@Arnei Arnei left a comment

Choose a reason for hiding this comment

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

This works and looks reasonable to me, thank you!

I've noticed some weird behaviour, where if you drag the zoom slider with the mouse, then click on the timeline, the "-->" and "<--" keys will move both the zoom slider and the scrubber. But that behaviour was already present previously, so it should not block this PR.

@Arnei Arnei merged commit 289944c into opencast:develop Apr 15, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:bug Something isn't working type:enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants