diff --git a/docs/spec/beads-overview-band.md b/docs/spec/beads-overview-band.md index c6f1c906..d9a6c000 100644 --- a/docs/spec/beads-overview-band.md +++ b/docs/spec/beads-overview-band.md @@ -279,7 +279,10 @@ the solution compiling (no compat shims, per house rules). geometry and refreshed by `ResizeObserver`, avoiding platform-font pixel tuning. An `IntersectionObserver` watches only the sentinel and reports pinned after it passes strictly above the dashboard boundary. Its Elmish subscription exists only while agent groups are rendered, so - removing the Agents DOM disposes the observers and resets pinned state. Entering the pinned state + removing the Agents DOM disposes the observers and resets pinned state. Attachment resolves its + nodes on each animation frame until they exist, because the subscription can start before React + has committed the band and a single missed lookup would otherwise leave the strip permanently + unpinned with no error. Entering the pinned state closes an agent drill-down and switches Agents to one `nowrap` row with hidden-scrollbar horizontal overflow. Expanded category columns still wrap normally. diff --git a/docs/spec/canvas-pane.md b/docs/spec/canvas-pane.md index c70ad9a8..7954ddea 100644 --- a/docs/spec/canvas-pane.md +++ b/docs/spec/canvas-pane.md @@ -58,6 +58,10 @@ A `SystemView` drives its own updates, so it needs neither morph nor the author - The pane opens and closes from the header Canvas button and the `C` key. - Open or closed state persists in global config. - Position selector supports left, right, top, and bottom docking, and the selected position persists. + `.app-layout` always renders the dashboard first and the pane second; docking is purely a CSS + concern (`flex-direction` plus `order` keyed off the layout's position class). Reordering the two + same-typed children in the DOM instead would make React reconcile them by index, silently + re-rendering each existing node with the other subtree and stranding anything bound to those nodes. - Size selector supports 1:1 (default) and 2:1 — at 2:1 the open pane takes two-thirds of the layout instead of half — and the selected size persists in global config. - The pane normally follows the focused worktree. An explicit card-level SystemView action may target another worktree without moving dashboard card focus; the next explicit card selection clears that override. - The worktree diff is explicit-only when another canvas document exists. Automatic fallback and explicit card selection prefer another document; `diff.html` is selected automatically only when it is the worktree's sole canvas document. The card Diff action and direct tab selection still open it. The server omits the generated `diff.html` from a confirmed-clean worktree's inventory (`docs/spec/worktree-diff-viewer.md`), so a clean worktree shows no diff tab — the tab strip needs no per-view visibility rule of its own. diff --git a/docs/spec/overview-drilldown.md b/docs/spec/overview-drilldown.md index e631e2d0..f98387ce 100644 --- a/docs/spec/overview-drilldown.md +++ b/docs/spec/overview-drilldown.md @@ -157,7 +157,9 @@ type OverviewSelection = translation is derived from rendered geometry and refreshed by `ResizeObserver`, so platform font metrics land on the same center. An `IntersectionObserver` is active only while agent groups exist, watches only the sentinel, closes an agent selection after it passes strictly above the - dashboard boundary, and enables the pinned `nowrap` layout with horizontal overflow. + dashboard boundary, and enables the pinned `nowrap` layout with horizontal overflow. It re-resolves + its nodes each frame until the band is committed, so a subscription that starts ahead of React's + commit still attaches. - Render the breakdown panel below the relevant row when a matching group is selected: the ✕ close button (top-right corner, absolutely positioned so it adds no vertical space), repo-grouped members, agent chips vs. task bars. diff --git a/src/Client/App.fs b/src/Client/App.fs index 545a1423..16ddece9 100644 --- a/src/Client/App.fs +++ b/src/Client/App.fs @@ -1076,18 +1076,15 @@ let view model dispatch = let canvasEl = CanvasView.view model dispatch - let children = - match model.Canvas.CanvasPosition with - | CanvasPosition.Left - | CanvasPosition.Top -> [ canvasEl; dashboardEl ] - | CanvasPosition.Right - | CanvasPosition.Bottom -> [ dashboardEl; canvasEl ] - + // DOM order is fixed; the dock position is applied by CSS `order` on .app-layout's position + // class. Reordering these two same-typed divs instead makes React reconcile them by index, which + // silently re-renders each existing node with the other subtree and leaves node-bound resources + // (the Overview sticky observers) watching the wrong pane. React.Fragment [ viewAppHeader model dispatch Html.div [ prop.className layoutClass - prop.children children + prop.children [ dashboardEl; canvasEl ] ] ] diff --git a/src/Client/OverviewBand.fs b/src/Client/OverviewBand.fs index d395a480..2df4c45f 100644 --- a/src/Client/OverviewBand.fs +++ b/src/Client/OverviewBand.fs @@ -81,11 +81,21 @@ let private createPinnedObservers (onChange: bool -> unit) = | _ -> [] let observePinnedState (onChange: bool -> unit) = - // Observer attachment follows the React commit, so the handles must live across the frame callback. + // The band's nodes only exist once React has committed, which can happen after this + // subscription starts, so re-resolve them every frame until they appear rather than giving up + // silently on the first miss. Mutation is the impure boundary: the frame callback runs after + // this function returns, yet Dispose has to reach whatever it eventually attached. let mutable observers = [] - let frameId: int = - Dom.window?requestAnimationFrame(fun (_: float) -> - observers <- createPinnedObservers onChange) + let mutable frameId = 0 + + let rec attachOnNextFrame () = + frameId <- + Dom.window?requestAnimationFrame(fun (_: float) -> + match createPinnedObservers onChange with + | [] -> attachOnNextFrame () + | attached -> observers <- attached) + + attachOnNextFrame () { new System.IDisposable with member _.Dispose() = diff --git a/src/Client/index.html b/src/Client/index.html index df6a6d1c..c3857161 100644 --- a/src/Client/index.html +++ b/src/Client/index.html @@ -777,6 +777,9 @@ .app-layout.canvas-size-2to1.canvas-bottom > .canvas-pane.open { flex: 0 0 66.667%; } + /* Docking left/top paints the pane first without changing DOM order (see App.fs view). */ + .app-layout.canvas-left > .canvas-pane, + .app-layout.canvas-top > .canvas-pane { order: -1; } .app-layout.canvas-right > .canvas-pane { border-left: 1px solid #313244; } .app-layout.canvas-left > .canvas-pane { border-right: 1px solid #313244; } .app-layout.canvas-top > .canvas-pane { border-bottom: 1px solid #313244; } diff --git a/src/Tests/OverviewBandE2ETests.fs b/src/Tests/OverviewBandE2ETests.fs index 4be81e62..179629d9 100644 --- a/src/Tests/OverviewBandE2ETests.fs +++ b/src/Tests/OverviewBandE2ETests.fs @@ -167,6 +167,16 @@ let private bandProbeJs = } """ +/// Scrolls the dashboard just past the sticky boundary, where the band is fully pinned. +let private scrollPastStickyBoundaryJs = + """ + () => { + const dashboard = document.querySelector('.dashboard'); + const morphRange = parseFloat(getComputedStyle(dashboard).getPropertyValue('--overview-agents-morph-range')); + dashboard.scrollTop = morphRange + 1; + } + """ + let private cardProbeJs = """ () => { @@ -993,3 +1003,144 @@ type OverviewBandE2ETests() = PageWaitForFunctionOptions(Timeout = 5000.0f)) () } + + // Docking left/top used to reverse the two .app-layout children, which React reconciled by + // index — the pinned-state observers silently ended up watching the canvas pane, so neither + // scroll-closes-drill-down nor pinned-click-scrolls-to-top fired again. + [] + member this.``Docking the canvas left keeps pinned-state detection alive``() = + task { + let canvasBtn = + this.Page.Locator(".header-controls .ctrl-btn", PageLocatorOptions(HasText = "Canvas")) + do! canvasBtn.ClickAsync() + do! this.Page.Locator(".canvas-tab-bar").WaitForAsync(LocatorWaitForOptions(Timeout = 5000.0f)) + do! this.Page.Locator(".canvas-pos-btn[title='Dock left']").ClickAsync() + do! this.Page.Locator(".app-layout.canvas-left").WaitForAsync(LocatorWaitForOptions(Timeout = 5000.0f)) + + let! layoutJson = + this.Page.EvaluateAsync( + """() => { + const layout = document.querySelector('.app-layout'); + const dashboard = document.querySelector('.dashboard'); + const pane = document.querySelector('.canvas-pane'); + return JSON.stringify({ + firstChildIsDashboard: layout.firstElementChild === dashboard, + panePaintsLeft: pane.getBoundingClientRect().left < dashboard.getBoundingClientRect().left + }); + }""") + + let investigating = + this.Page.Locator(".overview-agents-band .overview-item", PageLocatorOptions(HasText = "Investigating")) + do! investigating.ClickAsync() + do! this.Page.Locator(".overview-breakdown").WaitForAsync(LocatorWaitForOptions(Timeout = 5000.0f)) + + let! _ = this.Page.EvaluateAsync(scrollPastStickyBoundaryJs) + + let! _ = + this.Page.WaitForFunctionAsync( + """() => !document.querySelector('.overview-breakdown') + && !document.querySelector('.overview-item-selected')""", + null, + PageWaitForFunctionOptions(Timeout = 5000.0f)) + + do! investigating.Locator(".overview-circle").First.ClickAsync() + let! _ = + this.Page.WaitForFunctionAsync( + """() => document.querySelector('.dashboard').scrollTop <= 0.5 + && !!document.querySelector('.overview-breakdown')""", + null, + PageWaitForFunctionOptions(Timeout = 5000.0f)) + + let layout = JObject.Parse(layoutJson) + Assert.That(layout.Value("firstChildIsDashboard"), Is.True, "the dashboard stays layout child #0 in every dock position") + Assert.That(layout.Value("panePaintsLeft"), Is.True, "canvas-left paints the pane left of the dashboard via CSS order") + } + + // A first attach that lands before React has committed the band must not give up silently: the + // sentinel is withheld from the first few lookups here, standing in for the real commit race + // that leaves the strip glued from page load. + [] + member this.``Pinned-state observers retry until the band is committed``() = + task { + do! + this.Page.AddInitScriptAsync( + """ + window.__sentinelMissBudget = 3; + const original = Document.prototype.querySelector; + Document.prototype.querySelector = function (selector) { + if (selector === '.overview-agents-stick-sentinel' && window.__sentinelMissBudget > 0) { + window.__sentinelMissBudget--; + return null; + } + return original.call(this, selector); + }; + """) + + let! _ = this.Page.ReloadAsync() + do! this.Page.Locator(".wt-card .branch-name").First.WaitForAsync(LocatorWaitForOptions(Timeout = 15000.0f)) + + let overviewBtn = + this.Page.Locator(".header-controls .ctrl-btn", PageLocatorOptions(HasText = "Overview")) + do! overviewBtn.ClickAsync() + do! this.Page.Locator(".overview-agents-band").WaitForAsync(LocatorWaitForOptions(Timeout = 5000.0f)) + + let! _ = + this.Page.WaitForFunctionAsync( + "() => window.__sentinelMissBudget === 0", + null, + PageWaitForFunctionOptions(Timeout = 5000.0f)) + + do! + this.Page + .Locator(".overview-agents-band .overview-item", PageLocatorOptions(HasText = "Investigating")) + .ClickAsync() + do! this.Page.Locator(".overview-breakdown").WaitForAsync(LocatorWaitForOptions(Timeout = 5000.0f)) + + let! _ = this.Page.EvaluateAsync(scrollPastStickyBoundaryJs) + + let! _ = + this.Page.WaitForFunctionAsync( + """() => !document.querySelector('.overview-breakdown') + && !document.querySelector('.overview-item-selected')""", + null, + PageWaitForFunctionOptions(Timeout = 5000.0f)) + () + } + + // Opening the band below the sticky boundary must report the pinned state on attach, with no + // scroll to trigger it — the observers resolve their nodes after the React commit, not before. + [] + member this.``Overview opened while the dashboard is scrolled starts pinned``() = + task { + let overviewBtn = + this.Page.Locator(".header-controls .ctrl-btn", PageLocatorOptions(HasText = "Overview")) + do! overviewBtn.ClickAsync() + do! Assertions.Expect(this.Page.Locator(".overview-agents-band")).ToHaveCountAsync(0) + + let! scrolled = + this.Page.EvaluateAsync( + """() => { + const dashboard = document.querySelector('.dashboard'); + dashboard.scrollTop = dashboard.scrollHeight; + return dashboard.scrollTop; + }""") + + Assert.That(scrolled, Is.GreaterThan(150.0), "the dashboard must be scrollable for this scenario to mean anything") + + do! overviewBtn.ClickAsync() + do! this.Page.Locator(".overview-agents-band").WaitForAsync(LocatorWaitForOptions(Timeout = 5000.0f)) + + do! + this.Page + .Locator(".overview-agents-band .overview-item", PageLocatorOptions(HasText = "Investigating")) + .Locator(".overview-circle") + .First.ClickAsync() + + let! _ = + this.Page.WaitForFunctionAsync( + """() => document.querySelector('.dashboard').scrollTop <= 0.5 + && !!document.querySelector('.overview-breakdown')""", + null, + PageWaitForFunctionOptions(Timeout = 5000.0f)) + () + }