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

modified standalone keyset to include the quitApplication keybinding #3010

Merged
merged 2 commits into from
Mar 5, 2023

Conversation

oaken-source
Copy link

The default keyset on unix and windows does not include the keybinding for exiting the application. On macos this works as expected because the keybinding is included in the macos keyset. This pull request adds the appropriate keybinding for the other platforms as well.

@dstillman
Copy link
Member

I don't think we'd do Ctrl-Q on Windows. When in doubt we generally defer to Firefox, and Firefox uses Ctrl-Q on Linux but Ctrl-Shift-Q on Windows (which I think is more reasonable anyway given the closeness to Ctrl-W and the lack of any sort of confirmation prompt). So if we do this, I think we'd want to use accel,shift and either use that for Linux as well or change the attribute programmatically at startup.

(Chrome on Windows seems to be content with Alt-f + x.)

@oaken-source
Copy link
Author

that's a good catch, it's been a long time since I used windows. Ctrl+Shift+Q to quit seems unintuitive to me, considering that macos also uses a simple Command+Q. I think windows is the odd one out in that regard.

@dstillman
Copy link
Member

dstillman commented Mar 2, 2023

Cmd-Q has been universal on macOS for decades (though Chrome does a weird non-standard two-press thing — somewhat reasonably, given how easy it is to trigger by accident in a tabbed app and how disruptive it can be), but Ctrl-Q just isn't a thing on Windows. There's Alt-F4 or Alt-F + X, but that's it. So I think Firefox's Ctrl-Shift-Q is just a compromise to have something vaguely similar to Firefox on other platforms without adding a non-standard shortcut that's very easy to trigger accidentally.

@oaken-source
Copy link
Author

understood. I'll try to find a configuration that works acceptably on all platforms, but it might take me a while to set up the build tools and environment on windows

@dstillman
Copy link
Member

Note that you don't need to build on Windows, and I wouldn't recommend it. You can build Windows (without packaging) anywhere with scripts/dir_build -p w (after ./fetch_xulrunner.sh -p w) and just open the built app in staging on Windows.

Thanks for working on this!

@qqobb
Copy link
Contributor

qqobb commented Mar 2, 2023

@oaken-source: On Windows, I would recommend using Alt-F4 for closing applications, since this works universally across programs. While shortcuts like Ctrl-W work in some apps like File Explorer (Win-E), Alt-F4 also works, e.g., in the Settings app.

You could perhaps remap the Alt-F4 shortcut to Ctrl-Q using Microsoft PowerToys, which provide a Keyboard Manager utility. But I personally wouldn't recommend it for the reasons given by dstillman.

I've been using Zotero.Utilities.Internal.quit() within the Zutilo add-on to create shortcuts for quitting/restarting Zotero. It's not in the repo yet, but I'll create a PR soon, see wshanks/Zutilo#249.

@qqobb
Copy link
Contributor

qqobb commented Mar 2, 2023

Alt-f+x would be a welcome addition to Zotero's default shortcuts.

@dstillman
Copy link
Member

dstillman commented Mar 2, 2023

Alt-F4 already works in Zotero.

Alt-F + X isn't really a shortcut — it's just selecting the File menu and then an access key for Exit. The first part already works. It looks like we're just missing the 'x' access key on Exit at the moment, so 'e' triggers it instead now. So I think we should just add 'x' and then forget about Ctrl-Shift-Q on Windows (meaning that we'll want to assign Ctrl-Q programmatically just for Linux).

…ied windows keyset to use the correct keybinding
@oaken-source
Copy link
Author

I pushed a new version that (should):

a) modify the unix menu to bind the configured key (q in en_US locale) to quit and add a Ctrl+(key) keybinding
b) modify the windows menu to bind configured key (x in en_US locale) to exit

It's pretty straightforward, and Alt+f, x should now work on windows. but I haven't tested that yet.

@dstillman dstillman merged commit 8ac77bf into zotero:master Mar 5, 2023
@dstillman
Copy link
Member

Windows needs an accesskey, not a shortcut key. Fixed and merged. Thanks!

dstillman added a commit that referenced this pull request Mar 9, 2023
- Add 'x' accesskey for "Exit" on Windows
- Show "Quit" instead of "Exit" on Linux
- Don't show Ctrl-Q for shortcut key on Windows

But Ctrl-Q still works on Windows when it shouldn't because of the Mac
keybinding, which still gets registered even if disabled
programmatically in platformKeys.js.

fx102 follow-up to #3010
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants