refactor(connection-form): one transport picker and four tabs in place of eleven panes - #2645
refactor(connection-form): one transport picker and four tabs in place of eleven panes#2645datlechin wants to merge 6 commits into
Conversation
…e of eleven panes Claude-Session: https://claude.ai/code/session_01EkZFosuu6fvVgdVmNaUFt9
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
|
Screenshots (1x, 1920x1080 display). Attached from the local capture run; paths for reference:
|
… and refresh the docs shots Claude-Session: https://claude.ai/code/session_01EkZFosuu6fvVgdVmNaUFt9
|
Updated since the first review round. A regression this branch had introduced is fixed. Twenty-two docs screenshots re-captured, replacing 11 of the 22 stale pairs listed above: Still stale, and why. Ten pairs need drivers that are registry-only, and a sandboxed run cannot load them: A note on the capture, because it repeats a lesson from this branch's UI test. The first batch reported 22 successes and every non-General shot was silently the General tab: System Events matches a control by Five collateral defects found while investigating this refactor are now built in #2647, stacked on this branch. |
…hooser reachable by keyboard (#2647) Claude-Session: https://claude.ai/code/session_01EkZFosuu6fvVgdVmNaUFt9
…gationSplitView sidebar Claude-Session: https://claude.ai/code/session_01EkZFosuu6fvVgdVmNaUFt9
Why
The connection form modelled one exclusive choice as five independent booleans, and modelled one object's facets as a navigation sidebar. Both are category errors, and together they produced a form with up to eleven destinations for a task whose common case is six fields.
The exclusivity was more than an IA problem. From
TablePro/Models/Connection/ConnectionTunnelKind.swift:76:With two transports enabled,
activeTunnelKindisnil,DatabaseManager.activeTunnelManagerreturnsnil, and the connection opens directly to the database host with neither transport applied.isFormValidnever checked for it, so Save stayed enabled. The form's only defence was an orange banner apologising for a state the model's own doc comment already calls impossible ("exactly one transport per connection"). For a database reachable only through a bastion that is a failed connect; for one reachable both ways it is an unintended direct connection that bypasses the proxy the user chose.Five of PostgreSQL's ten sidebar rows were those mutually-exclusive transports, and each was a full-height pane containing one
Toggle.What changed
Transport is one value.
ConnectionFormCoordinator.transportis aConnectionTunnelKind?, the same typeDatabaseConnection.activeTunnelKindreturns, so the form now edits exactly what the connect path reads. Selecting one disables the rest, which makes the broken state unrepresentable.TunnelExclusivityBanner,ConnectionFormCoordinator+TunnelExclusivity.swiftand the "Cannot use X and Y at the same time" rules in five view models are gone.normalizeTransport()collapses a connection stored by an older build onto its first transport on load, which is a repair rather than a change: the connect path was already ignoring both.Eleven panes become four tabs: General, Network, Options, Appearance. The set is fixed except Network, which is absent only for a driver that connects directly and cannot negotiate TLS, so the form no longer changes shape between database types.
The commit actions moved to a bottom action bar, which is where macOS puts them for a window that edits one object, and that is where the validation message now lives. Every pane computed
validationIssues: [String]and every one of those strings was thrown away by a warning triangle; the reason Save is dimmed now sits next to Save, prefixed with the tab that owns it when that is a different tab.Smaller repairs in the same subsystem:
NetworkPaneViewModel.setTypehad no caller, so the database type was fixed once the form opened. Change… on the General tab now retypes the connection.RemoteFilePaneViewwrappedConnectionSSHTunnelViewin a secondForm, so its grouped rows were inset twice. Both are sections of one form now.usePgpassis a password-only controller and was being lifted above Username along with the auth-method dropdowns.splitCredentialControllers()now returns username controllers and password controllers separately, so each sits directly above what it controls.movesFocusOnTab.SSLPaneViewModel.validationIssuesfired on drivers that render no SSL section, which could dim Save over a certificate field the form does not show.GeneralPaneViewwas the only pane missing.scrollContentBackground(.hidden).The window title, the accessibility identifiers the existing UI tests use,
ConnectionFormEdits, and everybuildTunnelMode()are unchanged, so persistence andConnectionFormEditsCoverageTestsare untouched.A note on the tab control
TabView+.tabItem { Text }was the first implementation, followingCompareApplySheetView. On macOS 26 the SwiftUI tab bar was redesigned and it renders as a collapsed ~30pt stub. Measured on macOS 27, screenshot below. The tab bar is anNSSegmentedControlthrough a.segmentedPickerinstead, which is what AppKit puts above the content of a single-object editor and is stable across versions. It publishes as a radio group, so VoiceOver and XCUITest both read it.Before / After
Before: General. Ten sidebar destinations, a red triangle with no text anywhere explaining it, and a dimmed Save.
Before: SSH Tunnel. A whole 600pt pane whose entire content is one off toggle, in a pane already titled "SSH Tunnel". Four more panes were identical.
After: General. Four segments, the type visible and changeable, and "Connection name is required" beside the dimmed Save.
After: Network. The five panes replaced by one Connect via picker with a line saying what the choice does, SSL below it, and the cross-tab validation message reading "General: Connection name is required".
After: Network with SSH Tunnel. The same pane that used to hold one toggle, now holding the transport and its whole configuration.
Screenshots are attached in a comment. They are 1x captures on a 1920x1080 display, so they are half the pixel density of the docs images taken on a 2x display.
Verification
verify.sh generateverify.sh buildverify.sh test(8 suites)verify.sh uitest ConnectionFormTransportUITestsverify.sh lint TablePro TableProTests TableProUITestsdocs/scripts/check-writing-style.shdocs/scripts/check-docs-against-source.pyConnectionFormTransportTestsis new and covers the exclusivity round-trip, the legacy collapse, that switching away keeps the previous transport's fields, that a transport the type no longer offers falls back to direct, and that every issue blocking Save is claimed by a visible tab.ConnectionFormTunnelExclusivityTestsis deleted: every case in it asserted the behaviour this PR removes.ConnectionFormTransportUITestsdrives the real form for the picker swap and the validation message.Review
Codex could not read this diff: the workspace is out of credits until Sep 7 (
You've hit your usage limit, read from the job log on two attempts, since a--waitrun exits 0 either way).Skill(code-review)reviewed it instead and raised 15 findings. Fourteen were real and are fixed in this branch:validationIssuessummed only visible tabs, so an issue on a hidden Network tab was dropped fromisFormValidand Save was enabled over a half-configured transport thatbuildEdits()still persisted.normalizeTransport()now also runs after the URL-import path, which is what could leave a transport enabled that no tab offered.transportgetter chose.sshvs.remoteFileby database type rather than by what was written, so a type offering both would let the picker select one and read back the other.availableTransportsnow offers exactly one of the pair.remoteFilePathandsshForwardUnixSocketPathcould survive into a transport whose form renders no control for them, while still being written on save. The setter clears each when it stops being reachable.changeTypemade retyping reachable for the first time, andsaveConnectiondeleted secure plugin fields for the new type only, leaving the old type's Keychain entries under the same connection id. It now mirrors whatownedAdditionalFieldIDs()already did for plain fields.visibleTabswas hand-assembled; it is nowallCasesfiltered by a per-tab rule, so the compiler forces the answer for a case added later.customization.validationIssueswhile Options renders the Safe Mode picker that view model owns, which would have sent the user to a tab with no such control.Text(String)renders verbatim, so the backticks in the transport summaries printed as characters; the summaries are plain prose now..sheetmodifiers on one view resolve to a single presenter on macOS, which could have leftisChoosingTypestuck true and Change… dead.directSummarypointed a file-based driver at a Host and Port it does not have.### Fixedinto two lists, and a Fixed entry for a pane this same unreleased change deletes.One finding is refuted by measurement:
.navigationTitlewas said to have no consumer after theNavigationSplitViewwas removed. The running build's window reads "New PostgreSQL Connection", so it still propagates.The review also caught that
XCTSkipUnlessin the UI test's tab helper would turn a broken tab bar green. Removing it exposed a genuine failure:app.searchFields.firstMatchwas matching the welcome window'ssidebar-filterrather than the chooser sheet's search field, so the driver name went into the connection filter and the chooser list was never filtered. The helper is scoped to the sheet now.TableProUITests/RedisConnectionModeUITests.swift:69has the same latent mismatch and passes only because "Redis" is realised in the unfiltered list.Docs
docs/connections/connection-form.mdxis rewritten around the four tabs and the transport picker, and its screenshot pair is re-captured. Fourteen other pages instructed readers to turn on toggles that no longer exist, named panes that are now sections, or gave a location for a control that moved; all are updated.quickstart.mdxin particular told the reader to click Test Connection in a Status row and Save & Connect in the toolbar, and neither is where it was.Screenshots still to re-capture. 22 image pairs under
docs/images/show the old form. This PR re-capturesconnection-form-fields; the other 21 pairs are stale and are not done here, because they span 20-odd database types in two appearances and several need plugins that are registry-only:cloud-sql-proxy-pane,cloudflare-d1-connection-form,cloudflare-tunnel-pane,cockroachdb-connection-form,connection-customization,connection-ssl-settings,connection-test,dameng-connection-form,duckdb-connection-form,dynamodb-connection-form,mongodb-connection-form,mssql-connection-form,oracle-connection-form,pglite-connection-form,postgresql-connection-form,snowflake-connection-form,socks-proxy-pane,sqlite-connection-form,ssh-tunnel-config,teradata-connection-form,trino-connection-form,tunnel-command-pane.https://claude.ai/code/session_01EkZFosuu6fvVgdVmNaUFt9