Fix Alt key handling for menubar toggle and enable Save button on non-data changes#13058
Fix Alt key handling for menubar toggle and enable Save button on non-data changes#13058xboxones1 wants to merge 2 commits intokeepassxreboot:developfrom
Conversation
droidmonkey
force-pushed
the
fix/minnor-fix
branch
from
b05c73a to
6d23bb2
Compare
There was a problem hiding this comment.
Pull request overview
This PR refines two GUI behaviors in KeePassXC’s main window: (1) preventing unintended menubar toggling by only reacting to a standalone Alt press/release, and (2) enabling the Save action for “non-data” changes (e.g., UI state changes) without marking the database as modified, addressing #9751.
Changes:
- Track whether Alt was pressed alone and only toggle the hidden menubar on an Alt-only press/release sequence.
- Enable the Database → Save action when the database has non-data changes and auto-saving those changes is disabled.
- Stop marking the database “modified” for non-data changes (instead notifying UI via signals).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/gui/MainWindow.h |
Adds state to track Alt-only key usage for menubar toggle behavior. |
src/gui/MainWindow.cpp |
Updates Save enablement logic to consider non-data changes; refines menubar toggle key handling. |
src/gui/DatabaseWidget.cpp |
Adjusts handling of non-data change events to avoid marking the DB modified. |
| // Force mark the database modified if we are not auto-saving non-data changes | ||
| if (!config()->get(Config::AutoSaveNonDataChanges).toBool()) { | ||
| m_db->markAsModified(); | ||
| emit databaseNonDataChanged(); | ||
| } |
There was a problem hiding this comment.
DatabaseWidget already relays Database::databaseNonDataChanged to its own databaseNonDataChanged signal in connectDatabaseSignals(). Emitting databaseNonDataChanged() again here will fire the signal twice for the same underlying change (and therefore call MainWindow::updateMenuActionState() twice via the action multiplexer). Consider removing this extra emit, or alternatively stop relaying the database signal and emit conditionally only from this slot.
Also, the comment above no longer matches the behavior (it mentions marking the DB modified, but the code now only emits a signal).
Menubar visibility previously reacted to any key combination involving Alt (e.g. Shift+Alt for keyboard layout switch or Alt+Arrow for moving entries).
Now it only toggles when Alt is pressed and released alone, preventing unintended menu activation during shortcuts.
Testing strategy
Manually
Type of change