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

Fix browser-like DbTab experience with macOS and Windows #4305

Conversation

varjolintu
Copy link
Member

@varjolintu varjolintu commented Feb 8, 2020

Type of change

  • ✅ Bug fix (non-breaking change which fixes an issue)

Description and Context

Changes the browser-type tab switching from ⌥Alt+num to the standard ⌘Command+num with macOS.

Also changes using 0 to 9 with all OS's. 9 always goes to the last tab, which is also a standard behavior with browsers.

Checklist:

  • ✅ I have read the CONTRIBUTING document. [REQUIRED]
  • ✅ My code follows the code style of this project. [REQUIRED]
  • ✅ All new and existing tests passed. [REQUIRED]
  • ✅ I have compiled and verified my code with -DWITH_ASAN=ON. [REQUIRED]

@JulianVolodia
Copy link
Contributor

JulianVolodia commented Feb 8, 2020

👎

@varjolintu as we were talking the 0 should be removed and 9 should go to last.

You said that browsers on MacOS have no behaviour on 0.

@varjolintu
Copy link
Member Author

@JulianVolodia That is now fixed.

JulianVolodia added a commit to JulianVolodia/keepassxc that referenced this pull request Feb 9, 2020
This is follow up of:
 PR keepassxreboot#4063 Enable browser-like DbTab experience (Alt + Nums)
 PR keepassxreboot#4305 Fix browser-like DbTab experience with macOS
This commit introduces desired behaviour across platforms.
 On MacOS only Command key + Nums combination should be used. (on MacOS, ⌘Command is QT::CTRL)
 On Linux and Windows there are combinations with Ctrl and (Left) Alt. (code could be reused)

This is not breaking for various keyboard layout organization on MacOS because of @varjolintu fixes,
nor breaking on Linux or Windows because they use AltGr (Right Alt) as modkey.
JulianVolodia added a commit to JulianVolodia/keepassxc that referenced this pull request Feb 9, 2020
This is follow up of:
 PR keepassxreboot#4063 Enable browser-like DbTab experience (Alt + Nums)
 PR keepassxreboot#4305 Fix browser-like DbTab experience with macOS
This commit introduces desired behaviour across platforms.
 On MacOS only Command key + Nums combination should be used. (on MacOS, ⌘Command is QT::CTRL)
 On Linux and Windows there are combinations with Ctrl and (Left) Alt. (code could be reused)

This is not breaking for various keyboard layout organization on MacOS because of @varjolintu fixes,
nor breaking on Linux or Windows because they use AltGr (Right Alt) as modkey.
@JulianVolodia
Copy link
Contributor

JulianVolodia commented Feb 9, 2020

Hey, thanks @varjolintu . I introducted your fixes with code format typo fix and some other improvements to main change which I will be pleased to be reviewed by You at #4306 PR.

So, if You @varjolintu and You @droidmonkey have 5 minutes please have a look at #4306
Best regards.

@varjolintu
Copy link
Member Author

@JulianVolodia I could've done any necessary fixes in this PR. Also, your new PR seems to break things.

@droidmonkey
Copy link
Member

I'm just keeping this PR

@JulianVolodia
Copy link
Contributor

@droidmonkey OK, but what is wrong with the other one? Previous change was first step, and #4306 completes it and added this PR also.

@droidmonkey
Copy link
Member

droidmonkey commented Feb 9, 2020

One PR per change. It makes no sense, and causes confusion, to work across multiple PR's. If you see something that needs to be fixed, make a comment on this PR.

@droidmonkey droidmonkey changed the title Fix browser-like DbTab experience with macOS Fix browser-like DbTab experience with macOS and Windows Feb 9, 2020
@JulianVolodia
Copy link
Contributor

@droidmonkey this PR is nice.
This PR need to be merged to make MacOS not breaking.

But rather than making big arc I wanted to introduce full enhancement of browser-like behaviour across MacOS, Linux and Windows. Enhancement, for your previous change, which was builded on stub of mine #4063 . Maybe I am not clear sorry @varjolintu and @droidmonkey

@droidmonkey droidmonkey force-pushed the hotfix/browser_like_dbtab_macos branch from 9fe9b27 to 61d9722 Compare February 9, 2020 14:50
@droidmonkey
Copy link
Member

I have streamlined the code.

* macOS and Windows browsers do not use `Alt+#` to change tabs. Windows uses `Ctrl` and macOS uses `Command`. Linux uses `Alt`.
* Remove shortcut for `Key+0` and assign `Key+9` as last tab selection
* Streamline tab selection code in MainWindow
@droidmonkey droidmonkey force-pushed the hotfix/browser_like_dbtab_macos branch from 61d9722 to b71279c Compare February 9, 2020 14:51
@JulianVolodia
Copy link
Contributor

JulianVolodia commented Feb 9, 2020

I have streamlined the code.

Hey, I checked. Now only ctrl on win and lAlt on linux.
If you check on Windows - OK. I could test on all platforms after 02/16/2020.

On Linux I see that Alt is used on Chromium, Firefox and Thunderbird, and Ctrl is used on Chrome.
From @varjolintu I know what is on MacOS - Qt::Ctrl aka Command key.
About Windows from 'beginners hack sites' that same as described above.

I don't know what to do best, binding 2 or 1 varying on platform will be hard to read when done in inline style.

I'm out of ideas, @varjolintu and @droidmonkey you will decide the best.

@droidmonkey
Copy link
Member

