fix(client): stop tab changes corrupting and scrambling widget layouts - #176
Conversation
Switching tabs with the dashboard unlocked overwrote the destination tab's saved layout, and visibly shuffled the widgets while doing it. Both came from the same place: the grid being handed a layout that did not describe the children it was rendering. `layout` state is rebuilt by an effect, so during a tab change it still describes the previous tab while the children are already the new tab's. Passing it straight to the grid gives entries whose `i` matches no child, and children with no entry at all. With preventCollision set, the grid synthesises placements for the orphans and moves non-static items around until nothing collides — the visible scramble. It then reports that arrangement through onLayoutChange, and it was persisted under the new tab's number. Widgets present on both tabs inherited the previous tab's coordinates; widgets only on the destination tab fell through to `item.minW ?? 2` and collapsed to 2x2. On a two-tab dashboard of six widgets and two, one unlocked switch destroyed the six-widget tab and left three overlapping pairs. It only happened unlocked because static items are pinned and excluded from collision movement, which is why a wall display never showed it. Two changes: `buildLayout` derives the layout during render, one entry per child, from `widget.savedLayout` — which the caller has already scoped to the active tab, so it is correct even mid-transition. A widget without a saved layout takes the first free cell rather than its default position: defaults are not unique, every plugin widget declares (0,0) at 6x4, and honouring them naively stacks the whole plugin set in one place. The rebuild effect now uses the same helper instead of its own copy of the placement loop. `shouldAcceptLayoutChange` refuses to persist a layout whose tab does not match the active one. The derived layout should prevent a bad arrangement from being reported at all, but writing one tab's geometry under another's number is the damaging outcome here, so it is worth refusing outright. The lock transition takes the same check, since it persists on its own trigger. Tab 0 is falsy and valid, so the "no layout yet" case is an explicit null check rather than a truthiness test. Verified on a live deployment: the corruption was deterministic beforehand - byte-identical damage from two different routes to the same tab - and after the change a tab switch writes nothing, a drag still persists, and no overlaps are created.
|
Reviewed critically and merging. I reproduced the scramble on The scramble reproduces, and the fix is exactSeeded a two-tab device — tab 1 At 1400px with 12 columns those work out to x=2 and x=7, y=5 — the saved layout, exactly. Main collapses both widgets onto the origin. That is the visible bug, and deriving the layout from I also confirmed a tab switch now writes nothing: 0 calls to The guard cannot get stuck, which was my main worry
Finding:
|
jherforth#176 landed in WidgetContainer first, so both sides added imports at the same place. Nothing else collided: the resize guard sits inside handleResize, and jherforth#176 touched the rebuild effect, handleLayoutChange and the grid's layout prop. Both import lines are kept. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2 participants
Fixes #173
Switching tabs with the dashboard unlocked overwrote the destination tab's saved layout, and visibly shuffled the widgets while doing it. Both came from the same place: the grid being handed a layout that did not describe the children it was rendering.
layoutstate is rebuilt by an effect, so during a tab change it still describes the previous tab while the children are already the new tab's. Passing it straight to the grid gives entries whoseimatches no child, and children with no entry at all. With preventCollision set, the grid synthesises placements for the orphans and moves non-static items around until nothing collides - the visible scramble. It then reports that arrangement through onLayoutChange, and it was persisted under the new tab's number.Widgets present on both tabs inherited the previous tab's coordinates; widgets only on the destination tab fell through to
item.minW ?? 2and collapsed to 2x2. On a two-tab dashboard of six widgets and two, one unlocked switch destroyed the six-widget tab and left three overlapping pairs. It only happened unlocked because static items are pinned and excluded from collision movement, which is why a wall display never showed it.Two changes:
buildLayoutderives the layout during render, one entry per child, fromwidget.savedLayoutwhich the caller has already scoped to the active tab, so it is correct even mid-transition. A widget without a saved layout takes the first free cell rather than its default position: defaults are not unique, every plugin widget declares (0,0) at 6x4, and honouring them naively stacks the whole plugin set in one place. The rebuild effect now uses the same helper instead of its own copy of the placement loop.shouldAcceptLayoutChangerefuses to persist a layout whose tab does not match the active one. The derived layout should prevent a bad arrangement from being reported at all, but writing one tab's geometry under another's number is the damaging outcome here, so it is worth refusing outright. The lock transition takes the same check, since it persists on its own trigger.Tab 0 is falsy and valid, so the "no layout yet" case is an explicit null check rather than a truthiness test.
Verified on a live deployment: the corruption was deterministic beforehand - byte-identical damage from two different routes to the same tab - and after the change a tab switch writes nothing, a drag still persists, and no overlaps are created.