Skip to content

fix: redraw views that read a value through an object they observe - #2944

Merged
datlechin merged 4 commits into
mainfrom
fix/nested-observable-forwarding
Sep 17, 2026
Merged

datlechin merged 4 commits into
mainfrom
fix/nested-observable-forwarding

Conversation

@datlechin

@datlechin datlechin commented Sep 17, 2026

Copy link
Copy Markdown
Member

Follow-up to #2940. Two more of main's red UI tests, and the rest of the #2874 observation audit: views that read a value through an object they observe.

One pattern left from the macOS 13 port

@Published var child: SomeObservableObject publishes only when the reference is replaced, never when a value inside the child changes. Observation tracked parent.child.value read in a body; ObservableObject does not. #2940 covered properties with no @Published and objects nothing observed. This covers the nested case.

Surface What broke Fix
Connection form Picking Sentinel left the Redis mode picker on Standalone with Host and Port still showing. Every pane observes the form's coordinator and reads its values through one of 12 pane view models, so any field that shows or hides on another field's value stayed as drawn The coordinator passes each pane view model's changes on to its own observers, and moves the subscription to a replacement view model
Editor tab strip A tab's busy spinner followed whatever else redrew the strip. It read tabExecution through a weak reference, which cannot be @ObservedObject TabExecutionObservation holds the coordinator weakly and relays tabExecution alone, not the cursor updates the coordinator also publishes on every keystroke
Main window The onChange for the sidebar selection and for the inspector's view mode read values held by WindowSidebarState and RowInspectorState, which the view does not observe onValueChange(of:in:) observes the object inside the modifier, so its other changes redraw only the modifier

For the main window, both sidebar layouts open a clicked table from the outline itself, so I did not reproduce a visible symptom there. The change puts back what that onChange relied on before the port.

Checked and left alone: the other nested reads are inside action closures, or sit in a view that is redrawn with a parent observing the same object (MainEditorContentView takes closures from MainContentView, which observes toolbarState). ImportDialog reads its service's isImporting without observing it, but the progress sheet covers that button for the whole import.

None of this shipped, since #2874 is unreleased, so there is no CHANGELOG entry.

ColumnJumpUITests

The jump works. The test did not.

  • isHittable on a grid header never becomes true. After the jump, col_60 sat at x 1795 to 1880 inside a grid spanning 959 to 1881, and XCUITest still reported it not hittable. HeaderSortUITests hit the same thing in test: fix the three tests that are red on main #2676. The test now checks the header frame's centre against the grid's frame.
  • The 60-column query was typed straight after a click. CI reported no col_60 header and no panel on Command Shift J, which is what a query that lost its first keystrokes produces. It goes through typeQuery now, which confirms the text arrived.

Verification

  • New unit tests: ConnectionFormChildObservationTests (fails 3 of 3 without the coordinator change), TabExecutionObservationTests, ObservedValueChangeTests. The last one hosts a view and also asserts that the read-through form misses the change, so the passing case is not vacuous.
  • UI tests passing locally: RedisConnectionModeUITests, ColumnJumpUITests (twice), ConnectionFormTransportUITests, JSONRowInspectorUITests, InspectorEditReachesGridUITests, EditorTabReorderUITests, EditorTabKeepOpenUITests, DataGridReloadViewportUITests.
  • One flake: ConnectionFormTransportUITests.testChoosingATransportReplacesThePreviousOnesFields failed once on "Typing should reach the chooser's search field", which is before the form exists. It then passed two runs in a row.

Crash reports from UI shards

QueryInsightsTabUITests crashes the app on the macOS 26 runner right after Database > Query Insights, in every run since #2874, and passes on macOS 27. XCTest reports only "crashed in " and attaches no report, so there is nothing to read. A failed shard now uploads TablePro* from both DiagnosticReports folders as macos-ui-crash-reports-<shard>. This run should produce the first one.

@datlechin
datlechin merged commit 1b39115 into main Sep 17, 2026
8 checks passed
@datlechin
datlechin deleted the fix/nested-observable-forwarding branch September 17, 2026 11:32
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