Fix re-focusing on pause button when displaying OSD #6510
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Delay testing of the focused element and do not replace the pending element with the pause button until then.
dmitrylyzo
added
bug
Cloudflare Pages deployment
|
FocusManager.isCurrentlyFocusableInternal doesn't work with fixed elements.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -49,7 +49,8 @@ class SkipSegment extends PlaybackSubscriber { | |||
| if (!this.skipElement && this.currentSegment) { | |||
| let buttonHtml = ''; | |||
|
|
|||
| buttonHtml += '<button is="emby-button" class="skip-button hide skip-button-hidden"></button>'; | |||
| // FIXME: Move skip button to the video OSD | |||
There was a problem hiding this comment.
Unexpected 'fixme' comment: 'FIXME: Move skip button to the video OSD'. no-warning-comments
|
|
||
| return debounced; | ||
| }();*/ | ||
| const _focus = function () { |
There was a problem hiding this comment.
'_focus' is assigned a value but never used. no-unused-vars
dmitrylyzo
commented
Feb 4, 2025
Comment on lines
+406
to
+415
| const _focus2 = function (focusElement) { | ||
| // If no focus element is provided, try to keep current focus if it's valid, | ||
| // otherwise default to pause button | ||
| const currentFocus = focusElement || document.activeElement; | ||
| if (!currentFocus || !focusManager.isCurrentlyFocusable(currentFocus)) { | ||
| focusElement = osdBottomElement.querySelector('.btnPause'); | ||
| } | ||
|
|
||
| if (focusElement) focusManager.focus(focusElement); | ||
| }; |
There was a problem hiding this comment.
Hmm, do we need to debounce/timeout focus at all? _focus2 seems to work in webOS 1.2, webOS 5, Firefox 134, Chrome 132.
Maybe timeout was for Internet Explorer? 🤔
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
Delay testing of the focused element and do not replace the pending element with the pause button until then.
Issues
Fixes jellyfin/jellyfin-tizen#331
I made 2 versions: with
lodash/debounceand without it. Which one do you prefer?