Skip to content

fix: shortcuts#2104

Merged
jschwxrz merged 16 commits into
mainfrom
fix-shortcuts
May 19, 2026
Merged

fix: shortcuts#2104
jschwxrz merged 16 commits into
mainfrom
fix-shortcuts

Conversation

@jschwxrz
Copy link
Copy Markdown
Collaborator

  • standardized shortcut display across the renderer, including keycap styling, tooltip badges, enter glyph handling and macOS modifier ordering
  • updated keyboard shortcut settings so reset appears to the left of clear when both actions are available
  • fixed the terminal drawer scripts settings button so it opens the project settings tab directly
  • changed Mod+T to create a new conversation
  • removed the direct theme-toggle global shortcut
  • added toggle theme as a searchable command palette action

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 18, 2026

Greptile Summary

This PR standardizes how keyboard shortcuts are displayed across the renderer by replacing the old ShortcutHint component with a new unified Shortcut/BoundShortcut pair that supports three variants (text, badge, keycaps). It also reassigns Mod+T from theme-toggling to new-conversation, promotes the theme toggle into the searchable command palette, and fixes the terminal drawer settings button to land on the settings tab directly.

  • New shortcut display layer: shortcut-format.ts centralizes key labeling, modifier ordering (macOS vs Windows/Linux), spoken ARIA labels, and optical-alignment helpers; shortcut.tsx consumes it with a single polymorphic component exported as Shortcut (raw hotkey) and BoundShortcut (resolved from settings key).
  • toggleAppTheme() extracted to theme-toggle.ts: performs an optimistic cache update against React Query, calls the RPC, and rolls back on error with invalidateQueries in the finally block.
  • Keyboard settings card: swap order so Reset (RotateCcw) appears left of Clear (X); switch the shortcut display button from formatForDisplay string to the new Shortcut keycaps variant.

Confidence Score: 5/5

Safe to merge — changes are a clean consolidation of shortcut display logic, intentional shortcut reassignment, and a well-guarded theme toggle migration to the command palette.

The refactoring is mechanical and well-tested: the new Shortcut/BoundShortcut components have unit tests for all formatting paths, the toggleAppTheme optimistic update correctly rolls back on error, and the Reset/Clear button swap in the keyboard settings card is logically correct. No regressions were found in the changed code paths.

No files require special attention.

Important Files Changed

Filename Overview
src/renderer/lib/ui/shortcut.tsx New unified shortcut display component; exports Shortcut (raw hotkey) and BoundShortcut (settings-key-bound); correctly handles null/undefined, ARIA, and three visual variants
src/renderer/lib/ui/shortcut-format.ts Pure formatting utilities for key labels, modifier ordering, optical alignment classes, and spoken descriptions; well-tested by the accompanying test file
src/renderer/lib/theme/theme-toggle.ts Extracts theme toggle to a standalone async function with optimistic React Query updates and rollback on error; staleTime of 5 min on fetchQuery is safe given the invalidateQueries in the finally block
src/renderer/features/settings/components/KeyboardSettingsCard.tsx Swaps Reset/Clear button order (Reset now left of Clear) and replaces formatForDisplay with describeHotkey; logic is correct
src/renderer/features/command-palette/command-palette-modal.tsx Replaces hand-rolled formatHotkey and inline kbd elements with the Shortcut component; dual-render (text vs badge) on selection state via CSS is intentional
src/shared/shortcuts.ts Removes toggleTheme shortcut entry and reassigns Mod+T to newConversation (was Mod+Shift+C)
src/renderer/lib/components/app-keyboard-shortcuts.tsx Removes the toggleTheme hotkey listener; no other shortcuts affected
src/renderer/features/tasks/terminals/terminal-drawer-sidebar.tsx Settings button now calls project.view.setProjectView('settings') before navigating, with a guard that disables the button when the project store is not mounted
src/renderer/lib/commands/app-commands.ts Adds app.toggleTheme command that calls toggleAppTheme(); no shortcut key assigned (command-palette only)

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Shortcut / BoundShortcut] --> B{variant?}
    B -->|text| C[Inline span, muted text]
    B -->|badge| D[Rounded badge, bg-background-secondary]
    B -->|keycaps| E[Individual Kbd elements with border/shadow]

    F[BoundShortcut] -->|reads settingsKey| G[useAppSettingsKey keyboard]
    G --> H[getEffectiveHotkey]
    H -->|Hotkey string or null| A

    I[shortcut-format.ts] --> J[getShortcutKeys - sort modifiers by platform order]
    J --> K[formatShortcutKey - visible glyph]
    J --> L[describeShortcut - spoken ARIA label]
    A --> I

    M[toggleAppTheme] --> N[fetchQuery theme meta]
    N --> O[getNextTheme]
    O --> P[Optimistic setQueryData]
    P --> Q[rpc.appSettings.update]
    Q -->|success| R[invalidateQueries - finally]
    Q -->|error| S[Rollback setQueryData]
    S --> R
Loading

Reviews (2): Last reviewed commit: "fix(tasks): guard script settings naviga..." | Re-trigger Greptile

Comment thread src/renderer/lib/theme/theme-toggle.ts Outdated
Comment thread src/renderer/features/tasks/terminals/terminal-drawer-sidebar.tsx
@jschwxrz
Copy link
Copy Markdown
Collaborator Author

@greptileai

@jschwxrz jschwxrz merged commit 5b6c600 into main May 19, 2026
1 check passed
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.

1 participant