Skip to content

Enable browser-like DbTab (Alt + Num) experience across platforms with fixes#4306

Closed
JulianVolodia wants to merge 5 commits intokeepassxreboot:developfrom
JulianVolodia:develop
Closed

Enable browser-like DbTab (Alt + Num) experience across platforms with fixes#4306
JulianVolodia wants to merge 5 commits intokeepassxreboot:developfrom
JulianVolodia:develop

Conversation

@JulianVolodia
Copy link
Copy Markdown
Contributor

Improvement of PR #4063 and apply of PR #4305 hotfix made by @varjolintu

Type of change

  • ✅ Bug fix (non-breaking change which fixes an issue)
  • ✅ Refactor (significant modification to existing code

Description and Context

Enable browser-like DbTab (Alt + Num) experience across platforms

This is follow up of:
PR #4063 Enable browser-like DbTab experience (Alt + Nums)
PR #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.

edge case over common case
Selecting last not use shortcut variable and is edge case here. This minimalize PR commit diff also.

Change Key_0 to Key_9
Fix browser-like DbTab experience with macOS
white space removal

Testing strategy

not tested

Checklist:

  • ✅ I have read the CONTRIBUTING document. [REQUIRED]
  • ✅ My code follows the code style of this project. [REQUIRED]

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.
Selecting last not use shortcut variable and is edge case here. This minimalize PR commit diff also.
new QShortcut(Qt::ALT + Qt::Key_0, this, SLOT(selectLastDatabaseTab()));

auto shortcut = new QShortcut(Qt::ALT + Qt::Key_1, this);
new QShortcut(Qt::CTRL + Qt::Key_9, this, SLOT(selectLastDatabaseTab()));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now you are keeping CTRL+9 shortcut alive even with WIndows systems. Why did you remove the previous ifdef?

@JulianVolodia
Copy link
Copy Markdown
Contributor Author

@droidmonkey could you rethink that last time?

If Chrome (on LInux) have it enabled and is most used browser maybe they know how to do it right way?

Ppl using KeePassXC on different platforms could be suprised that on Windows work something but on Linux not. Chrome is cross-platform and Edge is not equal to Chrome.
With same keyboard layouts (Windows and Linux have using actually the same keyboard) and typing when using some 'muscle memory' will could do the same thing on KeepassXC on all platforms when introducted shortcuts with CTRL and ALT on both Win and Linux, and MacOS have their CTRL at the same place on keyboard but called differently. That would be far less confusing.

Also, there is no problem with collisions on Linux and Windows now for Keepass, and Win Key (modkey 4 if I correctly recall) is used by default on AwesomeWM or some freaky WM out there, to manipulate desktops/views.

@droidmonkey
Copy link
Copy Markdown
Member

At this point I have already spent enough time on this feature and will not be addressing any further PR's for it. There are much more important things to work on for 2.6.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants