Skip to content

fix(workbench): dock the preview behind Layers in the Edit rail again - #208

Open
lstein wants to merge 3 commits into
mainfrom
fix/restore-preview-edit-rail
Open

fix(workbench): dock the preview behind Layers in the Edit rail again#208
lstein wants to merge 3 commits into
mainfrom
fix/restore-preview-edit-rail

Conversation

@lstein

@lstein lstein commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

PR #180's preview float/dock pair and the floated-window Invoke control were never deleted — commit 30de929 (the right-rail dock fold) removed the preview widget from the Edit preset's right rail (['layers', 'preview', 'gallery', 'image-map', 'queue']['layers']) and its migration rewrote existing projects' legacy default rails to that Layers-only shape. With the preview no longer placed in the rail, its float icon and the floated Invoke titlebar were simply unreachable there.

This PR restores the preview to the Edit rail, and then restores the center-panel float experience the same commit-era work had constrained:

  1. fix(workbench): dock the preview behind Layers in the Edit rail again — the Edit preset ships ['layers', 'preview']; the migration treats the brief Layers-only rail as another shipped shape, so already-swept projects get the preview back on load. Customized rails are untouched.
  2. feat(workbench): float the last center view, and dock it back where it floated from — the float pair no longer refuses the last enabled center view (the emptied surface falls back to the center's fallback view; only the destructive close/toggle keep their refusal), and the float records which region's chrome it was asked from, so floating the preview from the center docks it back into the center — not into the right rail it also belongs to.
  3. fix(workbench): honour an emptied center across reloads, presets, and re-selects — adversarial review found the emptied center was refilled with the default arrangement on reload (injecting a phantom Gallery view) and baked into saved presets; selectRegionWidget could also reveal a floated instance alongside its window. All fixed and pinned by tests.

Test plan

  • pnpm test — 509 files / 7429 tests pass; test:browser — 158 files / 1036 tests pass; test:fixtures pass
  • pnpm lint (oxfmt, oxlint, tsc, architecture) clean; vite build + both performance gates exit 0 against the committed baselines (no re-record needed)
  • Driven in the dev server against the mock backend:
    • Edit preset: Preview docks in the rail (Float control, Invoke logomark while floated, Dock returns it)
    • Edit preset center view Preview: Float from the center chrome → floating window with the Invoke control → Dock returns it to the center, active again
    • Video preset (preview as the sole center view): Float now offered; float → reload → float survives with no phantom view injected; Dock restores it as the active center view
  • Fresh-context adversarial review of each behavior change (hydrate/reconcile ordering, sole-view float persistence, dual-membership dock-back, preset save/apply round trips, boot preload, kept-alive dedup) — confirmed findings fixed; empirical round-trip tests executed in the worktree

Known follow-ups (pre-existing, out of scope here)

  • ensureRightRegion can leave a dangling activeInstanceId when rewriting the pre-image-map default rail, leaving the right panel empty until a tab is clicked.
  • Account-saved Edit preset overrides from the fold era (layoutPresetOverrides[edit] with a Layers-only rail) bypass the rail migration.

🤖 Generated with Claude Code

The right-rail dock fold (30de929) shipped the Edit rail as the Layers
panel alone, and its migration rewrote legacy default rails to that shape —
which removed the preview widget from the rail the float/dock pair and the
floated Invoke control live in. The capability was never deleted, but its
default home was, and existing projects were swept to match.

Ship the preview as the second tab behind Layers again, and treat the
brief Layers-only rail as a shipped shape so projects that already adopted
it get the preview back on load. Customized rails stay untouched.

Co-Authored-By: Claude Code <noreply@anthropic.com>
lstein and others added 2 commits September 3, 2026 22:59
…t floated from

The float pair refused the last enabled center view in both the UI and the
reducer, on the theory that floating it out is the same removal as closing —
the work surface would stand with nothing to show. But a float keeps the view
one dock click away, the emptied surface falls back to the center's fallback
view, and rails already allow floating their last widget (they collapse).
Only the destructive placements — close and toggle — keep their refusal.

One instance may also be a member of several regions (the preview is placed
in the center and a rail), and the float's host-region guess — the first
member region in the region map's order, which persisted projects do not
agree on — could detach the rail's membership when the button clicked was
the center's. The region whose chrome was clicked now rides along in the
action, and the window docks back into it.

Co-Authored-By: Claude Code <noreply@anthropic.com>
… re-selects

Adversarial review of the last-center-view float found two live corruptions
and one invariant gap, each verified by executing the reducer:

- ensureCenterRegion read an explicitly emptied center as missing data and
  refilled the default arrangement on every reload, injecting a Gallery
  view the project never placed — permanently, and into the sync baseline.
  A present center is now authoritative even when empty; the refilled
  pointer still names the floated instance so the boot preload keeps the
  window's chunk ready.
- saveLayoutPreset normalized the live project before snapshotting, so the
  same refill baked the phantom view into a saved preset. Fixed with the
  refill change.
- selectRegionWidget could set a region's active instance to a widget that
  was floating — the rail would render it alongside its window, the exact
  shape openRegionWidget's invariant forbids. Selecting a slot now docks.
- the normalization test fed a shape the reducer never produces; it now
  drives the real persisted sole-view shape through a JSON round trip.

Co-Authored-By: Claude Code <noreply@anthropic.com>
@lstein

lstein commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator Author

Adversarial review

Two contracts this PR introduces are load-bearing and neither is enforced:

  • an empty center is always explained by a live floating window
  • a right rail exactly equal to ['layers'] is an untouched shipped default

Everything below was reproduced by driving the reducer on cf75825 in a worktree, with main as the control. Sequences are given as reducer actions; each one is reachable from shipped UI unless noted.


1. Floating the preview out of the Edit rail and reloading permanently deletes it from the center

reconcileFloatingWidgets strips a floated instance from every region, but dockFloatingWidget restores it only to returnRegion. That balanced while no instance was dual-placed. layoutPresets.ts:218 now places preview in both Edit's center (['canvas','preview']) and Edit's right rail (['layers','preview']), so the two sides no longer balance:

applyPreset('edit')            center=['canvas','preview']  right=['layers','preview']  floating=[]
floatWidget preview from right center=['canvas','preview']  right=['layers']            floating=['preview']
reload (normalizeWorkbenchProject)
                               center=['canvas']            right=['layers']            floating=['preview']   ← center stripped too
dockFloatingWidget preview     center=['canvas']            right=['layers','preview']  floating=[]

Preview is silently and permanently gone from the center view menu; the user has to re-add it through "Add To Center". Note the asymmetry with finding 3 — the rail self-heals on the next load via the new ['layers'] migration entry, but there is no equivalent migration for the center, so the center is the side that always loses the placement, in both float directions. Reconcile needs to remember the regions it stripped (or strip only returnRegion) so dock can put them all back.

2. An emptied center is now permanent, with a dangling pointer and no window to dock

ensureCenterRegion honours any explicitly emptied center, but nothing checks that the emptiness is explained by a live float. Both guards that used to make an empty center unreachable — the refill here and the center branch of reconcileFloatingWidgets — are removed unconditionally, so any path that empties the center for another reason now persists forever:

applyPreset('video')                  center=['preview']  (the sole center view)
floatWidget preview from center       center=[]  activeInstanceId='preview'  floating=['preview']
openRegionWidget right/preview        ← the rail's "add widget" menu; drops the float entry
                                      center=[]  activeInstanceId='preview'  floating={}
reload                                unchanged — center stays [] forever

The instance itself is fine (it is docked in the rail and usable). What is stranded is the center: a permanently empty work surface with a pointer naming a non-member, recoverable only through "Add To Center". None of floatWidget/dockFloatingWidget/selectRegionWidget/openRegionWidget/toggleRegionWidget/moveWidgetInstance push undo, so there is no undo out of it.

moveWidgetInstance from the center produces the same stranded state at the reducer level — on main it self-heals to ['preview','gallery:center'] on reload, on this branch it does not — but it is not currently UI-reachable (WidgetBar.tsx:29 types its region as Exclude<WidgetRegion, 'bottom' | 'center'>, and the center's view switcher is a Menu, not a sortable). Still worth closing, since the repair it removed covered every cause, not just this one.

Narrow fix: keep the empty center only when activeInstanceId names a live floating instance, and refill the default arrangement otherwise. That still passes the reload and preset tests added here.

3. The ['layers'] migration entry makes the Preview tab impossible to close — in any project

LEGACY_EDIT_RIGHT_REGION_WIDGET_IDS is documented as "every Edit rail this app shipped as a default", and the other two entries are 4–5 element rails nobody reproduces by hand. ['layers'] is different: with the new shipped rail ['layers','preview'] it is exactly what closing the Preview tab produces.

applyPreset('edit')                       right=['layers','preview']
toggleRegionWidget right/preview  (the tab's ✕)
                                          right=['layers']
reload                                    right=['layers','preview']   ← back again

And ensureEditRightRegion is applied to every project in normalizeWorkbenchProject (workbenchState.ts:1883) with no layout.presetId check, so any project whose right rail is exactly ['layers'] gets preview injected on load — not only Edit-preset ones.

Shape alone cannot separate "swept by the fold" from "the user closed one tab". This needs a one-shot marker (a persisted schema/migration version), or the ['layers'] entry has to go and already-swept projects keep what they have.

4. selectRegionWidget discards a float without honouring returnRegion

The new destructure drops the floating entry but never returns the instance to the region it floated from:

applyPreset('edit')
floatWidget preview from right     right=['layers'], float returnRegion='right'
selectRegionWidget center/preview  ← the center view menu (revealWidgetPlacement)
                                   floating.preview gone, right stays ['layers']

In the Edit preset finding 3 accidentally masks this (the next reload re-injects the rail tab); in Automate — which already dual-places preview — nothing masks it. This is the same dual-membership hazard the returnRegion hint in commit 2 exists to fix; the dock-back path honours it and this one doesn't. Re-inserting into returnRegion at returnIndex when action.region !== returnRegion closes it.

5. The shipped Edit preset now makes the same instance mount twice, live

withoutInstancesShownElsewhere keeps an id when id === activeId and only filters ids active elsewhere. With preview in both Edit regions: click Preview in the right rail (right.active='preview'), then pick Preview in the center view menu (center.active='preview') — CenterArea keeps it (id === its activeId) and the panel slot keeps it (id === its activeId), so WidgetRendererById instanceId="preview" mounts twice, live, in two trees. That is the exact hazard useMountedInstanceIds' own doc comment says the filter exists to prevent, and its one blind spot. Pre-existing via Automate, but this PR puts it two clicks from the default Edit layout.

6. The empty center renders error copy

CenterArea.tsx:170 renders <FallbackCenterView label="Center widget unavailable" /> — hardcoded, untranslated, and the same component used for a widget that failed to resolve (CenterArea.tsx:427). The PR's comments describe this as "the center's fallback view", which reads as a designed state; it isn't. The designed copy already exists — widgets.centerViewEmpty = "No view" (en.json:1493) — but is only used for the menu trigger at CenterArea.tsx:265. If an empty center is now supported, the surface should say "No view".

7. Nits

  • selectRegionWidget runs its destructure unconditionally, so a project that has never floated anything gets floatingWidgets: {} written into autosaved state on the first rail tab click. Benign downstream (areFloatingWidgetsEqual and normalizeFloatingWidgets both normalize empty-vs-undefined, so no false preset drift) but it is churn in the persisted document; only rebuild the map when project.floatingWidgets?.[action.widgetId] exists.
  • reconcileFloatingWidgets: removing the center block leaves let remainingFloating with no remaining reassignment — can be const. Won't fail lint (prefer-const is oxlint style, and .oxlintrc.json enables only categories.correctness plus an explicit list).

Attacks that came back clean

  • Switching to a built-in preset while a widget floatsapplyLayoutPresetToProject reconciles against the snapshot's floats, not the project's, so the new preset's center is always populated. No blank surface.
  • The rail migrations re-docking a floated widget on loadreconcileFloatingWidgets runs on ensureEditRightRegion's output and drops it again, as the bottom-rail comment claims. (This is also what causes finding 1.)
  • Dismissing a floating window into nothingFloatingWidgetWindow carries only Dock, no close.
  • Emptying the center destructivelycloseWidgetPlacement's last-center-view refusal and the center menu's disabled Close both still hold, including with an already-emptied center (activeItem undefined ⇒ isActiveViewRequired true).
  • Retired-widget pruning emptying the centerwithoutRetiredInstances is applied to the right region only.
  • The canFloat rewrite widening where the button appearsisWidgetRegion excludes 'floating', so it is strictly narrower than the old region !== 'floating' test apart from the intended center relaxation.
  • Center rendering a non-member activeInstanceIdactiveCenterViewId is derived from centerViewItems (membership), so a dangling pointer renders nothing rather than a ghost.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant