linuxmint / nvidia-prime-applet

39 stars 16 forks source link

Applet Menu Reflects a Queued Switch before Logging Out #14

Closed hammy275 closed 2 years ago

hammy275 commented 2 years ago

As of the moment, when a user picks a new option in the menu, the menu doesn't reflect the upcoming change. As a result, the user cannot "cancel" a switch without logging out and logging back in without using the terminal or some other application. This commit solves this by making the following changes:

Here are some comparison screenshots. Apologies for the bad quality, I couldn't find an easy way to get a screen capture of a context menu on its own. For this comparison, I am switching from NVIDIA (Performance Mode) to Intel (Power Saving Mode):

Before Switching (applies to both this PR and the applet as it currently is): image

After Switching with the applet as it currently is:

image

After switching with this PR:

image

Although this should help to improve the user experience with the applet, it comes at the cost of adding a decent amount of code to a relatively-simple program. Additionally, it also adds a translation burden, as this adds two extra strings to the applet. If that combined burden seems too big to warrant merging this PR in (or you feel this shouldn't be merged in for some other reason), I would be happy to work on a middle-ground (if you are interested in one, of course), where the current mode is determined after a switch, not just on the applet's startup. In other words, most code from Tray.__init__ would be moved into a separate function which would be called from both Tray.__init__ and from Tray.switch. This way, the additional code in comparison to how the applet currently stands would be very minimal, and no extra translating would be required.

hammy275 commented 2 years ago

Apparently, renaming branches closes PRs. Will re-open this with master again. Apologies about that.

mtwebster commented 2 years ago

I like the idea of making the applet more aware of the current state but I think maybe this overcomplicates it a little (both for you and for the user).

Maybe once a mode is pending, the menu can change to display only the first line like normal, another 'Log out to finish switching etc..' and lastly one to revert the switch.

So once a mode is pending, there's two possible actions - complete the switch or cancel it. What do you think?

Adding strings isn't really a problem except just prior to a release or during beta, when there's a time crunch.

mtwebster commented 2 years ago

I like the idea of making the applet more aware of the current state but I think maybe this overcomplicates it a little (both for you and for the user).

Maybe once a mode is pending, the menu can change to display only the first line like normal, another 'Log out to finish switching etc..' and lastly one to revert the switch.

So once a mode is pending, there's two possible actions - complete the switch or cancel it. What do you think?

Adding strings isn't really a problem except just prior to a release or during beta, when there's a time crunch.

hammy275 commented 2 years ago

That honestly sounds perfect! I can work on changing my PR to that later today! :)

hammy275 commented 2 years ago

I was force pushing to remove a commit to change to the newly proposed system. GitHub decided to autoclose it, so I'll re-open once I've pushed the change

hammy275 commented 2 years ago

I've re-tooled the applet to reflect the functionality from your comment, @mtwebster . Here's a screenshot of what it looks like when you go to cancel a switch:

The menu: image

When you click "Cancel Switch": image

Note that the menu does not change if a user selects "No" at the confirmation prompt, or if the operation fails in some way (such as cancelling at the pkexec prompt).

EDIT: A minor bug that this introduces is if nvidia-prime-applet is Quit then re-opened without logging out but after declaring a switch, the applet thinks the switch already took place, even though in practice, the mode hasn't been switched yet because the user hasn't logged out. I'm open to any solutions to this, or just leaving it as-is.