fix(hig): replace hand-rolled controls with native appkit and swiftui equivalents - #2104
Merged
datlechin merged 1 commit intoAug 13, 2026
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
datlechin
merged commit Aug 13, 2026
6d79728
into
worktree-single-window-connections
3 of 4 checks passed
datlechin
added a commit
that referenced
this pull request
Aug 14, 2026
datlechin
added a commit
that referenced
this pull request
Aug 15, 2026
* refactor(coordinator): give a window a registry of connection workspaces * feat(tabs): open editor tabs in one window instead of one window per tab * feat(connections): host every open connection in a single window * fix(tabs): draw the editor tab strip the way the system tab bar draws * fix(tabs): match the system tab bar capsule, hover fill and separator rules * fix(tabs): build the tab strip on glass, to the system tab bar's measured geometry * fix(tabs): keep glass off the tab strip track and fix the dark mode selected tab * fix(tabs): keep the connection open when its last tab closes * fix(tabs): open the tab a payload names when its connection is already open * fix(tabs): clear saved tabs when the user closes the last one * fix(tabs): restore every saved tab into the connection's one tab list * docs(plans): record the window to workspace scoping refactor * refactor(tabs): give a connection one tab list on the write side too * refactor(tabs): drop the window-group dimension from tab persistence * fix(tabs): give each connection its own undo history * fix(tabs): save and tear down every connection when its window closes * fix(connections): act on the connection the window is showing * fix(connections): close one connection, not the window that hosts them all * fix(coordinator): repoint the detail pane and toolbar on a workspace switch * docs(claude-md): correct the invariants that still describe one window per connection * docs(plans): record what the workspace scoping refactor has landed * test(tabs): assert the single-window menu contract * fix(coordinator): run a broadcast command only in the connection on screen * fix(connections): open into the window that already hosts the connection * fix(tabs): prompt for unsaved work in every connection the window closes * test(connections): cover the rule that decides which window hosts a connection * fix(sidebar): repaint the workspace rail when the window switches connection * fix(coordinator): hand over key-window state when the window switches connection * docs(plans): bring the refactor status up to date * fix(hig): replace hand-rolled controls with native appkit and swiftui equivalents (#2104) Claude-Session: https://claude.ai/code/session_01A3rb597qZtq4h5xZxwg43W * chore: regenerate the string catalog Claude-Session: https://claude.ai/code/session_01A3rb597qZtq4h5xZxwg43W * fix(tabs): show the tab strip as soon as a second tab opens * refactor(hig): rebuild the sidebar on NSOutlineView and fix the PR #2104 regressions * fix(import): lay out the failing statement inside the import error alert * refactor(sidebar): drop dead code and the duplicated search and alert plumbing * refactor(hig): give every alert one window resolver and one presentation path * refactor(sidebar): split the tree coordinator and derive its shape once * fix(connections): let the workspace rail follow the connection its window is showing * fix(connections): connect a workspace adopted into a window already on screen * chore(perf): trace where a workspace switch spends its time * chore(perf): record the switch trace at a level the unified log keeps * fix(coordinator): key the sidebar and inspector panes to the connection they show * fix(hig): drop an inspector control hint that only exists in an unreleased sdk * test(tabs): scope the bulk close tests to a connection's one tab list * fix(perf): stop the switch trace ending its signpost interval twice * fix(connections): key window lifecycle by connection, not by window * fix(connections): resolve a rail command's coordinator through the window hosting it
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.
Audit of the whole UI for hand-rolled controls, faked chrome and HIG violations, with the native replacement verified against the installed SDK for each one. 67 candidates were reviewed adversarially; 6 were refuted, 10 cleared as justified custom code, and the surviving 45 are implemented here.
Stacked on #2097. Base is
worktree-single-window-connections, notmain, because several items build on that branch's commits (the editor tab strip, the workspace registry). Retarget once #2097 lands.On
NSSegmentedControl.role = .tabsWorth recording, since it was the starting point. The API is real but
API_AVAILABLE(macos(27.0)), and this app targets 14.0. Availability is the smaller problem: a segmented control has no per-segment close button, hover fill, tooltip or scrolling track, so it cannot express a document tab strip at any target. Neither canNSTabView. macOS ships no public closable-tab control; Safari and Finder use the privateNSTabBarbehindNSWindow's tab group, which cannot show per-connection tab sets in one window. The strip stays custom, and what it was actually missing was native behaviour, not a native class. One place does use the API: the inspector's split-column alert, guarded, with.valueSelection.Bugs found on the way
Dropon.keyboardShortcut(.defaultAction).proceed().orderswhileuserswas selected truncatedusers.Cmd+W; it ran Close Tab instead.What changed
NSToolbarItems and the two pairs becameNSToolbarItemGroups with subitems and no view, so Icon Only, display-mode customization and the overflow menu work. Only the connection group stays hosted; it carries two SwiftUI popovers and a variable-length row.NSTitlebarAccessoryViewControllerat.bottom, repointed on workspace switch, with the window minimum accounting for it. Tabs are now realButtons with a contextual menu and.isTabBar.NSAlertor a proper modal. Three custom result views deleted. The pairing path no longer nestsrunModalinside a continuation.alternateSelectedControlTextColor; selection dims on window emphasis; tag badges, Pro badge, Vim indicator and the date picker derive a legible label from their fill.Verification
Builds clean,
swiftlint --strictclean on all 101 touched files, 111 tests pass across 11 new suites, no failures.Reviewer notes
NSHostingViewnever responded tosetEnabled:, so the validation table had no visual effect. Any wrong entry will now show as a permanently dimmed button. Worth a pass over the toolbar on first launch.newWindowForTabremoves AppKit's tab-bar plus button. Correct for a single-window app, but it is a visible affordance going away. The toolbar's New Tab was repointed first so it did not go inert.Localizable.xcstringsis deliberately not in this PR. The working copy was dirty with keys from unrelated work, including one for a menu item this PR removes. The new strings fall back to their English key until a build regenerates the catalog.PersistedTabRoundTripTests.swiftcalledtoPersistedTab(windowGroupIndex:), removed on the base branch without the test being updated, which broke the whole test target. Changed to set the field directly, preserving coverage.Single commit rather than one per item: the changes share new types (
Color+Emphasis,MotionAccessibility,DialogFooter), so any split would produce intermediate commits that do not build.https://claude.ai/code/session_01A3rb597qZtq4h5xZxwg43W