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 Quit App functionality #196

Merged
merged 1 commit into from
Jan 12, 2022

Conversation

iainsaxonhome
Copy link
Contributor

This is the JMP side of my PR to the Jellyfin Web project: jellyfin/jellyfin-web#3318

I'm using the Jellyfin Media Player in TV mode as a full screen app (open via Flex Launcher) and needed a way to close the program via the keyboard navigation. To resolve this I've added a Quit Application link below the Sign Out menu item.

Unfortunately I haven't been able to test this in the JMP yet so requires testing but the menu item exists in the menu and is hidden using the appHost.supports() function as per the other web menu items. I'm not sure if further conditional checks to determine if this should appear (eg if its an embedded or iOS/Android app the Quit Application link should not appear) need to happen in the Web project or if that is determined here in the player.

I can see that there was an existing function for the system component to exit the application so I have tried to reuse this. I'm also not sure if it should be a function that requires a user confirmation (eg Are you sure you want to exit?).

Is this something that the Jellyfin Community would consider adding to the application?

@iwalton3
Copy link
Member

You can drop the updated nativeshell.js file into the extension folder and the updated web client into the web client folder on an already installed copy and it should let you test it. I don’t see any outright issues but the code should get tested.

@iainsaxonhome
Copy link
Contributor Author

Thanks, I'll give it a try now. I installed via Flatpak so I'm sure this is the directory I need to be working in: /var/lib/flatpak/app/com.github.iwalton3.jellyfin-media-player/current/active/files/share/jellyfinmediaplayer/web-client/extension.

@iwalton3
Copy link
Member

I think flatpak file systems are read only… it may be a bit more involved, but you can use flatpak-builder to compile it yourself.

@iwalton3
Copy link
Member

There are also some config options that allow you to use a different web directory.

@iainsaxonhome
Copy link
Contributor Author

iainsaxonhome commented Jan 12, 2022

Swapping the /var/lib/flatpak/app/com.github.iwalton3.jellyfin-media-player/current/active/files/share/jellyfinmediaplayer/web-client/desktop for symbolic link to the jellyfin-web/dist didn't work so well so I'll check the config options as you suggest for a better testing solution ;) .

Update: never mind, it might have been Flatpak having a problem with the non-root permissions in the linked directory. Copying the files as root worked. Testing now.

@iainsaxonhome
Copy link
Contributor Author

I've tested this by copying the dist version of the jellyfin-web into Flatpak and applying the commit changes to the extensions/nativeshell.js file and the Quit Application button closes the app as expected :) .

@iwalton3 iwalton3 merged commit 91e8741 into jellyfin:master Jan 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants