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

Added support for spatial navigation (Navigation using arrow keys) and also support for tv remote keys #1885

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gibahjoe
Copy link
Contributor

@gibahjoe gibahjoe commented Feb 15, 2023

This PR adds support for Spatial navigation which is especially useful when using jellyfin-vue on a TV.

Also support for TV remote keys during playback (Tested with LGTV remote) was also added.

The main focus of this PR is to let TV users select items on their screen using the arrow keys of the remote and also to allow the remote media keys (tested with LGTV magic remote, webOS 5 and 6 but it can be easily expanded for other tv types if their keycodes are different).

@jellyfin-bot jellyfin-bot added frontend:plugins Pull requests that edit or add Nuxt plugins vue Pull requests that edit or add Vue files labels Feb 15, 2023
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@ferferga ferferga added the discussion needed This PR or issue needs discussion before going further label Mar 14, 2023
@jellyfin-bot jellyfin-bot added the merge conflict Something has merge conflicts label Apr 3, 2023
@Ritchie1108
Copy link

so, anybody knows? does this project support smart tv remote?
i want to use it to replace jellyfin-web in jellyfin-tizen

@ThibaultNocchi
Copy link
Member

ThibaultNocchi commented Apr 7, 2023

I haven't yet gone around to review this PR, we had a lot of work with the recent Vue 3 merge.

I don't mind adding support for keys and remotes, what I don't guarantee is that the client will work on smart TVs. We use a pretty recent software stack, with which TVs usually aren't compatible.

But adding support for keys isn't something I'll block, I just need to take some time :)

Also, @gibahjoe , can you resolve the conflicts with the codebase please? :)

@Ritchie1108
Copy link

sorry, im not good at web code, so im not not sure what the browser version this project depends on
maybe chromium m85?

@ThibaultNocchi
Copy link
Member

ThibaultNocchi commented Apr 7, 2023

It seems at least to need Chromium 87, as Vite specifies, and this isn't checking the other modules we use.

@Ritchie1108
Copy link

oops, so sad

@gibahjoe
Copy link
Contributor Author

gibahjoe commented Apr 8, 2023

@ThibaultNocchi The previous vue app worked on TVs (tested on LGTV) but Vite didn't as at 3 months ago when I tested it. I will take a look at it again and resolve these merge conflicts

@ferferga
Copy link
Member

ferferga commented Apr 8, 2023

@gibahjoe Still it's useful if this can be navigated through keyboard for accessibility reasons or TVs that have latest Chromium versions.

There's no harm in supporting as much use cases as possible, as long as the support is good!

@gibahjoe
Copy link
Contributor Author

Update:

I already began work on this a few days back. Unfortunately, the plug-in I used previously doesn't support Vue 3 so I have had to copy over the code and make a couple of adjustments.

Anyway, I should be done by this weekend.

@ferferga
Copy link
Member

@gibahjoe The other day I was thinking about the media type redesign and it came across to my mind again what to do for this PR.

In general, I think we should have three views for libraries: card grid (the only one right now), coverflow one (the current one for fullscreen music player, which is already useful for touch-based units like Echo Show or Android Auto but it also might be a presentation people like for books and TVs) and a list view (specially for music)

For the home sections, music and that hypothetical coverflow view, we already have full keyboard support through swiper.

Why I'm saying all of this? Maybe before you do further work here, perhaps we should gather feedback from people who might benefit from this the most. I'm not a TV user myself, so all I can do is give my review in the impl details, etc, but maybe not much insight in how it should be UX-wise

@gibahjoe gibahjoe closed this Apr 16, 2023
@jellyfin-bot jellyfin-bot removed frontend:plugins Pull requests that edit or add Nuxt plugins vue Pull requests that edit or add Vue files labels Apr 16, 2023
… keys on keyboard and remote control. Also add support for play/pause button on webos

# Conflicts:
#	frontend/src/store/playbackManager.ts
@gibahjoe gibahjoe reopened this Apr 16, 2023
@jellyfin-bot jellyfin-bot added vue Pull requests that edit or add Vue files and removed merge conflict Something has merge conflicts labels Apr 16, 2023
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

currentFocusedElement.blur();
}

focusNscroll(elem);

Check warning

Code scanning / CodeQL

Superfluous trailing arguments

Superfluous argument passed to [function focusNscroll](1).
return false;
}

focusNscroll(elem);

Check warning

Code scanning / CodeQL

Superfluous trailing arguments

Superfluous argument passed to [function focusNscroll](1).
// This might also be split into different directives to handle remove eventlisteners
Vue.directive('focus-events', {
mounted(el, binding) {
for (const [i, key] of Object.keys(binding.value).entries()) {

Check notice

Code scanning / CodeQL

Unused variable, import, function or class

Unused variable i.
for (const candidate of candidates) {
var rect = getRect(candidate);

if (rect) {

Check warning

Code scanning / CodeQL

Useless conditional

This use of variable 'rect' always evaluates to true.
@jellyfin-bot
Copy link

Cloudflare Pages deployment

Latest commit ba6e882
Status ✅ Deployed!
Preview URL https://jf-vue.pages.dev (https://0d8cb1de.jf-vue.pages.dev)
Type ⚙️ Production

View build logs
View bot logs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Something depends on another issue or Pull Request discussion needed This PR or issue needs discussion before going further merge conflict Something has merge conflicts vue Pull requests that edit or add Vue files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants