Added Quit App to main menu

[iainsaxonhome]

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 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 here or if that is determined in the Jellyfin Media Player.

I'm also not sure if this should be a function that requires a user confirmation (eg Are you sure you want to exit?).

It also requires translations for other languages.

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

Changes
Add a Quit Application button to the main menu.

[iwalton3]

This feature looks good on my end from a theoretical standpoint but it needs to get tested.

[dmitrylyzo]

We already have exit function in appHost/NativeShell.

Here is how it is solved in the TV apps
When the user presses Back on the remote control or Escape on the keyboard while on the Home screen, appHost executes exit function. It can be overloaded in NativeShell.

jellyfin-web/src/components/apphost.js

Lines 329 to 335 in 838b881

	exit: function () {
	if (!!window.appMode && browser.tizen) {
	askForExit();
	} else {
	doExit();
	}
	},

jellyfin-web/src/components/apphost.js

Lines 276 to 290 in 838b881

	function doExit() {
	try {
	if (window.NativeShell?.AppHost?.exit) {
	window.NativeShell.AppHost.exit();
	} else if (browser.tizen) {
	tizen.application.getCurrentApplication().exit();
	} else if (browser.web0s) {
	webOS.platformBack();
	} else {
	window.close();
	}
	} catch (err) {
	console.error('error closing application: ' + err);
	}
	}

This approach may not fit for JMP in Desktop mode, so it is better to have an exit menu item.

There is also an unused exitmenu feature. Another question is why is it set for browsers?

jellyfin-web/src/components/apphost.js

Line 202 in 838b881

features.push('exitmenu');

[iainsaxonhome]

@dmitrylyzo , thanks for pointing out the existing exit functionality. I was unaware of this code before I issued the JMP PR jellyfin/jellyfin-media-player#196. I tried to use the long-press Escape via keyboard as suggested in input mapping files I found however it didn't exit the Jellyfin Media Player app and assumed there was no functionality for it yet.

I saw there was a Client Settings menu item that appeared to only be used for the various native applications where jellyfin-web is a base so it seemed OK to add the Quit Application link in the same way.

Adding a button to quit the app from the main menu helps because it would allow the user to exit the app (where supported) using any input that can already operate the menus such as a touch screen, gamepad, keyboard, mouse, remote or any emulated device that replicates these like a mini wireless keyboard with trackpad or an Air Mouse.

In the meantime I'll update this PR soon to use "Exit" instead of "Quit" and tie it into the existing code as highlighted in your response for consistency of language and functionality.

@iwalton3 , I'll issue an updated PR to the JMP soon to use the window.NativeShell.AppHost.exit(); method overload approach described in this issue instead of creating a new function for window.NativeShell.quitApp().

[dmitrylyzo]

@iainsaxonhome I think we need to stop pushing exitmenu for browsers (just remove) because we can't close the window from the script.

jellyfin-web/src/components/apphost.js

Line 202 in 838b881

features.push('exitmenu');

Also I could be wrong about usage of exitmenu. Yes, it isn't in use, but can be used for showing a confirmation dialog on exit. We can invent a new one though.

• exit - can exit/close app (currently by Back/Escape - TVs)
• exitmenu - add a menu entry (current PR, can imply exit)
• exitprompt - need a confirmation dialog (can imply exit)

At some point we need to document all these features 😅

[iainsaxonhome]

@dmitrylyzo, definitely agree with documenting these features though I'd say that's often as difficult as naming things and clearing cache :) .

Last time I looked at the code you mentioned I did wonder whether the Web project should be trying to do any of these supported feature checks and move it to the various clients that use the Web as a base however now that I look at the conditional statements again its specifically checking the browser type so that's probably OK in most cases.

If browsers can't be instructed to close a tab programmatically then you're right that it shouldn't be added to the feature list here.

I think the idea of having the 3 separate exit related features is OK as each controls a particular aspect and can work together to achieve an ideal UX for each client.

For example the Jellyfin Media Player app would perhaps have all three enabled so the Exit App menu item appears if its running in TV display mode and full screen, would ideally ask for confirmation to exit and will exit as defined in the AppHost.

On the other hand the Android or iOS apps maybe wouldn't need the exit menu or the confirmation if the exit behaviour is to stash the current data and switch to another app or the home screen (this is a guess on my part).

I am trying to think of a client where the exit menu would be shown, can exit the app but is intended to quick-switch and not require an exit confirmation 🤔 .

[dmitrylyzo]

Last time I looked at the code you mentioned I did wonder whether the Web project should be trying to do any of these supported feature checks and move it to the various clients that use the Web as a base however now that I look at the conditional statements again its specifically checking the browser type so that's probably OK in most cases.

That code is only called for browsers (in apps we overload appHost.supports), so we are probably free to remove the exit part entirely.
Also, I see no usage of the plugins feature and don't know its purpose.

If browsers can't be instructed to close a tab programmatically then you're right that it shouldn't be added to the feature list here.

At the moment, the script can close the window that it has opened using Window.open.
https://developer.mozilla.org/en-US/docs/Web/API/Window/close

For example the Jellyfin Media Player app would perhaps have all three enabled so the Exit App menu item appears if its running in TV display mode and full screen, would ideally ask for confirmation to exit and will exit as defined in the AppHost.

I think this can be achieved by checking viewdata.mode in NativeShell.AppHost.supports or by having a separate list of supported features for each mode.

On the other hand the Android or iOS apps maybe wouldn't need the exit menu or the confirmation if the exit behaviour is to stash the current data and switch to another app or the home screen (this is a guess on my part).

Agree, but for some reason iOS has exit and exitmenu features: https://github.com/jellyfin/jellyfin-expo/blob/3c0d6639d5265e31d322d227c9ac4be27a70a4c9/assets/js/NativeShell.staticjs#L10-L11

Android has only exit (back action exit): https://github.com/jellyfin/jellyfin-android/blob/e04d6dded3a38aedcd4014f923340acddc76c9e4/app/src/main/assets/native/nativeshell.js#L7

Probably, the exit action on mobile doesn't actually close the app, but switches to the previous activity.

I am trying to think of a client where the exit menu would be shown, can exit the app but is intended to quick-switch and not require an exit confirmation.

I would remove that exitmenu from the browser (not to bother users) and the feature is ready.
Currently, confirmation is only shown for Tizen (btw, it also has exitmenu - I just copied it from an old Android app).

jellyfin-web/src/components/apphost.js

Lines 330 to 332 in afd37d6

	if (!!window.appMode && browser.tizen) {
	askForExit();
	} else {

We can add exitprompt later, but it would definitely be nice to have a roadmap for "exit" behavior.
In general, on TV exit is activated by the back action. Samsung recommends asking the user to confirm the exit. On webOS we show the home screen, so that the user can return back to the app (using the back button again).

If we want the "menu exit" be instant and the back action exit with the confirmation, we can add a param (say needConfirm or isBack), which false by default, to AppHost.exit and pass true at the inputManager.

jellyfin-web/src/scripts/inputManager.js

Line 117 in afd37d6

appHost.exit();

[dmitrylyzo]

@iainsaxonhome You forgot to handle click in the navDrawer menu (Library entries are handled automatically).

jellyfin-web/src/scripts/libraryMenu.js

Line 325 in afd37d6

}

        const btnExit = navDrawerScrollContainer.querySelector('.exitApp');
        if (btnExit) {
            btnExit.addEventListener('click', () => {
                appHost.exit();
            });
        }

Or shorter

        navDrawerScrollContainer.querySelector('.exitApp')?.addEventListener('click', () => {
            appHost.exit();
        });

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[iainsaxonhome]

@dmitrylyzo , thanks for spotting that, I was testing in the TV display mode and had missed that addition to the library menu.

Its a little more verbose but I opted to add the event handling the same way as the existing buttons ;) .

[thornbill]

I want to test this with the iOS app before it is merged.

[thornbill]

I have logged an issue in the expo repo to remove this as a supported feature and noted that it will require a new app release for the 10.8 release.

jellyfin/jellyfin-expo#340