fix(shortcuts): clear modifier-hint badges when main consumes a Cmd/Ctrl chord#1412
Open
brennanb2025 wants to merge 3 commits intomainfrom
Open
fix(shortcuts): clear modifier-hint badges when main consumes a Cmd/Ctrl chord#1412brennanb2025 wants to merge 3 commits intomainfrom
brennanb2025 wants to merge 3 commits intomainfrom
Conversation
…trl chord Holding Cmd over a terminal pane and pressing +/- left the Cmd+N number badges stuck on worktrees even after releasing Cmd. Root cause: main's before-input-event preventDefault()s the zoom chord, so the '-'/'+' keydown never reaches the renderer and useModifierHint's "other key while modifier held" branch can't self-clear. The subsequent Cmd keyup is also frequently dropped on macOS after a prevented chord. Instead of scattering dispatchClearModifierHints() calls across every forwarded-IPC handler (fragile; one gap already existed in the Cmd+N early-return path), introduce a single 'ui:shortcutConsumed' IPC. Main emits it once per intercepted chord (before-input-event, zoom-changed, native menu accelerators). One renderer subscriber clears the overlay. All 9 previously scattered dispatches are now redundant and removed. Tests updated to verify the single-subscriber architecture, plus a regression guard that individual handlers don't re-introduce the old scattered pattern. Co-authored-by: Orca <help@stably.ai>
… forwards Without this, the stuck-badge regression the PR fixes still reproduces when focus is inside a webview guest: browser-guest-ui.ts forwards 5+ shortcut IPCs (worktreeHistoryNavigate, toggleWorktreePalette, openQuickOpen, openNewWorkspace, jumpToWorktreeIndex, plus the browser-specific Cmd+T/W/L/R/F, Cmd+Shift+B/[/], Cmd+Alt+[/], Ctrl+PageUp/Down) but never called emitConsumedShortcut. Consolidate emit via an IIFE-thunk so every matched branch emits once, before the action send, honoring the main->renderer ordering contract. Also tighten createMainWindow.ts zoom-changed ordering and update the JSDoc on emitConsumedShortcut to accurately describe trigger conditions and FIFO delivery. Co-authored-by: Orca <help@stably.ai>
…edShortcut Every forwarded browser-guest chord now sends ui:shortcutConsumed immediately before the action IPC, so the 9-call sequence is now 18 calls alternating shortcutConsumed/action. Co-authored-by: Orca <help@stably.ai>
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
1 participant
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.
Summary
Holding Cmd over a terminal pane and pressing
+/-left the Cmd+N worktree number badges stuck on-screen even after Cmd was released. Same shape applies to any main-intercepted chord — the renderer'suseModifierHinthook never saw the chord's non-modifier keydown, and macOS frequently drops the subsequent Cmd keyup after a prevented chord.Approach
Rather than scatter
dispatchClearModifierHints()across every main-forwarded IPC handler (the pre-existing pattern, which already had a gap in theCmd+Nearly-return branch and was fragile-by-design), introduce a single authoritative signal:ui:shortcutConsumed— main emits once per intercepted chord.createMainWindow.tsbefore-input-eventafterpreventDefault()(covers all 8 action types in the allowlist in one place),zoom-changedfallback, allCmdOrCtrl-accelerated menu callbacks (onOpenSettings,onZoomIn/Out/Reset,onToggleLeft/RightSidebar, Export as PDF).useIpcEventsdispatchesCLEAR_MODIFIER_HINTS_EVENT.dispatchClearModifierHints()calls from forwarded-IPC handlers — they're now redundant.Net result: main is the sole authority on "I consumed a chord"; the renderer reacts in one place. Any future main-intercepted shortcut gets hint-clearing automatically.
Files
src/main/window/emit-consumed-shortcut.ts— 7-line helper documenting why the signal exists.src/main/window/createMainWindow.ts— emit afterpreventDefault()and inzoom-changed.src/main/index.ts— emit in menu-accelerator callbacks.src/main/menu/register-app-menu.ts— emit in Export as PDF click.src/preload/index.ts+src/preload/api-types.ts— exposeonShortcutConsumed.src/renderer/src/hooks/useIpcEvents.ts— single subscriber; 9 scattered dispatches removed.Tests
useIpcEvents.test.ts"shortcut hint clearing" suite to verify the new single-subscriber architecture (positive:onShortcutConsumeddispatches clear; regression guard: individual handlers no longer dispatch).createMainWindow.test.tssend-count/index assertions for the pairedui:shortcutConsumedsend.tc:web+tc:nodeclean. Relevant test files pass (38/38 across the four affected suites).Manual test plan
-while holding Cmd — font zooms out, badges still visible (correct; Cmd still held).+,0,,,J,P,1–9,Alt+Arrow, menu-clicked Settings/Zoom items. All clear correctly on Cmd release.Made with Orca 🐋