Skip to content

fix(hig): replace hand-rolled controls with native appkit and swiftui equivalents - #2104

Merged
datlechin merged 1 commit into
worktree-single-window-connectionsfrom
native-hig-conformance
Aug 13, 2026
Merged

fix(hig): replace hand-rolled controls with native appkit and swiftui equivalents#2104
datlechin merged 1 commit into
worktree-single-window-connectionsfrom
native-hig-conformance

Conversation

@datlechin

Copy link
Copy Markdown
Member

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, not main, because several items build on that branch's commits (the editor tab strip, the workspace registry). Retarget once #2097 lands.

On NSSegmentedControl.role = .tabs

Worth 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 can NSTabView. macOS ships no public closable-tab control; Safari and Finder use the private NSTabBar behind NSWindow'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

  • Return key dropped a table. The confirmation was a hand-built sheet with Drop on .keyboardShortcut(.defaultAction).
  • The inspector deleted with no prompt at all when it could not resolve a window; the guard called proceed().
  • Right-clicking a table acted on the previous selection, so right-clicking orders while users was selected truncated users.
  • Tab trapped focus in the data grid permanently, and walked hidden columns.
  • The shortcut recorder could not record Cmd+W; it ran Close Tab instead.
  • Theme, schema and diagram export swallowed write errors, so a read-only folder looked like success.

What changed

  • Toolbar: six hosted SwiftUI buttons became real bordered NSToolbarItems and the two pairs became NSToolbarItemGroups 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.
  • Tab strip moved into the titlebar as an NSTitlebarAccessoryViewController at .bottom, repointed on workspace switch, with the window minimum accounting for it. Tabs are now real Buttons with a contextual menu and .isTabBar.
  • Alerts: drop/truncate, import and export results, and the pairing grant are NSAlert or a proper modal. Three custom result views deleted. The pairing path no longer nests runModal inside a continuation.
  • Colour: eleven hardcoded whites replaced with alternateSelectedControlTextColor; selection dims on window emphasis; tag badges, Pro badge, Vim indicator and the date picker derive a legible label from their fill.
  • Keyboard and assistive tech: Tab escapes the grid, type-select in the tree, cell selection reaches VoiceOver, Settings pickers and Quick Switcher rows are named.
  • Motion and colour independence: one Reduce Motion gate over 12 sites; split diffs marked with glyphs under Differentiate Without Colour; the Reduce Transparency scrim is genuinely opaque.
  • Plus native controls for tag pills, colour swatches, the group picker, host-list buttons and tag chips; Settings resizes; SQL files can be dropped on a window; dead code removed.

Verification

Builds clean, swiftlint --strict clean on all 101 touched files, 111 tests pass across 11 new suites, no failures.

Reviewer notes

  1. Toolbar validation is now live. NSHostingView never responded to setEnabled:, 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.
  2. Deleting newWindowForTab removes 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.
  3. Localizable.xcstrings is 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.
  4. One file here is not mine: PersistedTabRoundTripTests.swift called toPersistedTab(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.
  5. Not done on purpose: the Redis single-click to double-click change for namespace browsing. That is a behaviour change, not a conformance fix, so it needs a decision rather than a patch.

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

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@datlechin
datlechin merged commit 6d79728 into worktree-single-window-connections Aug 13, 2026
3 of 4 checks passed
@datlechin
datlechin deleted the native-hig-conformance branch August 13, 2026 11:31
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
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