Repository navigation
feat(web): open Settings with ⌘, on macOS - #641
Draft
Andrey Markin (Mark-Life) wants to merge 18 commits into
Draft
Andrey Markin (Mark-Life) wants to merge 18 commits into
Andrey Markin (Mark-Life) wants to merge 18 commits into
Conversation
Cmd+Q did nothing on macOS because Electrobun role menu items get no default accelerator. Quit now needs a 1.2s hold or a double press and fires on release, with a centered hint; menu, Dock, and update quits stay direct and share the existing quit coordinator. Shortcuts follow VS Code: Cmd+Q / Linux Ctrl+Q quit (confirmed); Cmd+W, Win/Linux Ctrl+W and Ctrl+F4 close the focused dialog or tab through the tab X-button path and never close the window. Hide, Hide Others, and Minimize get their macOS accelerators. The confirmation rules live once in contracts (quitConfirmation); macOS feeds them CoreGraphics key state because menu events repeat without a repeat flag, Linux feeds them web key events. A web shortcut command table serves native commands and Win/Linux keys. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Cmd+W bumped a closeRequestKey state in Shell, re-rendering Shell, WorkspaceWorkbench and Workbench per press. A shell-owned closeRequestChannel now fires the request; Workbench gets it via an injected subscribeCloseRequest prop, so layout stays props-driven and the mount-time stale-key rule goes away. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Shell no longer calls useAppShortcuts. The AppShortcuts leaf runs the hook and renders QuitHintOverlay, so quit hint changes re-render only that leaf. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
matchShortcut now takes a lazy terminal check and calls it only after a chord matches. useAppShortcuts adds window keydown/keyup listeners only on platforms whose table binds web chords (windows, linux), so macOS desktop has none. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
close-item dismisses layers with an untrusted Escape. Spell out what each dismissible layer must do to keep closing on the shortcut. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Decision #3 named one function and its injection details. It now states the rule (types plus small dependency-free runtime) and points to the contracts spec for the list. README no longer says types-only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On AZERTY/Dvorak the KeyW/KeyQ positions type other letters, so Ctrl+Z closed tabs and double Ctrl+A in a Linux terminal quit the app. Letter chords now match event.key, falling back to code for non-Latin layouts. The quit hold releases on the keyup of the physical key that started it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A key pressed after Command-Q no longer completes the hold. The hold tracks the non-modifier keys down when the menu action fired, so it stays layout-independent. Key-repeat presses never add keys. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The tab strip shares data-group-id with its group root, so closest() from a focused tab returned the strip and missed the overlay; close-item then closed the chat. Match every element with the focused group id. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Assert QUIT_CONFIRMATION matches the documented 1200/500/40 ms values, and check that a hold released before the threshold never quits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Shell, web, and desktop specs said contracts is types-only, but both import createQuitConfirmation. Shell always mounts AppShortcuts; the hook is inert without the bridge. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With Command up, a last key-down newer than the last modifier change means the Quit item was picked from the menu by keyboard, not by Command-Q. A late-handled tap still arms: its Command release is newer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The busy-close step waits for a computed marker printed by the child shell. The terminal Ctrl+W step checks the shell erased a word. Use pressPlatformShortcut for the new-workspace chord. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Split CoreGraphics reads from a pure decodeKeyState and test the modifier keycode range, Command flag, and event-age ordering. Test createShortcutsBridge pub/sub directly. Type the e2e bridge stub as NativeShortcutsBridge so drift fails typecheck. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The macOS app menu gains "Settings…" (⌘,), forwarded to the web client as the `open-settings` native command. In a browser on a Mac the web client owns ⌘, itself. Both paths open Settings unless a modal or menu is open, and `openSettings()` with no section now reopens the last section shown. Closes JetBrains#623 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Settings reopens on the last section, but focus still landed on the first rail item, so two items looked selected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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.
Problem
On macOS, ⌘, does nothing. Every Mac app opens its settings with it, so in ThinkRail Settings opens only with the mouse. The gear also always opens on Providers, even when the user was last on another section.
Approach
⌘, opens Settings on macOS only. Windows and Linux get no Ctrl+, so terminals and the browser keep that key. Each runtime has exactly one owner for the key, the rule #609 set:
open-settingsnative command. If the window is minimized, it is restored first.Both paths do nothing while a modal, popover or menu is open. Otherwise Settings would open over the menu, and the menu would still be open after Settings closed. In the browser the key is still caught there, so Chrome's own settings page never opens.
openSettings()with no section now reopens the last section shown in this session, for both the gear and ⌘,. Links to a specific section (Templates, Terminal, Updates, Providers) still go there. When Settings opens, focus goes to the active section's button. Before, it went to Providers, so two items looked selected.Stacked on #609: the diff includes its commits until it merges. The last two commits are this change.
Changes
contracts:NativeCommandaddsopen-settings.desktop: Settings… (CommandOrControl+,) in the macOS app menu; restores a minimized window before forwarding the command.web: the ⌘, browser handler inuseGlobalHotkeys, matched on the typed,with a physical-key fallback for non-Latin layouts, like ⌘W/⌘Q. Includes the shared gate (openSettingsUnlessLayered), last-sectionopenSettings(), focus on the active section, and the gear tooltipSettings (⌘,)plusaria-keyshortcuts.UI-TERMINOLOGY.md.Screenshots
Browser at 1440×900, platform set to Mac. Flow: open Settings → Appearance → Esc → ⌘,.
Packaged desktop build (
bun run desktop:build), Settings opened by ⌘,:Related issues
Closes #623. Depends on #609.
Checklist
bun run lint,bun run typecheck,bun run test. The only failure was a timing-sensitive server test (review resolution rejects a commit past the newest-200 listCommits cap, 5.2s against a 5s timeout), which passes when rerun alone.bun run e2e477/477 onfadaf313.SPEC.md/ top-level specs updated to reflect any boundary, contract, or behavior changeChecked by hand on a packaged macOS build through System Events: the app menu shows
Settings… ,with Command only; ⌘, and the menu item open Settings; a minimized window comes back with Settings open on the last section. Not tested: Safari and Firefox (Playwright runs Chromium only), and Windows/Linux desktop builds.🤖 Generated with Claude Code