Add shortcut to close plugin manager dialog and quit application

[goanpeca]

Fixes #38
Fixes #4

Add shortcut to close plugin manager dialog on mac systems (Cmd+W) and quit application on all other systems (Ctrl+Q)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.10%. Comparing base (e1cee5e) to head (78df5c2).
Report is 6 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #64      +/-   ##
==========================================
+ Coverage   86.89%   87.10%   +0.21%     
==========================================
  Files           9        9              
  Lines        1343     1365      +22     
==========================================
+ Hits         1167     1189      +22     
  Misses        176      176              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Czaki]

Why only on macos system?

[goanpeca]

Why only on macos system?

Hi @Czaki, the Cmd+W is a mac only thing as far as I know.

The quit application one, does work for all systems

[Czaki]

it is for all system

There was a bug in the past that removed shortcut, but on main it is for all os.

[psobolewskiPhD]

Command-W works nice! Huge improvement!
And yea, I think Control-W is also used on Windows to close stuff like tabs, so I'd make that one work on all systems to close the dialog.
I used Command-Q, which worked, but I was somewhat surprised it just straight quit, without the usual warning/confirmation.

I think it would be best to have that confirmation/dialog for consistency, so users arn't shocked like I was and lose data. But frankly, we could drop that -- it's not odd to have to close the dialog before quitting application, I don't think.

[Czaki]

I used Command-Q, which worked, but I was somewhat surprised it just straight quit, without the usual warning/confirmation.

Command+W/Q do not ask for confirmation if there is no layer.

[psobolewskiPhD]

running napari from terminal and then Command-Q I get asked to confirm.

[Czaki]

running napari from terminal and then Command-Q I get asked to confirm.

You are right, for command+Q should always be a confirmation if not disabled in settings...

[jaimergp]

Isn't the usual Quit shortcut Alt+F4 in Windows? Or is that no longer recommended?

[Czaki]

Isn't the usual Quit shortcut Alt+F4 in Windows? Or is that no longer recommended?

Alt + F4 is system shoutcut, not application one.

[goanpeca]

Hello team, thanks for testing and comments, will leave then the shortcut for all systems, and check about the confirmation dialog :)

Cheers!

[goanpeca]

@Czaki and @psobolewskiPhD I have added a fix to emulate the current behavior of Cmd+Q of napari.

Could you please check :) ?

[goanpeca]

Could you check this one @dalthviz ? Thanks !

[dalthviz]

Gave this a check on Windows and this is what I'm seeing:

Confirm config + number of layers\Shortcut	Ctrl+W	Ctrl+Q
Confirm dialog enabled without layers
Confirm dialog enabled with one layer
Confirm dialog disabled without layers
Confirm dialog disabled with one layer

So I would say that this is fixing the mentioned issues 👍

However, something that maybe is worthy to carefully think about (probably related with napari/napari#6993) is that since actions (install/uninstall) continue running even after the dialog closes, you can end up seeing a message like:

WARNING: QProcess: Destroyed while process ("C:\\Users\\dalth\\anaconda3\\envs\\napari-dev\\python.exe") is still running.

By triggering the shortcuts two consecutive times or even by executing the Ctrl+Q only once if the confirmation dialog is disabled when you previously started an install or uninstall plugin action from the plugin dialog

[jaimergp]

By triggering the shortcuts two consecutive times or even by executing the Ctrl+Q only once if the confirmation dialog is disabled when you previously started an install or uninstall plugin action from the plugin dialog

Yes I guess we will need #56 and napari/napari#6993 for this. Tracking issue is #53.

[goanpeca]

Awesome, so this looks like it is good to go. Should I merge? or should someone besides the author do it :) ?

[jaimergp]

We can merge but we should also investigate that numpy 2 breakage.