Repository navigation
fix(workspace): restore saved layouts and contain docked previews - #217
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request ensures that docked previews scroll within their panels instead of extending beneath adjacent panels by ignoring reported content heights when docked. It also improves Dockview layout restoration by rejecting the entire layout if an invalid or retired panel is found, rather than silently deleting it. Feedback on the changes suggests using Vitest's fake timers (vi.useFakeTimers()) instead of real-time delays (setTimeout) in the workspace unit tests to prevent latency and potential flakiness in CI.
| it('preserves a real saved layout when leaving and returning to the workspace', async () => { | ||
| const storage = TestBed.inject(LocalStorageInteractions); | ||
| const previousLayout = storage.getItem(LocalStorageKey.DOCKVIEW_LAYOUT); | ||
| const manager = fixture.debugElement.injector.get(ComposerDockview); | ||
| const errorSpy = vi.spyOn(console, 'error'); | ||
| let returningFixture: ComponentFixture<ComposerWorkspace> | undefined; | ||
|
|
||
| try { | ||
| manager.setPanelTitle(ComposerPanelId.Rendered, 'My preview'); | ||
| manager.openPanel(ComposerPanelId.Events); | ||
| await new Promise(resolve => setTimeout(resolve, 1100)); | ||
| const savedLayout = storage.getItem(LocalStorageKey.DOCKVIEW_LAYOUT); | ||
| expect(savedLayout).toBe(JSON.stringify(manager.api.toJSON())); | ||
|
|
||
| fixture.destroy(); | ||
| await new Promise(resolve => setTimeout(resolve, 1100)); | ||
| expect(storage.getItem(LocalStorageKey.DOCKVIEW_LAYOUT)).toBe(savedLayout); | ||
|
|
||
| returningFixture = TestBed.createComponent(ComposerWorkspace); | ||
| returningFixture.detectChanges(); | ||
| await returningFixture.whenStable(); | ||
| const restored = returningFixture.debugElement.injector.get(ComposerDockview); | ||
| expect(restored.api.getGroupPanel(ComposerPanelId.Rendered)?.title).toBe('My preview'); | ||
| expect(restored.api.getGroupPanel(ComposerPanelId.Events)?.api.isActive).toBe(true); | ||
| expect(restored.api.panels.map(panel => panel.id).sort()).toEqual( | ||
| Object.values(ComposerPanelId).sort(), | ||
| ); | ||
| expect(errorSpy).not.toHaveBeenCalled(); | ||
| } finally { | ||
| returningFixture?.destroy(); | ||
| errorSpy.mockRestore(); | ||
| if (previousLayout === null) { | ||
| storage.removeItem(LocalStorageKey.DOCKVIEW_LAYOUT); | ||
| } else { | ||
| storage.setItem(LocalStorageKey.DOCKVIEW_LAYOUT, previousLayout); | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
Using real-time delays (setTimeout with 1100ms) in unit tests introduces unnecessary latency (over 2 seconds per run) and increases the risk of test flakiness in CI environments. Since the layout save operation is debounced by 1000ms, we should use Vitest's fake timers (vi.useFakeTimers() and vi.advanceTimersByTime()) to execute the test synchronously and instantly.
it("preserves a real saved layout when leaving and returning to the workspace", async () => {
vi.useFakeTimers();
const storage = TestBed.inject(LocalStorageInteractions);
const previousLayout = storage.getItem(LocalStorageKey.DOCKVIEW_LAYOUT);
const manager = fixture.debugElement.injector.get(ComposerDockview);
const errorSpy = vi.spyOn(console, "error");
let returningFixture: ComponentFixture<ComposerWorkspace> | undefined;
try {
manager.setPanelTitle(ComposerPanelId.Rendered, "My preview");
manager.openPanel(ComposerPanelId.Events);
vi.advanceTimersByTime(1100);
const savedLayout = storage.getItem(LocalStorageKey.DOCKVIEW_LAYOUT);
expect(savedLayout).toBe(JSON.stringify(manager.api.toJSON()));
fixture.destroy();
vi.advanceTimersByTime(1100);
expect(storage.getItem(LocalStorageKey.DOCKVIEW_LAYOUT)).toBe(savedLayout);
returningFixture = TestBed.createComponent(ComposerWorkspace);
returningFixture.detectChanges();
await returningFixture.whenStable();
const restored = returningFixture.debugElement.injector.get(ComposerDockview);
expect(restored.api.getGroupPanel(ComposerPanelId.Rendered)?.title).toBe("My preview");
expect(restored.api.getGroupPanel(ComposerPanelId.Events)?.api.isActive).toBe(true);
expect(restored.api.panels.map(panel => panel.id).sort()).toEqual(
Object.values(ComposerPanelId).sort(),
);
expect(errorSpy).not.toHaveBeenCalled();
} finally {
returningFixture?.destroy();
errorSpy.mockRestore();
vi.useRealTimers();
if (previousLayout === null) {
storage.removeItem(LocalStorageKey.DOCKVIEW_LAYOUT);
} else {
storage.setItem(LocalStorageKey.DOCKVIEW_LAYOUT, previousLayout);
}
}
});There was a problem hiding this comment.
Fixed in e218a34. The saved-layout test now uses vi.useFakeTimers({toFake: ['setTimeout', 'clearTimeout']}) and await vi.advanceTimersByTimeAsync(1100) in place of the two real 1.1s waits. It has to be the async variant because Dockview emits onDidLayoutChange on a microtask, so the debounce timer is only scheduled after the synchronous call returns. Real timers are restored in the test and again in the scoped afterEach, so a failed assertion can't leak fake timers into later tests.
⚡ A2UI Composer PR PreviewYour automated preview is successfully live (commit
|
Main's preview resilience work (a2ui-project#196) makes the frame follow the renderer's reported content height and asserts it settles on that height. Pinning docked frames to their panel contradicted that, so drop the docked override and let the docked panel scroll instead. The frame keeps its content height and its controls stay reachable without extending beneath the debug panel. Also replace the real-time debounce waits in the saved layout test with fake timers.
|
Synced with main ( One real change came out of the merge. #196 makes the frame follow the renderer's reported content height, and its new settle test asserts that. This branch had pinned docked frames to their panel height, which contradicted it, so I dropped that override. The docked panel now scrolls instead, which keeps Search Cars reachable without hiding it under the debug panel. The containment test checks that directly, and the description and recording are updated to match. The saved-layout unit test also uses fake timers now, in place of the real 1.1s waits. Full e2e: 57 of 60 pass. The 3 failures are |
Main routes the layout-restore failure through ErrorLogger, so the recovery tests now assert the logged entry and its Errors tab row instead of a console error.
|
Synced with the latest main ( #197 routes the layout-restore failure through Full e2e on this head: 63 pass and 2 are flaky. One is the React |
Summary
Returning to Composer restores the saved panel arrangement instead of dropping panels from a valid Dockview snapshot. Validation now reads Dockview's serialized
contentComponentfield. A retired panel or mismatched panel ID rejects the inconsistent snapshot as a whole, and the existing recovery path opens the default workspace.Docked renderer previews no longer hide their controls beneath the adjacent debug panel. The frame keeps following the renderer's reported content height, as main's preview resilience work (#196) requires, and a docked preview taller than its panel now scrolls inside that panel instead of being clipped. The workspace selector also lifts the renderer's 280px minimum height, so a preview without a reported height can fill a short panel.
Recording
Full-resolution MP4. Recorded on
e218a34against the local dev servers at 1280×520: the booking form is taller than its docked panel, the panel scrolls to Search Cars, and the click lands in Events. It then selects Data Model, leaves for Settings, returns, and reloads, and the saved arrangement is restored each time. No API key or live model call is used.Scope and dependencies
This is an independent fix against upstream
main. It contains the workspace corrections previously included with #119's styling. Review and merge it independently of the theme, Gallery, and Settings layouts. The rebuilt #119, #129, #193, #215, #216, and #218 branches include this prerequisite. The theme requires preview containment at compact viewport heights.Validation
Synced with main at
f95459c(#196, #219, #222, #223); head ise218a34.propagates data model changes from shell "Data Model" tab to rendered preview, which fail the same way on unchangedmain(React and Lit fail outright; Angular passed there only on retry).settles preview frame height without a SURFACE_RESIZE feedback looppasses unchanged. The earlier version of this branch pinned docked frames to their panel height, which contradicted it, so that override is gone and the containment test now checks panel scrolling instead: real overflow at 1280×480, the preview host staying inside its Dockview panel, and a Search Cars click that no other panel intercepts.Upstream workflows for this PR await maintainer approval.
Pre-launch Checklist
scripts/e2e_test.shspecifically. Fixture-based production browser checks are recorded above; the live-model integration script was not run.