fix(connection-form): restore the editor's Delete and make the type chooser reachable by keyboard - #2647
Merged
datlechin merged 1 commit intoSep 5, 2026
Conversation
…hooser reachable by keyboard Claude-Session: https://claude.ai/code/session_01EkZFosuu6fvVgdVmNaUFt9
datlechin
added a commit
that referenced
this pull request
Sep 6, 2026
…e of eleven panes (#2645) * refactor(connection-form): one transport picker and four tabs in place of eleven panes Claude-Session: https://claude.ai/code/session_01EkZFosuu6fvVgdVmNaUFt9 * fix(connection-form): drop the duplicate type-chooser sheet presenter and refresh the docs shots Claude-Session: https://claude.ai/code/session_01EkZFosuu6fvVgdVmNaUFt9 * fix(connection-form): restore the editor's Delete and make the type chooser reachable by keyboard (#2647) Claude-Session: https://claude.ai/code/session_01EkZFosuu6fvVgdVmNaUFt9 * refactor(connection-form): navigate the editor's sections with a NavigationSplitView sidebar Claude-Session: https://claude.ai/code/session_01EkZFosuu6fvVgdVmNaUFt9 * fix(connection-form): let the sidebar run the window's full height Claude-Session: https://claude.ai/code/session_01EkZFosuu6fvVgdVmNaUFt9 * docs(connections): describe the editor's sections and re-capture its screenshots Claude-Session: https://claude.ai/code/session_01EkZFosuu6fvVgdVmNaUFt9 * refactor(connection-form): build the editor on NSSplitViewController with a sidebar toolbar Claude-Session: https://claude.ai/code/session_01EkZFosuu6fvVgdVmNaUFt9 * fix(connection-form): drop the sidebar toggle from the editor's toolbar Claude-Session: https://claude.ai/code/session_01EkZFosuu6fvVgdVmNaUFt9
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.
Stacked on #2645. Base that first; this branch is based on
refactor/connection-form-ia, notmain.Five defects found while investigating the connection-form rewrite, reported there and built here.
Delete Connection, missing since 0.39.0
The editor shipped a Delete at
ToolbarItemPlacement.destructiveActionthrough 0.38.0.9f361ef78(#995) rebuilt the form and dropped the button while porting its body verbatim intoConnectionFormCoordinator.deleteCurrent(), which has had no caller since. There is noRemovedentry in the CHANGELOG and the docs never mentioned it, so this was a silent regression rather than a decision.The window has no toolbar after #2645, so the button takes the bottom bar's leading edge, which is where
SSHProfileEditorViewalready puts the same action. The originalAlertHelper.confirmDestructiveconfirmation comes back with it.This was nearly not built. The investigation concluded "redundant, not missing" on the strength of four existing delete routes in the welcome window; the adversarial pass overturned it on git history.
The type chooser could not be driven from the keyboard
Two defects, one change:
NativeSearchFieldalready implements the documented route, anNSSearchFieldDelegatehandlingcontrol(_:textView:doCommandBy:)formoveUp:/moveDown:, and publishes it asonMoveUp/onMoveDown. Five call sites use it; the chooser passed neither. Wiring it is the whole fix. SwiftUI has no alternative here:onMoveCommandandonKeyPressboth need the receiving view focused, and a focused field editor consumes arrow keys asdoCommandBySelector:first;searchFocusedis macOS 15.searchText.The obvious fix for the second one is a data-loss bug, and the adversarial pass is what caught it.
filteredTypesis an unrankedcontainsover the driver name, its tagline and its category, and the list renders in category order then alphabetically. So arming the first row means typingPostgreSQLarms CockroachDB, whose tagline reads "Distributed SQL, PostgreSQL-compatible" and which sorts ahead of PostgreSQL inside Relational. Return then commits it, and on an existing connectiondidChangeTyperesets auth, SSL, driver options and transport. TypingMySQLarms MariaDB the same way.So the match is ranked before anything is armed: exact driver name, then prefix, then name-contains, then a sole result. A query matching several rows on tagline or category alone arms nothing and leaves Continue dimmed, because no row is a defensible default there.
DatabaseTypeChooserModelTestspins exactly that, including both dangerous cases.Arrowing follows
orderedTypes(what the list draws) rather thanfilteredTypes(what the filter returns), and.onChangescrolls the armed row into view.focusOnAppearis deliberately not passed: neither defect needs it, and it would route Escape through the field editor'scancelOperationarm in a sheet that also carries.cancelAction.VoiceOver read the chooser row's icon before the driver
Each row's driver icon published its own element, so VoiceOver announced "Cylinder Shape, Filled" ahead of the name.
.accessibilityHidden(true)at the call site;Image(decorative:)cannot cover it becauseDatabaseType.iconImagealso returns SF Symbols.RedisConnectionModeUITests reached the wrong search field
app.windows.firstMatchis the welcome window, and the chooser is a.sheeton it, sochooser.searchFields.firstMatchresolved to the welcome window's own connection filter. Both carry the identifiersidebar-filter, so identifiers cannot separate them. "Redis" went into the connection filter and the chooser list was never filtered; the suite passed only because Redis is realised in the unfiltered list. Scoped to the sheet, matching the sibling suite added in #2645, which failed outright for this reason.CLAUDE.md named a symbol that does not exist in Swift
scripts/check-doc-symbols.shflaggedAXCellat CLAUDE.md:220, andverify.sh lintturned that into a FAIL onmainas well as on this branch.AXCellis real only as a raw string: Swift spells itNSAccessibility.Role.cell, whoserawValueis literally"AXCell", so the sentence keeps its exact meaning.verify.sh lintnow reportsclean: 291 references check out.Worth stating plainly for the next reader: the check passes because a dotted path yields no token to extract, not because the symbol resolves.
One finding withdrawn
I reported the chooser's footer buttons as publishing no accessibility name. That was a measurement artifact of my own tooling: System Events reads
AXTitle, which SwiftUI never publishes for aButton, soname/titlearemissing valuefor every SwiftUI button in every app. The buttons do carryAXDescription("Import from URL…", "Cancel", "Continue") and VoiceOver reads them. A probe app with an explicit.accessibilityLabelbehaved identically. No defect, no change.Verification
verify.sh generateverify.sh buildverify.sh test(6 suites)verify.sh uitest(both connection-form suites)verify.sh lint TablePro TableProTests TableProUITestsdocs/scripts/check-writing-style.shdocs/scripts/check-docs-against-source.pyTwo of the new chooser tests failed on their first run. Both expectations were wrong rather than the code: keeping a still-visible highlight when the query is cleared is the in-repo precedent, and
SQLis a genuine prefix of SQLite so arming it is correct. The assertions now say what the design actually intends.Codex is still out of credits until Sep 7, so this diff has not had an independent model review either. The design was produced and then adversarially refuted through the workflow described above, which is what changed the chooser fix.
https://claude.ai/code/session_01EkZFosuu6fvVgdVmNaUFt9