droidmonkey commented Feb 9, 2020

I tested Edge (Chrome) and Firefox on Windows -> Alt+# does nothing, Ctrl+# changes tabs

I tested Firefox and Nautilus (file browser) on Ubuntu Linux -> Alt+# changes tabs, Ctrl+# does nothing

I tested Firefox and Edge on macOS -> Alt+# does nothing, Ctrl+# does nothing, Cmd+# changes tabs

In Qt, Qt::CTRL auto-changes to mean macOS Command key.

@droidmonkey droidmonkey merged commit f227a2d into keepassxreboot:develop Feb 9, 2020
@varjolintu varjolintu deleted the hotfix/browser_like_dbtab_macos branch February 9, 2020 16:12
@JulianVolodia
Copy link
Contributor

OK. Thanks for testing, fixing that and merging. That will be great to have another great feature introducted by you.

As I know, Edge using Chromium, but is not equal to Chrome. Chrome on Linux have both Ctrl+# and Alt+#. I have no access to Windows now so don't know what is there.

And as we are talking about UI but UX there could be differences - same thing with MIUI from Xiaomi and vanilla Android. Both are Android-like... but they are not equal. Cheers.

droidmonkey added a commit that referenced this pull request Jul 7, 2020
Added

- Custom Light and Dark themes [#4110, #4769, #4791, #4796, #4892, #4915]
- Compact mode to use classic Group and Entry line height [#4910]
- View menu to quickly switch themes, compact mode, and toggle UI elements [#4910]
- Search for groups and scope search to matched groups [#4705]
- Save Database Backup feature [#4550]
- Sort entries by "natural order" and move lines up/down [#4357]
- Option to launch KeePassXC on system startup/login [#4675]
- Caps Lock warning on password input fields [#3646]
- Add "Size" column to entry view [#4588]
- Browser-like tab experience using Ctrl+[Num] (Alt+[Num] on Linux) [#4063, #4305]
- Password Generator: Define additional characters to choose from [#3876]
- Reports: Database password health check (offline) [#3993]
- Reports: HIBP online service to check for breached passwords [#4438]
- Auto-Type: DateTime placeholders [#4409]
- Browser: Show group name in results sent to browser extension [#4111]
- Browser: Ability to define a custom browser location (macOS and Linux only) [#4148]
- Browser: Ability to change root group UUID and inline edit connection ID [#4315, #4591]
- CLI: `db-info` command [#4231]
- CLI: Use wl-clipboard if xclip is not available (Linux) [#4323]
- CLI: Incorporate xclip into snap builds [#4697]
- SSH Agent: Key file path env substitution, SSH_AUTH_SOCK override, and connection test [#3769, #3801, #4545]
- SSH Agent: Context menu actions to add/remove keys [#4290]

Changed

- Complete replacement of default database icons [#4699]
- Complete replacement of application icons [#4066, #4161, #4203, #4411]
- Complete rewrite of documentation and manpages using Asciidoctor [#4937]
- Complete refactor of config files; separate between local and roaming [#4665]
- Complete refactor of browser integration and proxy code [#4680]
- Complete refactor of hardware key integration (YubiKey and OnlyKey) [#4584, #4843]
- Significantly improve performance when saving and opening databases [#4309, #4833]
- Remove read-only detection for database files [#4508]
- Overhaul of password fields and password generator [#4367]
- Replace instances of "Master Key" with "Database Credentials" [#4929]
- Change settings checkboxes to positive phrasing for consistency [#4715]
- Improve UX of using entry actions (focus fix) [#3893]
- Set expiration time to Now when enabling entry expiration [#4406]
- Always show "New Entry" in context menu [#4617]
- Issue warning before adding large attachments [#4651]
- Improve importing OPVault [#4630]
- Improve AutoOpen capability [#3901, #4752]
- Check for updates every 7 days even while still running [#4752]
- Improve Windows installer UI/UX [#4675]
- Improve config file handling of portable distribution [#4131, #4752]
- macOS: Hide dock icon when application is hidden to tray [#4782]
- Browser: Use unlock dialog to improve UX of opening a locked database [#3698]
- Browser: Improve database and entry settings experience [#4392, #4591]
- Browser: Improve confirm access dialog [#2143, #4660]
- KeeShare: Improve monitoring file changes of shares [#4720]
- CLI: Rename `create` command to `db-create` [#4231]
- CLI: Cleanup `db-create` options (`--set-key-file` and `--set-password`) [#4313]
- CLI: Use stderr for help text and password prompts [#4086, #4623]
- FdoSecrets: Display existing secret service process [#4128]

Fixed

- Fix changing focus around the main window using tab key [#4641]
- Fix search field clearing while still using the application [#4368]
- Improve search help widget displaying on macOS and Linux [#4236]
- Return keyboard focus after editing an entry [#4287]
- Reset database path after failed "Save As" [#4526]
- Use SHA256 Digest for Windows code signing [#4129]
- Improve handling of ccache when building [#4104, #4335]
- macOS: Properly re-hide application window after browser integration and Auto-Type usage [#4909]
- Auto-Type: Fix crash when performing on new entry [#4132]
- Browser: Send legacy HTTP settings to recycle bin [#4589]
- Browser: Fix merging browser keys [#4685]
- CLI: Fix encoding when exporting database [#3921]
- SSH Agent: Improve reliability and underlying code [#3833, #4256, #4549, #4595]
- FdoSecrets: Fix crash when editing settings before service is enabled [#4332]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants