Repository navigation
refactor(shortcuts): unify shortcut presentation - #1835
Merged
Merged
Conversation
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
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.
Problem
Shortcut hints were rendered through overlapping implementations:
KeyboardShortcut, the settings-onlyKeyBadge, raw sidebar text, and bespoke modal CSS. Their parsers, icon treatment, key widths, typography, and spacing had drifted, so letter keys did not occupy the same visual slot as SVG glyphs and Settings shortcuts behaved differently from other surfaces. The sidebarshortcutfield also carried relative-time metadata, hiding which values represented actual keyboard chords.Solution
Make
KeyboardShortcutthe canonical visual renderer across Settings, navigation rows, Quick Actions, API Calls, and the component-issue modal. It now supports the icon-backed keys and alternative chord parsing previously owned byKeyBadge; single-character letters use an icon-sized slot, shortcut text is reduced by one pixel at each pill size, and every pill size has 2px more left than right padding for optical balance.Remove
KeyBadgeand its dedicated tests/CSS, remove the redundantworkStationpresentation variant, and move sidebar age metadata into a distincttrailingLabelfield. Widen the shared panel-header subtitle toReactNodeso shortcut pills can be rendered without local wrappers. Update the cloud fork parent fixture to use the metadata field and assert that provenance text never renders as keyboard keys.Potential risks
Centralizing parsing can change the appearance of uncommon shortcut encodings; coverage includes named modifiers, packed macOS chords, function keys, special keys, arrows, empty values, and alternative chords. The
PanelHeadersubtitle type broadens an internal rendering contract but remains source-compatible with existing strings. No persistence, configuration format, network, IPC, or wire behavior changes.The padding and typography changes are user-visible. Automated markup assertions cover their tokens, but no post-change desktop screenshots were captured because Computer Use was not authorized; theme and live-layout appearance remain the primary unverified path.
Verification
pnpm test -- src/components/KeyboardShortcut/index.test.ts src/components/KeyboardShortcut/index.bindings.test.ts src/modules/MainApp/Settings/sections/ShortcutsSection/ShortcutRecorder.test.ts src/scaffold/NavigationSidebar/components/NavigationMenu/NavigationMenu/NavigationMenuRow.test.ts src/scaffold/NavigationSidebar/connectors/WorkstationSidebarConnector/cloudSessionsSection.rowItemBuilder.test.ts src/scaffold/NavigationSidebar/connectors/useSessionMenuItems/__tests__/menuItemBuilders.test.ts src/scaffold/NavigationSidebar/connectors/__tests__/workstationSidebarMenuItems.test.ts— 51 tests passed across 7 filespnpm test -- src/features/Org2Cloud/cloudSessionThreads.test.ts src/scaffold/NavigationSidebar/components/NavigationMenu/NavigationMenu/NavigationMenuRow.test.ts— 36 tests passed across 2 files after reproducing the CI failurepnpm lint:fileover all changed TypeScript/TSX files — passedpnpm typecheck:fast— passedgit diff --check— passedKeyBadgeimports or definitions and no production<kbd>outsideKeyboardShortcutAudit
Frontend UI audit: 2 fix, 2 keep with reason, 0 abstract. Report:
docs/frontend-ui-audit-2026-09-15/ShortcutPresentationConsolidation.md.Architecture review covered component ownership, public presentation types, import dependencies, dead-code removal, consumer migration, and regression boundaries. Rust, persistence, IPC/wire protocol, background lifecycle, and performance layers were skipped because this is a stateless frontend presentation refactor.