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

Support for Home and Search media keys in TV mode #5955

Merged
merged 1 commit into from
Sep 20, 2024

Conversation

mprasil
Copy link
Contributor

@mprasil mprasil commented Aug 25, 2024

The remote I have has some extra "media" buttons besides the already supported playback controls.

This adds support for "Find" button to navigate to search page and "BrowserHome" button to navigate back to main screen.

Adding these to the NavigationKeys only enables functionality for TV mode which I think is pretty reasonable constraint - on actual desktop people might prefer to use these keys to control the browser rather than Jellyfin interface.

The remote I have has some extra "media" buttons besides the already
supported playback controls.

This adds support for "Find" button to navigate to search page and
"BrowserHome" button to navigate back to main screen.

Adding these to the `NavigationKeys` only enables functionality for TV
mode which I think is pretty reasonable constraint - on actual desktop
people might prefer to use these keys to control the browser rather than
Jellyfin interface.
@mprasil mprasil requested a review from a team as a code owner August 25, 2024 10:56
Copy link

sonarcloud bot commented Aug 25, 2024

@jellyfin-bot
Copy link
Collaborator

Cloudflare Pages deployment

Latest commit fd18c3e60000475d81d9aa23a985b8ab0e962ee9
Status ✅ Deployed!
Preview URL https://cbfcec6a.jellyfin-web.pages.dev
Type 🔀 Preview

@thornbill thornbill added the enhancement Improve existing functionality or small fixes label Aug 27, 2024
@thornbill thornbill added this to the v10.10.0 milestone Aug 27, 2024
Copy link
Member

@thornbill thornbill left a comment

Choose a reason for hiding this comment

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

The code changes look good to me. @dmitrylyzo is this something you would like to test?

@dmitrylyzo
Copy link
Contributor

The code changes look good to me. @dmitrylyzo is this something you would like to test?

Yeah, the code looks fine, but I don't know which buttons in the emulator the new codes correspond to.

@mprasil
Copy link
Contributor Author

mprasil commented Sep 4, 2024

@dmitrylyzo
Copy link
Contributor

@dmitrylyzo would this help? https://developer.mozilla.org/en-US/docs/Web/API/UI_Events/Keyboard_event_key_values

I mean, I tried all buttons in the webOS 5 emulator and didn't get these codes. Or do you mean you have some sort of universal remote connected to the TV (mini keyboard)?

My main concern is the possibility of unnecessary overriding of default actions. All I can say is that Home (🏠, opens LG launcher) on the LG remote generates Meta code, so it doesn't seem to be overridden.

Btw, there's BrowserSearch code. I suppose your remote generates a more generic code (Find), right?

@mprasil
Copy link
Contributor Author

mprasil commented Sep 4, 2024

Or do you mean you have some sort of universal remote connected to the TV (mini keyboard)?

Yes this is exactly my situation. I do think some "multimedia" keyboards also have some of these buttons.

My main concern is the possibility of unnecessary overriding of default actions. All I can say is that Home (🏠, opens LG launcher) on the LG remote generates Meta code, so it doesn't seem to be overridden.

Yeah some remotes have Home mapped to Meta as that is what various systems use to go to OS home screen, show start menu, etc.. I would be careful about overriding that one as it would be either captured by OS or it might prevent user from returning to OS home easily. My remote specifically has BrowserHome button which is not Meta.

Btw, there's BrowserSearch code. I suppose your remote generates a more generic code (Find), right?

Yes, although I think one of the remotes I had did generate BrowserSearch instead. As far as browser/OS is concerned these are two totally different buttons. IIRC BrowserSearch is actually Search button which some systems intercept on OS level. (For example Gnome desktop will show overview screen with search focused)

My main concern is the possibility of unnecessary overriding of default actions.

That's why the override only applies to TV interface where both BrowserHome and Find aren't likely to be used for other purpose. FWIW Emby does intercept these in TV mode to go to home screen or show search interface. At the end of the day I'd be actually fine with any shortcut, worst case I'd just remap these keys to anything, but right now there is no keyboard shortcut for either of these actions, so I submitted PR with keys I'd personally use and that used to work in Emby. I do think using these keys makes sense, but any key would work for me.

@mprasil
Copy link
Contributor Author

mprasil commented Sep 5, 2024

@dmitrylyzo if you're using X11 on Linux to run your emulator, I think you can simulate the keys with xdotool key Find and xdotool key XF86HomePage. I'm not sure if there's similar tool for Wayland or other systems. Also not 100% sure about the key codes for xdotool - got them from here.

Copy link
Contributor

@dmitrylyzo dmitrylyzo left a comment

Choose a reason for hiding this comment

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

Using the wireless mini-keyboard, I was able to use Home in the browser (in TV mode).
Find did not work (KRunner menu appeared).
In Tizen and webOS, the buttons don't work. But they are unlikely to conflict.
I think in the future we will probably have a key mapping controller - we will be able to reassign them.

Since it works for you. LGTM

@thornbill thornbill merged commit c2aca25 into jellyfin:master Sep 20, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improve existing functionality or small fixes
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants