Repository navigation
fix(sail-finance): drop empty grid branches when restoring a Dockview layout - #392
Open
patrickboneill wants to merge 2 commits into
Open
patrickboneill wants to merge 2 commits into
patrickboneill wants to merge 2 commits into
Conversation
Dockview collapses a branch left with one child, but not one left with none. An empty branch keeps its share of the grid, so a workspace shows a blank column (or row), and because the emptied layout is what gets saved, the gap survives reloads. A branch can only reach zero children if it held exactly one leaf, which Dockview's drag/split UI never builds — so this only affects layouts written outside Dockview: pre-seeded, imported or hand-written `dockviewLayout` state. Add `normalizeDockviewLayout`, a pure pass over the serialized grid that drops branches without leaves, replaces a redundant single-leaf branch with its leaf (carrying the branch's size), lifts the grandchildren of a single-branch branch, and flips `grid.orientation` when the root is left holding one branch. Layouts needing no change come back by reference, so restore is unchanged for grids Dockview built itself. Call it in the restore effect of Layout.tsx. Normalizing on restore also keeps the gap from appearing in the live session: a seeded single-leaf branch is collapsed before the user closes anything, so closing that panel leaves the root with a single branch, which Dockview collapses itself. Fixes finos#388 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: patrickboneill <patrick.oneill46@gmail.com>
…ency The restore test reached straight for `dockview-core`, which the package does not declare — it only arrives transitively through `dockview-react`. `dockview` is a declared dependency and re-exports all of dockview-core, so take it from there instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: patrickboneill <patrick.oneill46@gmail.com>
❌ Deploy Preview for fdc3-sail failed. Why did it fail? →
|
SeeWhatsOn
self-requested a review
October 21, 2026 13:11
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.
Fixes #388.
Problem
Dockview collapses a branch left with one child (
Gridview.normalize), but not one left with none. An empty branch keeps its share of the grid, so the workspace shows a blank column (or row), and because the emptied layout is what gets saved, the gap survives reloads.A branch can only reach zero children if it held exactly one leaf, which Dockview's own drag/split UI never builds — so this only affects layouts written outside Dockview: pre-seeded, imported or hand-written
dockviewLayoutstate, exactly what a default-workspace hook like #313 would produce.Fix
New
packages/sail-finance/src/components/layout-grid/normalize-layout.tsexporting a purenormalizeDockviewLayout(layout: unknown): unknown, called in the restore effect ofLayout.tsx:It walks
layout.grid.rootand:grid.orientationwhen the root is left holding a single branch, re-sizing the root togrid.heightfor HORIZONTAL andgrid.widthfor VERTICAL;nullwhen no leaves are left, passes non-layout values through, and returns the same reference when nothing changed, so restore behaves exactly as today for grids Dockview built itself.Two points where the implementation is deliberately narrower than a naive recursion:
branch[leaf]into a bare leaf would produce somethingfromJSONdoes not accept — Dockview's root is always a branch, andbranch[leaf]is how it serializes a one-panel workspace. Root children are normalized without collapsing the root itself.visible: false, the leaf'ssizeis Dockview'scachedVisibleSize; overwriting it with the branch's size would corrupt the restored width of a hidden panel.Normalizing on restore prevents the gap rather than only repairing it
Because the redundant single-leaf branch is collapsed on first load — nothing moves, A keeps its 400 — closing that panel later leaves the root holding a single branch, which Dockview collapses itself. So B/C expand immediately in the live session, not just after a reload. There is a test pinning this.
The save path is left alone on purpose, and the live grid is never re-fed through
fromJSON: that remounts groups, which would reload the panel iframes and tear down their FDC3 connections.Tests
src/__tests__/components/normalize-layout.test.ts— 11 unit tests: a clean layout returns the identical object;[leaf, leaf, branch[]]drops the empty branch with the orientation unchanged;[branch[leaf(400)], branch[leaf, leaf]]yields[leaf(size 400), branch[leaf, leaf]];[branch[], branch[leaf, leaf]]flips the root to VERTICAL withsize === width; a hidden leaf keeps its own size; nested single-branch chains lift their grandchildren without changing the axis; a layout with no leaves returnsnull;null,undefined,{}and grid-less objects pass through;panels/activeGroupsurvive a rewrite.src/__tests__/components/dockview-layout-restore.test.ts— the issue's reproduction against a realDockviewComponent, run headlessly in jsdom (which needs only aResizeObserverstub, since these tests size the grid explicitly vialayout()):[branch[] size 400, …], B/C stay 600Plus: a layout Dockview built itself (via
addPanel) comes back by reference; collapsing the redundant seeded branch leaves every width unchanged; and after a normalized restore, closing A leaves no empty branch at all.Verification
vp test runforsail-finance: 44/44 passing (17 of them new).tsc,vp lint .andvp fmt --check .: clean.packages/sail-finance/src, and no manifest or lockfile is touched. The test importsDockviewComponentfromdockview, which this package declares and which re-exports dockview-core.The issue's note about seeded workspaces emitting a one-panel column as a plain leaf needs no change yet: nothing in the branch builds layouts programmatically (that arrives with #313).
🤖 Generated with Claude Code