feat(desktop): add a keyboard shortcuts help window (#392) - #656
Merged
Merged
Conversation
Ctrl+/ (or "Show all shortcuts" on the Keyboard shortcuts settings card) opens a window listing every shortcut Linthra answers, grouped, with the combination each one is bound to right now. It has no list of its own. Rows come from the registry #391 added and chords from the live binding map the dispatcher installs, so a remap shows up in the window without anything being kept in step by hand, live, while it is open, and so does a reset. A test holds every row to that map rather than to a written-out list, because a second hard-coded table is the exact failure this is meant to avoid. Grouping is part of the registry. ShortcutGroup is a field on the action, so "which heading does this go under" cannot drift from the action either. Headings follow the enum's declaration order and rows follow the registry's, so the window is deterministic and does not depend on map iteration, and a group nothing is filed under is not drawn as an empty heading. Playback holds play/pause, next and previous; Navigation holds queue and Now Playing; Library holds search and the Library tab, since what search searches is the library; Help holds the window itself. The chord goes through the registry like any other action, which makes it listed, remappable, and impossible to bind something else over. Shift+/ (the "?" people also reach for) is not offered: Shift plus a printable key is a capital letter, which ShortcutBinding refuses outright. Pressing it again while the window is open does nothing rather than stacking a second copy, which is worth guarding because the window lists its own chord. Ctrl+F is shown under search's binding as "or Ctrl + F" rather than on a row of its own, since it is a second way to press the same shortcut and not a second shortcut to learn. Keyboard and focus: Escape closes it through the DismissIntent every modal route already answers, rather than a second path to the same pop. The focus stop that opens with it sits inside the scroll view, which is what makes Page Up/Down and Ctrl+arrow scroll the list, since Flutter's ScrollAction looks for a Scrollable above whatever holds the keyboard. Tab moves on to Close. Closing hands the keyboard back to the control that opened it when there is one to name (the settings button passes its own node, because Flutter never focuses a button that was clicked), and otherwise to whatever held it when the chord fired. Both halves of a row wrap, so a long or translated action name grows the row taller instead of pushing the chord off the window. Nothing on the phone changes: the entry point is on the desktop-only settings card, and the one line pointing at that card is left out where the card does not exist. The window itself is not gated on the platform, the same rule the shortcuts follow, so a tablet with a keyboard case gets it for free. docs/linux-desktop.md updated.
TheZupZup
marked this pull request as ready for review
September 20, 2026 01:07
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
Repository integrity reviewCLEAN Previously reported repository-integrity findings are resolved. |
Owner
Author
|
Gradle could not resolve It died 32 seconds in, during dependency resolution, before anything was compiled. Why it is not mine: this branch changes ten files, all Dart and Markdown, with no Gradle, Android, Kotlin or pubspec changes. The same check passed on the previous head There is no fix to port. Nothing upstream addresses a one-off Maven 403, and adding retries would mean editing I tried to re-run the failed job and the API returns 403 for Generated by Claude Code |
…ation Run 35480575826 failed `Validate simulated unlocked state` on this branch with Maven Central answering "Received status code 403 from server: Forbidden" for kotlin-gradle-plugins-bom. It died 32 seconds in, during Gradle dependency resolution, before a line was compiled. That was not this branch's doing. The diff was Dart and Markdown only, with no Gradle, Android, Kotlin or pubspec changes. The `locked` row resolved the same artifacts on the other runner and built fine, `Build debug APK` did the same on that same commit, and every other run of this workflow that day passed. The cause is in the workflow itself. The matrix runs `locked` and `unlocked` in parallel, so two runners ask Maven Central for the same Kotlin plugin artifacts within seconds of each other, and one gets throttled. Nothing about the change under test decides which. So the build step retries, three attempts with a widening pause, following the prefetch retry in flatpak-build.yml and for the same reason: one bad response from a source host should not fail a whole run. The retry is deliberately narrow. It fires only when the output carries Flutter's "Gradle threw an error while downloading artifacts from the network." which is printed for this class of failure and nothing else. A compile error fails on the first attempt, at its own speed, with its own message, rather than being run three times and reported late. The happy path costs nothing. Gradle's own org.gradle.internal.repository.max.retries would not have helped: it retries 5xx and transport errors, and a throttle answered as 403 is a client error it treats as final. The same hazard exists in android-debug-apk.yml, which builds an APK the same way. It has not failed this way, so it is left alone rather than changed on spec. Retry logic exercised on all four paths: success first time, a compile error failing once without retrying, a network failure retried three times and then reported, and a network failure that passes on the second attempt.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #392
Ctrl+/, or Show all shortcuts on the Keyboard shortcuts settings card, opens a window listing every shortcut Linthra answers, grouped, with the combination each one is bound to right now.It has no list of its own
Rows come from the registry #391 added and chords from the live binding map the dispatcher installs. A remap shows up in the window without anything being kept in step by hand, live, while it is open, and so does a reset. A test holds every row to that map rather than to a written-out list, because a second hard-coded table is the exact failure this is meant to avoid.
Grouping is part of the registry
ShortcutGroupis a field on the action, so "which heading does this go under" cannot drift from the action either. Headings follow the enum's declaration order and rows follow the registry's, so the window is deterministic and does not depend on map iteration. A group nothing is filed under is not drawn as an empty heading.Ctrl+Space,Ctrl+→,Ctrl+←Ctrl+U,Ctrl+PCtrl+K(alsoCtrl+F),Ctrl+LCtrl+/Search sits under Library rather than Navigation because what it searches is the library.
Ctrl+Fis shown under search's binding as "or Ctrl + F" rather than on a row of its own, since it is a second way to press the same shortcut and not a second shortcut to learn.The chord goes through the registry like any other action
Ctrl+/is aShortcutAction, which makes it listed, remappable, and impossible to bind something else over.Shift+/(the "?" people also reach for) is not offered: Shift plus a printable key is a capital letter, whichShortcutBindingrefuses outright. Pressing it again while the window is open does nothing rather than stacking a second copy, which is worth guarding because the window lists its own chord.Keyboard and focus
Escape closes it through the
DismissIntentevery modal route already answers, rather than a second path to the same pop. The focus stop that opens with it sits inside the scroll view, which is what makes Page Up/Down andCtrl+arrow scroll the list: Flutter'sScrollActionlooks for aScrollableabove whatever holds the keyboard, so a stop outside it would leave those keys doing nothing. Tab moves on to Close. Closing hands the keyboard back to the control that opened it when there is one to name (the settings button passes its own node, because Flutter never focuses a button that was clicked), and otherwise to whatever held it when the chord fired.Nothing on the phone changes
The entry point is on the desktop-only settings card, and the one line pointing at that card is left out where the card does not exist. The window itself is not gated on the platform, the same rule the shortcuts themselves follow, so a tablet with a keyboard case gets it for free.
Both halves of a row wrap, so a long or translated action name grows the row taller instead of pushing the chord off the window.
Tests
test/features/help/keyboard_shortcuts_help_test.dartcovers the defaults appearing, a stored override appearing instead, a remap and a reset updating the window while it is open, deterministic grouping, every row matching the live map, Escape, Tab, scrolling, focus restoration from both entry points, a vanished opener, and narrow windows.test/app/shortcuts/shortcut_action_test.dartcovers the grouping being total and deterministic and the new action's default.test/app/shortcuts/linthra_shortcuts_test.dartcovers the chord dispatching, not stacking, and opening on a remap.test/features/settings/desktop/keyboard_shortcuts_section_test.dartcovers the settings entry point.dart format,flutter analyzeandflutter testall clean.docs/linux-desktop.mdupdated.