Unify desktop keyboard shortcuts and help / 统一桌面快捷键设置与帮助#4515
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c889b906e4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return SHORTCUT_DEFINITIONS.find((definition) => { | ||
| if (definition.action === action) return false; | ||
| return sameCombo(resolvedShortcutCombo(definition.action, platform), combo); |
There was a problem hiding this comment.
Include shortcut aliases when rejecting conflicts
When a user records the plus-key variant (Cmd/Ctrl+Shift+=, which arrives as key: "+") for another action, this only compares against each action's resolved primary combo and misses the textSize.increase aliases declared above. The settings page will accept that binding even though the docs promise conflicts are rejected, and pressing it will run both the chosen action and the text-size increase handler.
Useful? React with 👍 / 👎.
| if (isShortcutRecorderTarget(event.target)) return; | ||
| if (!definition.allowInEditable && isEditableTarget(event.target)) return; | ||
| if (!matchesShortcut(event, action, platform)) return; |
There was a problem hiding this comment.
Preserve global shortcuts while the composer is focused
Because the composer textarea is the normal focused element while using the app, this early return suppresses every advertised global shortcut whose definition does not opt into allowInEditable, including close tab, shell toggle, text-size changes, settings, and the ? help sheet. These were previously document-level handlers without an editable-target filter, and the guide still lists them as global shortcuts, so from the main composing flow Cmd/Ctrl+W or Cmd/Ctrl+ +/-/0 now silently do nothing.
Useful? React with 👍 / 👎.
| action: "settings.open", | ||
| section: "global", | ||
| labelKey: "shortcuts.action.settings", | ||
| descriptionKey: "shortcuts.desc.settings", | ||
| defaults: modCombo(","), |
There was a problem hiding this comment.
Keep the macOS menu accelerator in sync
On macOS the native File > Settings menu in desktop/menu.go still registers keys.CmdOrCtrl(",") and emits app:open-settings. If a user rebinds Open settings away from Cmd+, or assigns Cmd+, to another action after that, the old native accelerator still opens Settings outside this registry, so the Shortcuts page does not actually make this binding customizable on macOS and can allow one key to trigger two actions.
Useful? React with 👍 / 👎.
| aliases: { | ||
| darwin: [{ key: "+", meta: true, shift: true }], | ||
| windows: [{ key: "+", ctrl: true, shift: true }], | ||
| linux: [{ key: "+", ctrl: true, shift: true }], |
There was a problem hiding this comment.
Accept unshifted plus for text-size increase
This alias only matches key: "+" when Shift is also pressed, while the previous handler accepted e.key === "+" regardless of Shift. On keyboards that emit an unshifted plus, such as Ctrl/Cmd plus the numpad +, the event is { key: "+", shiftKey: false }, so it matches neither the = default nor this alias and the documented Ctrl/Cmd++ text-size shortcut stops working.
Useful? React with 👍 / 👎.
Remove the 'display: none' rule for .sidebar__new that was hiding the button. This rule was added assuming AppChrome component would handle the new chat button, but since we skipped esengine#4515 (AppChrome integration), the original sidebar button needs to remain visible.
- App.tsx: kept custom sidebar structure (rejected AppChrome from esengine#4515) - Composer.tsx: kept custom input layout and Send icon - ContextPanel.tsx: kept usage bar (replaced donut), kept formatCacheHitRate - context-panel-breakdown.test.ts: aligned with ContextPanel changes
- App.tsx: kept custom sidebar structure (rejected AppChrome from esengine#4515) - Composer.tsx: kept custom input layout and Send icon - ContextPanel.tsx: kept usage bar (replaced donut), kept formatCacheHitRate - context-panel-breakdown.test.ts: aligned with ContextPanel changes
- App.tsx: kept custom sidebar structure (rejected AppChrome from esengine#4515) - Composer.tsx: kept custom input layout and Send icon - ContextPanel.tsx: kept usage bar (replaced donut), kept formatCacheHitRate - context-panel-breakdown.test.ts: aligned with ContextPanel changes
Summary
keyboardShortcutsregistry instead of adding a second shortcuts library.?/topic-bar keyboard shortcuts sheet generated from the same registry, plus a skip-to-composer accessibility link.Related PRs
keyboardShortcutsmodule.Authorship note: @ttmouse is the primary author of the shortcut settings direction in #4202. @HUQIANTAO is the primary author of the shortcuts cheatsheet/a11y direction in #3125. @SivanCola contributed as a collaborator by integrating both directions on the latest
main-v2, resolving the source-of-truth conflict, adding tests/docs, and verifying the final state.Cache impact
Verification
pnpm --dir desktop/frontend exec tsx src/__tests__/keyboard-shortcuts.test.tspnpm --dir desktop/frontend check:csspnpm --dir desktop/frontend typecheckpnpm --dir desktop/frontend build