Skip to content

fix(connection-form): restore the editor's Delete and make the type chooser reachable by keyboard - #2647

Merged
datlechin merged 1 commit into
refactor/connection-form-iafrom
fix/connection-editor-collateral
Sep 5, 2026
Merged

fix(connection-form): restore the editor's Delete and make the type chooser reachable by keyboard#2647
datlechin merged 1 commit into
refactor/connection-form-iafrom
fix/connection-editor-collateral

Conversation

@datlechin

Copy link
Copy Markdown
Member

Stacked on #2645. Base that first; this branch is based on refactor/connection-form-ia, not main.

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.destructiveAction through 0.38.0. 9f361ef78 (#995) rebuilt the form and dropped the button while porting its body verbatim into ConnectionFormCoordinator.deleteCurrent(), which has had no caller since. There is no Removed entry in the CHANGELOG and the docs never mentioned it, so this was a silent regression rather than a decision.

$ git show '9f361ef78^:TablePro/Views/Connection/ConnectionFormView+Footer.swift'
ToolbarItem(placement: .destructiveAction) {
    Button(String(localized: "Delete"), role: .destructive) {
        ... AlertHelper.confirmDestructive(title: "Delete Connection", ...)

The window has no toolbar after #2645, so the button takes the bottom bar's leading edge, which is where SSHProfileEditorView already puts the same action. The original AlertHelper.confirmDestructive confirmation 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:

  • Down from the search field did nothing. NativeSearchField already implements the documented route, an NSSearchFieldDelegate handling control(_:textView:doCommandBy:) for moveUp:/moveDown:, and publishes it as onMoveUp/onMoveDown. Five call sites use it; the chooser passed neither. Wiring it is the whole fix. SwiftUI has no alternative here: onMoveCommand and onKeyPress both need the receiving view focused, and a focused field editor consumes arrow keys as doCommandBySelector: first; searchFocused is macOS 15.
  • Filtering to one driver left Continue dimmed, because nothing observed searchText.

The obvious fix for the second one is a data-loss bug, and the adversarial pass is what caught it. filteredTypes is an unranked contains over the driver name, its tagline and its category, and the list renders in category order then alphabetically. So arming the first row means typing PostgreSQL arms CockroachDB, whose tagline reads "Distributed SQL, PostgreSQL-compatible" and which sorts ahead of PostgreSQL inside Relational. Return then commits it, and on an existing connection didChangeType resets auth, SSL, driver options and transport. Typing MySQL arms 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. DatabaseTypeChooserModelTests pins exactly that, including both dangerous cases.

Arrowing follows orderedTypes (what the list draws) rather than filteredTypes (what the filter returns), and .onChange scrolls the armed row into view.

focusOnAppear is deliberately not passed: neither defect needs it, and it would route Escape through the field editor's cancelOperation arm 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 because DatabaseType.iconImage also returns SF Symbols.

RedisConnectionModeUITests reached the wrong search field

app.windows.firstMatch is the welcome window, and the chooser is a .sheet on it, so chooser.searchFields.firstMatch resolved to the welcome window's own connection filter. Both carry the identifier sidebar-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.sh flagged AXCell at CLAUDE.md:220, and verify.sh lint turned that into a FAIL on main as well as on this branch. AXCell is real only as a raw string: Swift spells it NSAccessibility.Role.cell, whose rawValue is literally "AXCell", so the sentence keeps its exact meaning. verify.sh lint now reports clean: 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 a Button, so name/title are missing value for every SwiftUI button in every app. The buttons do carry AXDescription ("Import from URL…", "Cancel", "Continue") and VoiceOver reads them. A probe app with an explicit .accessibilityLabel behaved identically. No defect, no change.

Verification

Step Result
verify.sh generate PASS
verify.sh build PASS
verify.sh test (6 suites) PASS, 50 cases
verify.sh uitest (both connection-form suites) PASS, 6 cases
verify.sh lint TablePro TableProTests TableProUITests 0 violations, docs symbols clean
docs/scripts/check-writing-style.sh PASS
docs/scripts/check-docs-against-source.py PASS

Two 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 SQL is 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

@datlechin
datlechin merged commit 6325fa9 into refactor/connection-form-ia Sep 5, 2026
7 of 8 checks passed
@datlechin
datlechin deleted the fix/connection-editor-collateral branch September 5, 2026 13:45
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
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