From c019660692958032ec62f95cbeb9323a35d172e8 Mon Sep 17 00:00:00 2001 From: Oleh Omelchenko Date: Tue, 30 Jun 2026 15:42:22 +0300 Subject: [PATCH] =?UTF-8?q?Editor:=20composition=20wireframe=20=E2=80=94?= =?UTF-8?q?=20pull-out,=20stacking,=20and=20simplify?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .claude/skills/alignment/SKILL.md | 9 +- .../architecture/08-vega-editor-techniques.md | 29 +- .../10-interaction-and-feedback.md | 26 +- docs/ux-second-pass.md | 11 + .../CompositionWireframe.module.css | 152 ++++++++- .../components/CompositionWireframe.test.tsx | 127 +++++++- src/app/components/CompositionWireframe.tsx | 301 ++++++++++++++++-- src/app/components/SpecEditor.tsx | 11 + src/app/services/spec-transform-actions.ts | 73 ++++- src/app/stores/AppStore.ts | 42 ++- src/core/spec-restructure.test.ts | 127 +++++++- src/core/spec-restructure.ts | 126 +++++++- src/core/spec-transforms.ts | 20 ++ 13 files changed, 966 insertions(+), 88 deletions(-) diff --git a/.claude/skills/alignment/SKILL.md b/.claude/skills/alignment/SKILL.md index e51c6d2..2035906 100644 --- a/.claude/skills/alignment/SKILL.md +++ b/.claude/skills/alignment/SKILL.md @@ -171,9 +171,16 @@ role`) or the rule it demonstrates. - **Positional sub-section cross-refs.** Cit high-precision: prefer structural/data-driven hints; lean on the intent front door + smart defaults for positive guidance rather than enumerating bad combinations. +16. **Editor transform-actions reuse an applier, not an inlined skeleton** + (`src/app/services/spec-transform-actions.ts`): a new `run*` action shaped + parse → `build(spec)` → `writeBack` (info toast on null) calls the matching shared + applier — `resolveTarget` (scoped), `applyArrayEdit` (one array), or `applyWholeSpecEdit` + (whole-spec drag/simplify) — never re-inlining the model/parse/writeBack prologue. The + family has grown by copy-paste twice (eng-council; arch 08). + ### Output -16. **Summary**: respond with a summary of changes — choices made due to these instructions, +17. **Summary**: respond with a summary of changes — choices made due to these instructions, choices where multiple approaches existed, and non-obvious architectural assumptions the user should know but might not spot in the diff. If the summary mentions an observation you chose not to fix (rule #8), confirm a `// TODO:` breadcrumb was placed at the code site. diff --git a/docs/architecture/08-vega-editor-techniques.md b/docs/architecture/08-vega-editor-techniques.md index b1ce3c1..691a316 100644 --- a/docs/architecture/08-vega-editor-techniques.md +++ b/docs/architecture/08-vega-editor-techniques.md @@ -314,7 +314,13 @@ thin app-layer services. as a plain _insert_, not redundant nesting), **collapse** the source's emptied container (unwrap a one-child, drop a zero-child, recursing up the chain), and **data-pin** a source's inherited `data` before it changes ancestor (`dataBindingAtPath`, so it never silently rebinds). Degenerate - drops — onto itself, its own ancestor/descendant, or the root — return null. + drops — onto itself, its own ancestor/descendant, or the root — return null. `wrapContainer` is the + complement for a frame-margin drop: it stacks the source against the _whole_ container rather than + beside one view — the root included, lifting spec-level metadata onto the wrapper via + `spec-transforms.concatRootBeside` — pulling a view out into a new full-span row/column. The same + flatten/collapse cleanup runs after. `simplifyStructure` collapses redundant single-child + compositions recursively (a `{hconcat:[v]}` is just `v`; `facet`/`repeat` hold one child by design + and are left alone) — the wireframe's Simplify, returning null when nothing is redundant. **Services (app, store-aware via `getState`):** `spec-transform-actions` (the wrap/simplify/add-view operations and their surfaces), `spec-dataset-hints` (completion, @@ -336,13 +342,20 @@ Decision rules: - **One edit path.** The lightbulb returns a `WorkspaceEdit` (no editor handle); the toolbar and palette use `executeEdits`. Both build the replacement through the same serialize-and-reindent step, bracketed by `pushUndoStop`, so ⌘Z restores the prior text. -- **Wireframe restructuring is one core op + the editor's undo.** Every drag resolves to either - `moveViewTo` (reorder within one container) or `wrapViews` (everything else — wrap, cross-container - move, insert); the wireframe only _requests_ it (`AppStore.requestComposeMove`/`requestComposeWrap`) - and `SpecEditor` applies it through the same `executeEdits` + `pushUndoStop` path as the other - transforms, so a drag is one ⌘Z and the editor stays the single text source. `wrapViews` covers - wrap and insert with one operation because it flattens bare same-orientation nesting afterward; the - drag's edge-zone interaction model lives in [arch 10](10-interaction-and-feedback.md). +- **Transform actions share an applier, never re-inline the skeleton.** Every `run*` action is + parse → `build(spec)` → `writeBack` (toast on null). That prologue lives in a shared applier per + family — `resolveTarget` (scoped), `applyArrayEdit` (one array), `applyWholeSpecEdit` (whole-spec + drag/simplify) — so a new action passes its core call and differs only in scope and feedback. A + fresh `run*` reuses the matching applier rather than copying the model/parse/writeBack lines. +- **Wireframe restructuring is a core op + the editor's undo.** Every drag resolves to `moveViewTo` + (reorder within one container), `wrapViews` (pair, cross-container move, insert), or `wrapContainer` + (pull a view out around a container); the Simplify prompt resolves to `simplifyStructure`. The + wireframe only _requests_ each (`AppStore.requestComposeMove`/`requestComposeWrap`/ + `requestComposeWrapContainer`/`requestComposeSimplify`) and `SpecEditor` applies it through the same + `executeEdits` + `pushUndoStop` path as the other transforms, so a drag is one ⌘Z and the editor + stays the single text source. `wrapViews` covers wrap and insert with one operation because it + flattens bare same-orientation nesting afterward; the drag's zone interaction model lives in + [arch 10](10-interaction-and-feedback.md). - **Composition CodeLens is cursor-scoped.** It follows the view the cursor sits in — `＋ Add view above/below` at the view's edges and `↑/↓ Move` to reorder among its siblings — rather than one fixed button per composition array; an empty composition shows a single `＋ Add view`, diff --git a/docs/architecture/10-interaction-and-feedback.md b/docs/architecture/10-interaction-and-feedback.md index 057e4e7..14ab3b5 100644 --- a/docs/architecture/10-interaction-and-feedback.md +++ b/docs/architecture/10-interaction-and-feedback.md @@ -247,16 +247,22 @@ never by writing the draft text directly — so the wireframe and editor share o the moved box for consecutive moves (so a screen reader re-announces its new position), a **polite** live region states the result, and `aria-keyshortcuts` advertises the keys. _(Council: APG listbox-rearrangeable.)_ -- **Restructure by drag — edge-zone (dock) model.** The nearest edge of the box under the pointer - picks the **axis** (left/right → a row `hconcat`, top/bottom → a column `vconcat`) and **side**. - A drop **along** a sibling's own container reorders within it; a drop **across** it — or onto an - opaque `layer`/`facet`/`repeat` box — pairs the two in a new concat. The hit-test descends only - through `hconcat`/`vconcat` and treats `layer`/`facet`/`repeat`/grid as **opaque** targets (their - children overlap or are data-generated, so the unit is the target, never inside it). A 3px accent - line marks the landing edge; a `wrap` drop also rings + tints the partner box. The drag is a - pointer accelerator over capabilities that stay keyboard-reachable (in-container reorder via - `Alt+↑/↓`; cross-container wrap via the editor's wrap actions), so it adds **no keyboard-only - gap**. Transform invariants in [arch 08](08-vega-editor-techniques.md). +- **Restructure by drag — zone model against the children's box.** Intent is read from where the + pointer falls relative to a row/column's children, not one nearest edge, so each gesture owns a + generous target: the **interior central band reorders** (an insertion slot by main-axis position — + a drag _along_ the block rearranges it anywhere, not only on a sibling's edge); the **cross-axis + frame margin** (a row's top/bottom, a column's left/right — the gutter between frame and children, + or past the block) **pulls the source out** into a new full-span row/column wrapping the whole + container, the root included; a drop onto **a view's far cross edge** pairs the two in a + perpendicular split (`Shift` forces a pair from the centre). The source is the **innermost** view + under the pointer — `beginDrag` stops propagation so a nested ancestor frame, itself draggable, + can't claim the drag (un-stopped, its handler runs last on bubble and wins). The hit-test descends + only through `hconcat`/`vconcat` and treats `layer`/`facet`/`repeat`/grid as **opaque**. Feedback: + a cursor **chip** names the pending action, the target previews it (reorder line, pair half-split, + pull-out band), and every frame's pull-out margins glow faintly while dragging. The drag is a + pointer accelerator over keyboard-reachable capabilities (in-container reorder via `Alt+↑/↓`; + cross-container restructure via the editor's wrap actions), so it adds **no keyboard-only gap**. + Transform invariants in [arch 08](08-vega-editor-techniques.md). **Resolved — pane toggle strip.** The persistent show/hide strip (spec §01A) is a **WAI-ARIA APG `toolbar`** (`role="toolbar"`, `aria-orientation="vertical"`, an `aria-label` such as diff --git a/docs/ux-second-pass.md b/docs/ux-second-pass.md index 5365196..fdc3cb4 100644 --- a/docs/ux-second-pass.md +++ b/docs/ux-second-pass.md @@ -24,6 +24,17 @@ record the resolution into the contract (`docs/architecture/09`+`10` and the rel `storageErrorNotification` and `entityStorageErrorNotification` in `services/storage-errors.ts` and the import-quota copy in `services/transfer.ts`. +- **"Row" vs "column" naming differs between the wireframe's pull-out and pair drags** — the + frame-margin pull-out chip/announcement (`pullLabel`, `commitDrop`) name the new full-span band + by its _spatial_ shape (a `vconcat` slot is "a new row above"; an `hconcat` slot "a new column + left"), while the pair-into-split chip/announcement use the _container_ convention (`hconcat` = + "row", `vconcat` = "column"). Both describe the same axis — pulling a view above a row and + pairing two views vertically are both a `vconcat` — yet one calls it a row and the other a + column. Each reading is locally sensible (a pulled-out band reads as a row; a 2-cell vertical + split reads as a column) but the divergence can confuse. Decide: unify on one vocabulary, or keep + the gesture-specific framing. In `components/CompositionWireframe.tsx` (`pullLabel`, the + `'row'`/`'column'` ternaries in `resolveDrop`/`commitDrop`). + ## Deferred (not design debts, revisit on demand) - **Drag-and-drop field assignment** — chips are click/keyboard-first by design; drag would diff --git a/src/app/components/CompositionWireframe.module.css b/src/app/components/CompositionWireframe.module.css index d7347a1..2489701 100644 --- a/src/app/components/CompositionWireframe.module.css +++ b/src/app/components/CompositionWireframe.module.css @@ -7,7 +7,7 @@ .pop { position: fixed; z-index: 1000; - width: 264px; + width: 300px; max-height: 60vh; overflow: auto; padding: var(--space-4); @@ -36,8 +36,14 @@ border: 1px solid var(--border-strong); background: var(--bg); } +/* The padding is the *pull-out margin*: a frame's gutter, between its border and its + child boxes, is the drop zone that lifts a view out into a new full-span row/column + (vs. a box's own edge, which pairs or reorders). Sized for a comfortable target. */ .container { - padding: var(--space-2); + --pull-gutter: var(--space-5); + --reveal-tint: color-mix(in srgb, var(--accent) 7%, transparent); + position: relative; + padding: var(--pull-gutter); cursor: pointer; } .leaf { @@ -63,26 +69,93 @@ .node[data-dragging] { opacity: 0.4; } -/* Drop indicator (arch 10 §5): a 3px accent line on the edge the dragged box would - land. `wrap` mode additionally rings + tints the target box, signalling the two - pair into a new split rather than just reordering. */ -.node[data-drop-edge='left'] { +/* Reorder (move) indicator (arch 10 §5): a 3px accent line on the target box's edge + marks where the dragged view lands in the sequence. */ +.node[data-drop-mode='move'][data-drop-edge='left'] { box-shadow: inset 3px 0 0 var(--accent); } -.node[data-drop-edge='right'] { +.node[data-drop-mode='move'][data-drop-edge='right'] { box-shadow: inset -3px 0 0 var(--accent); } -.node[data-drop-edge='top'] { +.node[data-drop-mode='move'][data-drop-edge='top'] { box-shadow: inset 0 3px 0 var(--accent); } -.node[data-drop-edge='bottom'] { +.node[data-drop-mode='move'][data-drop-edge='bottom'] { box-shadow: inset 0 -3px 0 var(--accent); } +/* Pair (wrap) indicator: the two views split into a new row/column. The target box is + outlined, and the half the incoming view will take is shaded with a seam line at the + split — so it previews the split, not just "drop here". */ .node[data-drop-mode='wrap'] { - background: var(--accent-soft); + position: relative; outline: 2px solid var(--accent); outline-offset: -2px; } +.node[data-drop-mode='wrap']::after { + content: ''; + position: absolute; + background: var(--accent-soft); + pointer-events: none; +} +.node[data-drop-mode='wrap'][data-drop-edge='top']::after { + inset: 0 0 50% 0; + border-bottom: 2px solid var(--accent); +} +.node[data-drop-mode='wrap'][data-drop-edge='bottom']::after { + inset: 50% 0 0 0; + border-top: 2px solid var(--accent); +} +.node[data-drop-mode='wrap'][data-drop-edge='left']::after { + inset: 0 50% 0 0; + border-right: 2px solid var(--accent); +} +.node[data-drop-mode='wrap'][data-drop-edge='right']::after { + inset: 0 0 0 50%; + border-left: 2px solid var(--accent); +} +/* Pull-out indicator: the dragged view would lift out into a new full-span row/column + here. A shaded band fills the frame's margin on the resolved side, capped by a solid + accent line — the "shaded drop-zone" feedback (vs. the box-edge line of a pair/move). */ +.node[data-drop-pull]::after { + content: ''; + position: absolute; + background: var(--accent-soft); + pointer-events: none; +} +.node[data-drop-pull='top']::after { + inset: 0 0 auto 0; + height: var(--pull-gutter); + border-top: 2px solid var(--accent); +} +.node[data-drop-pull='bottom']::after { + inset: auto 0 0 0; + height: var(--pull-gutter); + border-bottom: 2px solid var(--accent); +} +.node[data-drop-pull='left']::after { + inset: 0 auto 0 0; + width: var(--pull-gutter); + border-left: 2px solid var(--accent); +} +.node[data-drop-pull='right']::after { + inset: 0 0 0 auto; + width: var(--pull-gutter); + border-right: 2px solid var(--accent); +} +/* Reveal-on-drag: while a drag is live, every frame's pull-out margins glow faintly so + the targets are discoverable without hunting — across the container's own axis (a + row's margins are top/bottom, a column's left/right). The active target reads + stronger via the band above. */ +.tree[data-drag-active] .container[data-orientation='horizontal'] { + box-shadow: + inset 0 var(--pull-gutter) 0 var(--reveal-tint), + inset 0 calc(-1 * var(--pull-gutter)) 0 var(--reveal-tint); +} +.tree[data-drag-active] .container[data-orientation='vertical'] { + box-shadow: + inset var(--pull-gutter) 0 0 var(--reveal-tint), + inset calc(-1 * var(--pull-gutter)) 0 0 var(--reveal-tint); +} /* Selection mirrors the library's active-row language (arch 09 §4). */ .node.selected { border-color: var(--accent); @@ -179,3 +252,62 @@ .muted { color: var(--text-placeholder); } + +/* Drag chip: follows the cursor during a drag, naming the pending action ("New row + above", "Pair into a column", "Reorder"). Inverted ink-on-bg for contrast on any + theme; above the popover (z 1000) since it portals to the body. */ +.dragChip { + position: fixed; + z-index: 1100; + pointer-events: none; + padding: 3px 8px; + font-size: 11px; + font-weight: 600; + color: var(--bg); + background: var(--text); + border-radius: var(--radius); + box-shadow: 0 4px 12px rgba(0, 0, 0, 0.35); + white-space: nowrap; +} +.dragChip[data-empty] { + opacity: 0.6; +} + +/* Redundant single-child wrapper: a frame holding one view adds no structure. A soft + warning hairline marks it as cleanable (not broken); the prompt below offers the fix. */ +.node[data-degenerate] { + border-style: dashed; + border-color: var(--support-warning-fg); +} +.warning { + display: flex; + align-items: center; + gap: var(--space-3); + margin-top: var(--space-3); + padding: var(--space-2) var(--space-3); + font-size: 11px; + color: var(--support-warning-fg); + background: color-mix(in srgb, var(--support-warning) 12%, var(--bg)); + border: var(--border-width) solid color-mix(in srgb, var(--support-warning) 40%, var(--bg)); + border-radius: var(--radius); +} +.warning span { + flex: 1; +} +.simplify { + flex-shrink: 0; + font: inherit; + font-weight: 600; + color: var(--accent); + background: none; + border: none; + padding: 2px 4px; + cursor: pointer; +} +.simplify:hover { + text-decoration: underline; +} +.simplify:focus-visible { + outline: 2px solid var(--focus); + outline-offset: 1px; +} diff --git a/src/app/components/CompositionWireframe.test.tsx b/src/app/components/CompositionWireframe.test.tsx index dda893b..c1cba36 100644 --- a/src/app/components/CompositionWireframe.test.tsx +++ b/src/app/components/CompositionWireframe.test.tsx @@ -199,24 +199,26 @@ describe('CompositionWireframe — drag to restructure', () => { vi.restoreAllMocks(); }); - const drag = (fromKey: string, to: { x: number; y: number }) => { + const drag = (fromKey: string, to: { x: number; y: number }, shift = false) => { act(() => { item(fromKey).dispatchEvent( new MouseEvent('pointerdown', { bubbles: true, clientX: 100, clientY: 150 }), ); }); act(() => { - window.dispatchEvent(new MouseEvent('pointermove', { clientX: to.x, clientY: to.y })); + window.dispatchEvent( + new MouseEvent('pointermove', { clientX: to.x, clientY: to.y, shiftKey: shift }), + ); }); act(() => { window.dispatchEvent(new MouseEvent('pointerup', {})); }); }; - test('dropping onto a perpendicular edge wraps the two views into a row', async () => { + test('dropping onto a view’s far cross edge pairs the two into a row', async () => { setEditableSpec(COMPOSED); await renderOpen(); - drag('vconcat|1', { x: 190, y: 50 }); // right edge of the top view + drag('vconcat|1', { x: 190, y: 50 }); // right edge of the top view (cross axis) expect(useAppStore.getState().composeRequest).toMatchObject({ kind: 'wrap', targetPath: ['vconcat', 0], @@ -239,6 +241,123 @@ describe('CompositionWireframe — drag to restructure', () => { }); }); +describe('CompositionWireframe — pull a view out via the frame margin', () => { + const HCON = JSON.stringify({ hconcat: [{ mark: 'point' }, { mark: 'bar' }, { mark: 'line' }] }); + // A row of three boxes inset inside the root frame, leaving a margin (the pull-out + // zone) all around. happy-dom has no layout, so the hit-test rects are stubbed. + const RECTS: Record = { + root: { x: 0, y: 0, w: 300, h: 120 }, + 'hconcat|0': { x: 24, y: 24, w: 80, h: 72 }, + 'hconcat|1': { x: 110, y: 24, w: 80, h: 72 }, + 'hconcat|2': { x: 196, y: 24, w: 80, h: 72 }, + }; + beforeEach(() => { + vi.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockImplementation(function ( + this: HTMLElement, + ): DOMRect { + const r = (this.dataset?.key && RECTS[this.dataset.key]) || { x: 0, y: 0, w: 0, h: 0 }; + const box = { + left: r.x, + top: r.y, + right: r.x + r.w, + bottom: r.y + r.h, + width: r.w, + height: r.h, + x: r.x, + y: r.y, + }; + return { ...box, toJSON: () => ({}) }; + }); + }); + afterEach(() => vi.restoreAllMocks()); + + const drag = (fromKey: string, to: { x: number; y: number }, shift = false) => { + act(() => { + item(fromKey).dispatchEvent( + new MouseEvent('pointerdown', { bubbles: true, clientX: 60, clientY: 60 }), + ); + }); + act(() => { + window.dispatchEvent( + new MouseEvent('pointermove', { clientX: to.x, clientY: to.y, shiftKey: shift }), + ); + }); + act(() => { + window.dispatchEvent(new MouseEvent('pointerup', {})); + }); + }; + + test('dropping in the cross-axis margin pulls the view into a new full-span row', async () => { + setEditableSpec(HCON); + await renderOpen(); + drag('hconcat|0', { x: 150, y: 8 }); // top margin of the row — across its axis + expect(useAppStore.getState().composeRequest).toMatchObject({ + kind: 'wrap-container', + containerPath: [], + sourcePath: ['hconcat', 0], + axis: 'vertical', + side: 'before', + }); + }); + + test('a with-axis margin reorders to that end of the row, not a pull', async () => { + setEditableSpec(HCON); + await renderOpen(); + drag('hconcat|1', { x: 8, y: 60 }); // left margin (along the axis), before every box + expect(useAppStore.getState().composeRequest).toMatchObject({ + kind: 'move', + arrayPath: ['hconcat'], + from: 1, + to: 0, + }); + }); + + test('dropping over a sibling’s central band reorders past it', async () => { + setEditableSpec(HCON); + await renderOpen(); + drag('hconcat|2', { x: 50, y: 70 }); // central band of the first box → before it + expect(useAppStore.getState().composeRequest).toMatchObject({ + kind: 'move', + arrayPath: ['hconcat'], + from: 2, + to: 0, + }); + }); + + test('dropping on a sibling’s top edge stacks the two into a column', async () => { + setEditableSpec(HCON); + await renderOpen(); + drag('hconcat|2', { x: 130, y: 30 }); // top edge of the middle box (cross axis) + expect(useAppStore.getState().composeRequest).toMatchObject({ + kind: 'wrap', + targetPath: ['hconcat', 1], + sourcePath: ['hconcat', 2], + axis: 'vertical', + side: 'before', + }); + }); +}); + +describe('CompositionWireframe — simplify redundant wrappers', () => { + const simplifyButton = () => + Array.from(document.body.querySelectorAll('button')).find((b) => b.textContent === 'Simplify'); + + test('offers Simplify for a single-child composition and dispatches it', async () => { + setEditableSpec(JSON.stringify({ hconcat: [{ mark: 'point' }] })); + await renderOpen(); + const btn = simplifyButton(); + expect(btn).toBeTruthy(); + act(() => btn!.click()); + expect(useAppStore.getState().composeRequest).toMatchObject({ kind: 'simplify' }); + }); + + test('no Simplify prompt when every composition has multiple views', async () => { + setEditableSpec(COMPOSED); // a vconcat of two + await renderOpen(); + expect(simplifyButton()).toBeUndefined(); + }); +}); + describe('markIconName', () => { test('maps marks to glyphs, collapsing synonyms', () => { expect(markIconName('bar')).toBe('mark-bar'); diff --git a/src/app/components/CompositionWireframe.tsx b/src/app/components/CompositionWireframe.tsx index ee76eb8..c8f6cb0 100644 --- a/src/app/components/CompositionWireframe.tsx +++ b/src/app/components/CompositionWireframe.tsx @@ -10,17 +10,23 @@ * is the WAI-ARIA APG **tree** widget — `tree`/`treeitem`/`group`, single-select, * roving tabindex, arrow-key nav in logical (document) order (arch 10 §5). * - * On the editable draft a view can be **restructured by dragging its box**: - * - onto a sibling's edge *along* its container → reorder within it; - * - onto a view's edge *across* its container → pair the two in a new - * `hconcat`/`vconcat`, or, dragging in from elsewhere, move/insert it there. - * The nearest edge of the box under the pointer picks the axis (left/right → a row, - * top/bottom → a column) and side. The keyboard equivalent for in-container reorder - * is Alt+↑/↓ (the APG rearrangeable-listbox pattern); cross-container restructuring - * stays the editor's wrap actions for keyboard users. Every move is applied by the - * editor (which owns the undoable edit) via `AppStore.requestComposeMove` / - * `requestComposeWrap`, focus follows the affected box, and a polite live region - * announces the result (arch 10 §5). + * On the editable draft a view can be **restructured by dragging its box**. Intent is + * read from where the pointer falls against a row/column's children, not one nearest + * edge (arch 10 §5): + * - the interior **central band** reorders within the container; + * - the **cross-axis frame margin** (a row's top/bottom, a column's left/right, or + * past the block) **pulls the source out** into a new full-span row/column wrapping + * the whole container — the root included; + * - a view's **far cross edge** pairs the two in a perpendicular `hconcat`/`vconcat` + * (`Shift` forces a pair from the centre); a with-axis drop moves/inserts there. + * Opaque `layer`/`facet`/`repeat`/grid boxes keep the simpler nearest-edge model, and a + * redundant single-child wrapper is flagged with a one-click **Simplify**. The keyboard + * equivalent for in-container reorder is Alt+↑/↓ (the APG rearrangeable-listbox pattern); + * cross-container restructuring stays the editor's wrap actions for keyboard users. Every + * edit is applied by the editor (which owns the undoable edit) via + * `AppStore.requestComposeMove`/`requestComposeWrap`/`requestComposeWrapContainer`/ + * `requestComposeSimplify`, focus follows the affected box, and a polite live region + * announces the result. */ import { @@ -36,6 +42,7 @@ import { import { createPortal } from 'react-dom'; import { isPrefixPath, type SpecPath } from '@core/spec-insert'; import type { DropAxis } from '@core/spec-restructure'; +import { ARRAY_COMPOSITIONS } from '@core/spec-transforms'; import { viewTree, type Orientation, type ViewNode } from '@core/spec-view-tree'; import { usePopover } from '../hooks/usePopover'; import { useAppStore } from '../stores/AppStore'; @@ -54,9 +61,31 @@ const DESCENDABLE: ReadonlySet = new Set(['horizontal' type Edge = 'left' | 'right' | 'top' | 'bottom'; +/** + * Fraction of a view's cross-axis size, at each end, that reads as "pair here": drop a + * view onto another's far edge (a row view's top/bottom, a column view's left/right) to + * stack the two in a perpendicular split. The central band stays reorder, so an ordinary + * along-the-axis drag rearranges. (Shift pairs from the central band too.) + */ +const PAIR_BAND = 0.25; + /** A DOM-safe, unique key for a node from its path. */ const keyOf = (path: SpecPath): string => (path.length ? path.join('|') : 'root'); +// TODO: vocabulary diverges from the pair labels — pull-out names a vconcat slot "a row" +// (spatial: a full-span band) and an hconcat slot "a column", while the pair-into labels and +// the commit announcements use the container convention (hconcat = "row", vconcat = "column"). +// Both describe the same axis; unify or keep the gesture-specific framing (docs/ux-second-pass.md). +/** The pull-out action label for the margin a drop landed in. */ +const pullLabel = (edge: Edge): string => + edge === 'top' + ? 'New row above' + : edge === 'bottom' + ? 'New row below' + : edge === 'left' + ? 'New column left' + : 'New column right'; + /** A readable path like `vconcat[1].hconcat[0]` for the caption. */ function pathLabel(path: SpecPath): string { if (path.length === 0) return 'root'; @@ -115,7 +144,9 @@ function edgeOf(r: DOMRect, x: number, y: number): Edge { interface DropResolution { targetKey: string; edge: Edge; - mode: 'move' | 'wrap'; + mode: 'move' | 'wrap' | 'pull'; + /** A short human label for what the drop will do — shown in the drag chip. */ + label: string; commit: | { kind: 'move'; arrayPath: SpecPath; from: number; to: number } | { @@ -124,6 +155,13 @@ interface DropResolution { sourcePath: SpecPath; axis: DropAxis; side: 'before' | 'after'; + } + | { + kind: 'wrap-container'; + containerPath: SpecPath; + sourcePath: SpecPath; + axis: DropAxis; + side: 'before' | 'after'; }; } @@ -131,18 +169,34 @@ interface DragState { sourceKey: string; sourceNode: ViewNode; resolution: DropResolution | null; + /** Live pointer position, for the drag chip that follows the cursor. */ + pointer: { x: number; y: number }; } function WireframeTree({ tree }: { tree: ViewNode }) { const requestRevealView = useAppStore((s) => s.requestRevealView); const requestComposeMove = useAppStore((s) => s.requestComposeMove); const requestComposeWrap = useAppStore((s) => s.requestComposeWrap); + const requestComposeWrapContainer = useAppStore((s) => s.requestComposeWrapContainer); + const requestComposeSimplify = useAppStore((s) => s.requestComposeSimplify); // Restructure only on the editable draft — the published view is read-only. const editable = useSnippetStore((s) => s.editorView === 'draft' && s.activeSnippetId !== null); const flat = useMemo(() => flatten(tree), [tree]); const parentByKey = useMemo(() => new Map(flat.map((f) => [f.key, f.parent])), [flat]); const rootKey = keyOf(tree.path); + // Redundant single-child wrappers — a `{hconcat: [oneView]}` is just that view (a + // layer/concat of one is the same). Flagged on the boxes and offered up for a one-click + // Simplify. (facet/repeat hold one child by design, so they're never array-compositions.) + const degenerateKeys = useMemo(() => { + const keys = new Set(); + for (const { node, key } of flat) { + if (node.op && ARRAY_COMPOSITIONS.includes(node.op) && node.children.length === 1) + keys.add(key); + } + return keys; + }, [flat]); + const [selectedKey, setSelectedKey] = useState(null); const [focusedKey, setFocusedKey] = useState(null); const [hoverKey, setHoverKey] = useState(null); @@ -229,29 +283,151 @@ function WireframeTree({ tree }: { tree: ViewNode }) { ); const resolveDrop = useCallback( - (x: number, y: number, source: ViewNode): DropResolution | null => { + (x: number, y: number, source: ViewNode, pair: boolean): DropResolution | null => { const sourcePath = source.path; const target = dropTargetNode(x, y, keyOf(sourcePath)); if (!target) return null; + + // The row/column container we're acting within, and the child under the pointer + // (null when the pointer fell in the container's margin or an inter-child gap). + const targetIsRowCol = + target.kind === 'composition' && + !!target.orientation && + DESCENDABLE.has(target.orientation); + const container = targetIsRowCol ? target : (parentByKey.get(keyOf(target.path)) ?? null); + const childUnder = targetIsRowCol ? null : target; + + // Inside a row/column, intent is read against the children's bounding box: reorder + // is the default for the interior central band (so a drag along the block just + // rearranges it); a pull-out needs the pointer past the children on the *cross* axis + // (the frame margin or beyond the block); dropping onto a view's *far cross edge* + // pairs the two into a nested split. (Opaque layer/grid contexts keep the + // nearest-edge model further down.) + if (container?.orientation && DESCENDABLE.has(container.orientation) && container.op) { + const horizontal = container.orientation === 'horizontal'; + const arrayPath: SpecPath = [...container.path, container.op]; + const kids = container.children + .map((c) => ({ node: c, rect: rectOf(keyOf(c.path)) })) + .filter((k): k is { node: ViewNode; rect: DOMRect } => k.rect != null); + if (!kids.length) return null; + const bbox = { + left: Math.min(...kids.map((k) => k.rect.left)), + top: Math.min(...kids.map((k) => k.rect.top)), + right: Math.max(...kids.map((k) => k.rect.right)), + bottom: Math.max(...kids.map((k) => k.rect.bottom)), + }; + + // 1) Pull-out — pointer past the children on the cross axis. Pulling a node into + // its own descendant is the one degenerate case. + const before = horizontal ? y < bbox.top : x < bbox.left; + const after = horizontal ? y > bbox.bottom : x > bbox.right; + if (before || after) { + if (isPrefixPath(sourcePath, container.path)) return null; + const edge: Edge = horizontal ? (before ? 'top' : 'bottom') : before ? 'left' : 'right'; + return { + targetKey: keyOf(container.path), + edge, + mode: 'pull', + label: pullLabel(edge), + commit: { + kind: 'wrap-container', + containerPath: container.path, + sourcePath, + axis: horizontal ? 'vertical' : 'horizontal', + side: before ? 'before' : 'after', + }, + }; + } + + // 2) Pair — drop onto a view's far cross edge (a row view's top/bottom, a column + // view's left/right) to nest the two in a perpendicular split; Shift pairs from + // the central band too. The cross half picks the order. + if (childUnder) { + const cr = rectOf(keyOf(childUnder.path)); + const related = + isPrefixPath(sourcePath, childUnder.path) || isPrefixPath(childUnder.path, sourcePath); + if (cr && !related) { + const cf = horizontal ? (y - cr.top) / cr.height : (x - cr.left) / cr.width; + if (pair || cf < PAIR_BAND || cf > 1 - PAIR_BAND) { + const lead = cf < 0.5; + const axis: DropAxis = horizontal ? 'vertical' : 'horizontal'; + const edge: Edge = horizontal ? (lead ? 'top' : 'bottom') : lead ? 'left' : 'right'; + return { + targetKey: keyOf(childUnder.path), + edge, + mode: 'wrap', + label: `Pair into a ${axis === 'horizontal' ? 'row' : 'column'}`, + commit: { + kind: 'wrap', + targetPath: childUnder.path, + sourcePath, + axis, + side: lead ? 'before' : 'after', + }, + }; + } + } + } + + // 3) Reorder (default) — the insertion slot from the pointer's main-axis position, + // anchored on the child whose edge marks the gap. + const mainPos = horizontal ? x : y; + let index = 0; + for (const k of kids) { + const center = horizontal + ? (k.rect.left + k.rect.right) / 2 + : (k.rect.top + k.rect.bottom) / 2; + if (mainPos > center) index += 1; + } + const atEnd = index >= kids.length; + const anchor = atEnd ? kids[kids.length - 1].node : kids[index].node; + const edge: Edge = horizontal ? (atEnd ? 'right' : 'left') : atEnd ? 'bottom' : 'top'; + + if (keyOf(sourcePath.slice(0, -1)) === keyOf(arrayPath)) { + const from = sourcePath[sourcePath.length - 1] as number; + let to = index > from ? index - 1 : index; + to = Math.max(0, Math.min(to, container.children.length - 1)); + if (to === from) return null; // no-op + return { + targetKey: keyOf(anchor.path), + edge, + mode: 'move', + label: 'Reorder', + commit: { kind: 'move', arrayPath, from, to }, + }; + } + // From another container → insert here, as a with-axis wrap that flattens in. + if (isPrefixPath(sourcePath, anchor.path) || isPrefixPath(anchor.path, sourcePath)) + return null; + return { + targetKey: keyOf(anchor.path), + edge, + mode: 'move', + label: 'Move here', + commit: { + kind: 'wrap', + targetPath: anchor.path, + sourcePath, + axis: horizontal ? 'horizontal' : 'vertical', + side: atEnd ? 'after' : 'before', + }, + }; + } + + // Opaque container (layer/grid) or root: the simpler nearest-edge model. const targetPath = target.path; - // Degenerate: the root, or an ancestor/descendant relationship. if (targetPath.length === 0) return null; if (isPrefixPath(sourcePath, targetPath) || isPrefixPath(targetPath, sourcePath)) return null; - const r = rectOf(keyOf(targetPath)); if (!r) return null; const edge = edgeOf(r, x, y); const axis: DropAxis = edge === 'left' || edge === 'right' ? 'horizontal' : 'vertical'; const side: 'before' | 'after' = edge === 'left' || edge === 'top' ? 'before' : 'after'; - const parent = parentByKey.get(keyOf(targetPath)); const tIdx = targetPath[targetPath.length - 1]; const along = (axis === 'horizontal' && parent?.orientation === 'horizontal') || (axis === 'vertical' && parent?.orientation === 'vertical'); - - // A drop *along* a sibling's own container is a reorder/insert into it; same - // container → a plain move, a different one → a wrap that flattens to an insert. if (along && parent && typeof tIdx === 'number') { const arrayPath = targetPath.slice(0, -1); if (keyOf(sourcePath.slice(0, -1)) === keyOf(arrayPath)) { @@ -264,15 +440,16 @@ function WireframeTree({ tree }: { tree: ViewNode }) { targetKey: keyOf(targetPath), edge, mode: 'move', + label: 'Reorder', commit: { kind: 'move', arrayPath, from, to }, }; } } - // Across the container (or into an opaque target) → wrap the two together. return { targetKey: keyOf(targetPath), edge, mode: 'wrap', + label: along ? 'Move here' : `Pair into a ${axis === 'horizontal' ? 'row' : 'column'}`, commit: { kind: 'wrap', targetPath, sourcePath, axis, side }, }; }, @@ -285,6 +462,14 @@ function WireframeTree({ tree }: { tree: ViewNode }) { const { arrayPath, from, to } = res.commit; const count = parentByKey.get(res.targetKey)?.children.length ?? 0; reorder(arrayPath, from, to, source, count); + } else if (res.commit.kind === 'wrap-container') { + const { containerPath, sourcePath, axis, side } = res.commit; + requestComposeWrapContainer(containerPath, sourcePath, axis, side); + // Focus/selection lands on the container's slot — now the new split holding it. + settleOn( + keyOf(containerPath), + `Pulled ${descriptor(source)} into a new ${axis === 'vertical' ? 'row' : 'column'}`, + ); } else { const { targetPath, sourcePath, axis, side } = res.commit; requestComposeWrap(targetPath, sourcePath, axis, side); @@ -299,15 +484,31 @@ function WireframeTree({ tree }: { tree: ViewNode }) { ); } }, - [parentByKey, reorder, requestComposeWrap, settleOn], + [parentByKey, reorder, requestComposeWrap, requestComposeWrapContainer, settleOn], ); const beginDrag = (e: ReactPointerEvent, source: ViewNode) => { if (e.button !== 0 || !editable) return; + // Only the innermost view under the pointer starts the drag: every nested frame is + // itself draggable, and without this the pointerdown bubbles to each ancestor, whose + // beginDrag runs *after* (bubble order) and overwrites the source — so grabbing a + // child would drag its outer block instead. + e.stopPropagation(); const startX = e.clientX; const startY = e.clientY; let started = false; + // The last pointer + Shift state, so a Shift press/release re-resolves in place + // (Shift forces a pair when the pointer is over a view's central band). + let last = { x: startX, y: startY, pair: e.shiftKey }; + const resolveAt = () => { + setDragState({ + sourceKey: keyOf(source.path), + sourceNode: source, + resolution: resolveDrop(last.x, last.y, source, last.pair), + pointer: { x: last.x, y: last.y }, + }); + }; const onMove = (ev: PointerEvent) => { if (!started) { if (Math.hypot(ev.clientX - startX, ev.clientY - startY) < DRAG_THRESHOLD) return; @@ -315,16 +516,21 @@ function WireframeTree({ tree }: { tree: ViewNode }) { document.body.style.cursor = 'grabbing'; document.body.style.userSelect = 'none'; } - setDragState({ - sourceKey: keyOf(source.path), - sourceNode: source, - resolution: resolveDrop(ev.clientX, ev.clientY, source), - }); + last = { x: ev.clientX, y: ev.clientY, pair: ev.shiftKey }; + resolveAt(); + }; + const onShift = (ev: WindowEventMap['keydown']) => { + if (started && ev.key === 'Shift') { + last = { ...last, pair: ev.type === 'keydown' }; + resolveAt(); + } }; const cleanup = () => { window.removeEventListener('pointermove', onMove); window.removeEventListener('pointerup', onUp); window.removeEventListener('pointercancel', onCancel); + window.removeEventListener('keydown', onShift); + window.removeEventListener('keyup', onShift); document.body.style.cursor = ''; document.body.style.userSelect = ''; }; @@ -345,6 +551,8 @@ function WireframeTree({ tree }: { tree: ViewNode }) { window.addEventListener('pointermove', onMove); window.addEventListener('pointerup', onUp); window.addEventListener('pointercancel', onCancel); + window.addEventListener('keydown', onShift); + window.addEventListener('keyup', onShift); }; const onKeyDown = (e: KeyboardEvent) => { @@ -429,8 +637,11 @@ function WireframeTree({ tree }: { tree: ViewNode }) { className={cls} data-draggable={draggable || undefined} data-dragging={drag?.sourceKey === key || undefined} - data-drop-edge={onTarget?.edge} - data-drop-mode={onTarget?.mode} + data-orientation={n.orientation} + data-degenerate={degenerateKeys.has(key) || undefined} + data-drop-edge={onTarget && onTarget.mode !== 'pull' ? onTarget.edge : undefined} + data-drop-mode={onTarget && onTarget.mode !== 'pull' ? onTarget.mode : undefined} + data-drop-pull={onTarget?.mode === 'pull' ? onTarget.edge : undefined} onPointerDown={draggable ? (e) => beginDrag(e, n) : undefined} onClick={(e) => { e.stopPropagation(); @@ -478,10 +689,27 @@ function WireframeTree({ tree }: { tree: ViewNode }) { role="tree" aria-label="View composition" className={styles.tree} + data-drag-active={drag ? '' : undefined} onKeyDown={onKeyDown} > {renderNode(tree, null)} + {editable && degenerateKeys.size > 0 && ( +
+ + {degenerateKeys.size === 1 + ? 'A single-view wrapper adds no structure.' + : `${degenerateKeys.size} single-view wrappers add no structure.`} + + +
+ )}
{announcement}
+ {/* A chip following the cursor names what the drop will do — the live "what + happens" hint (pointer-only affordance, so aria-hidden; SR users get the + commit announcement above). Portaled out of the clipped popover. */} + {drag && + createPortal( + , + document.body, + )} ); } diff --git a/src/app/components/SpecEditor.tsx b/src/app/components/SpecEditor.tsx index e391f95..7a0ed94 100644 --- a/src/app/components/SpecEditor.tsx +++ b/src/app/components/SpecEditor.tsx @@ -38,8 +38,10 @@ import { installSpecTransformActions, installSpecTransformCodeLens, runMoveViewTo, + runSimplifyStructure, runUnwrap, runWrap, + runWrapContainer, runWrapViews, } from '../services/spec-transform-actions'; import { configureSpecDatasetHints } from '../services/spec-dataset-hints'; @@ -523,6 +525,15 @@ export function SpecEditor() { if (!editor || !composeRequest) return; if (composeRequest.kind === 'move') runMoveViewTo(editor, composeRequest.arrayPath, composeRequest.from, composeRequest.to); + else if (composeRequest.kind === 'simplify') runSimplifyStructure(editor); + else if (composeRequest.kind === 'wrap-container') + runWrapContainer( + editor, + composeRequest.containerPath, + composeRequest.sourcePath, + composeRequest.axis, + composeRequest.side, + ); else runWrapViews( editor, diff --git a/src/app/services/spec-transform-actions.ts b/src/app/services/spec-transform-actions.ts index 018d3d8..e1d4b19 100644 --- a/src/app/services/spec-transform-actions.ts +++ b/src/app/services/spec-transform-actions.ts @@ -41,7 +41,7 @@ import { moveViewTo, type SpecPath, } from '@core/spec-insert'; -import { wrapViews, type DropAxis } from '@core/spec-restructure'; +import { simplifyStructure, wrapContainer, wrapViews, type DropAxis } from '@core/spec-restructure'; import { unwrapSingleton, wrapInConcat, @@ -331,30 +331,30 @@ export function runMoveViewTo( } /** - * Restructure the spec by pairing the dragged `sourcePath` view beside the drop - * `targetPath` view in a new concat of `axis` — the wireframe's cross-container - * drag (wrap, move-in, collapse, all in core/spec-restructure). One undoable edit; - * no toast and no editor focus-steal, as with the reorder path. A degenerate or - * stale drop surfaces an info toast rather than silently doing nothing. + * Apply a whole-spec structural edit: parse the draft, `build` the next spec, and + * write it back as one undoable edit — or surface the "could not restructure" info + * toast when `build` returns null (a degenerate or stale drop). The whole-spec sibling + * of `applyArrayEdit`, for the wireframe's drag/simplify family — no cursor-follow and + * no success toast, since the wireframe gives its own feedback. `notifyOnNull` of false + * stays silent: a no-op (e.g. Simplify with nothing redundant) is not a failure. */ -export function runWrapViews( +function applyWholeSpecEdit( editor: monaco.editor.IStandaloneCodeEditor, - targetPath: SpecPath, - sourcePath: SpecPath, - axis: DropAxis, - side: 'before' | 'after', + build: (spec: JsonObject) => JsonObject | null, + notifyOnNull = true, ): void { const model = editor.getModel(); if (!model) return; const spec = parseSpecObject(model.getValue()); if (!spec) return; - const next = wrapViews(spec, targetPath, sourcePath, axis, side); + const next = build(spec); if (!next) { - notify({ - kind: 'info', - title: 'Could not restructure', - message: 'That drop isn’t possible here — the composition may have changed.', - }); + if (notifyOnNull) + notify({ + kind: 'info', + title: 'Could not restructure', + message: 'That drop isn’t possible here — the composition may have changed.', + }); return; } writeBack( @@ -365,6 +365,45 @@ export function runWrapViews( ); } +/** + * Restructure the spec by pairing the dragged `sourcePath` view beside the drop + * `targetPath` view in a new concat of `axis` — the wireframe's cross-container drag + * (wrap, move-in, collapse, all in core/spec-restructure). + */ +export function runWrapViews( + editor: monaco.editor.IStandaloneCodeEditor, + targetPath: SpecPath, + sourcePath: SpecPath, + axis: DropAxis, + side: 'before' | 'after', +): void { + applyWholeSpecEdit(editor, (s) => wrapViews(s, targetPath, sourcePath, axis, side)); +} + +/** + * Pull the dragged `sourcePath` view out into a new full-span row/column around the + * whole `containerPath` container — the wireframe's frame-margin drag (core + * `wrapContainer`, the complement to the edge-drop `wrapViews`). + */ +export function runWrapContainer( + editor: monaco.editor.IStandaloneCodeEditor, + containerPath: SpecPath, + sourcePath: SpecPath, + axis: DropAxis, + side: 'before' | 'after', +): void { + applyWholeSpecEdit(editor, (s) => wrapContainer(s, containerPath, sourcePath, axis, side)); +} + +/** + * Collapse every redundant single-child composition in the spec (core + * `simplifyStructure`) — the wireframe's Simplify action, offered when it detects a + * `{hconcat: [oneView]}`-style wrapper. A no-op (silent) when nothing is redundant. + */ +export function runSimplifyStructure(editor: monaco.editor.IStandaloneCodeEditor): void { + applyWholeSpecEdit(editor, simplifyStructure, false); +} + /** Insert a view above/below the one the cursor is in (the keyboard path). */ function runInsertRelative( editor: monaco.editor.IStandaloneCodeEditor, diff --git a/src/app/stores/AppStore.ts b/src/app/stores/AppStore.ts index fbeaa0b..5f07fc6 100644 --- a/src/app/stores/AppStore.ts +++ b/src/app/stores/AppStore.ts @@ -75,10 +75,11 @@ export interface AppState { revealTarget: { offset: number; length: number; nonce: number } | null; /** * A request from the composition wireframe to restructure — reorder a view - * within its array (`move`), or pair the dragged view beside a drop target in a - * new concat (`wrap`, the cross-container drag). Applied by the editor (which - * owns the undoable edit) as one ⌘Z step; the nonce makes a repeat re-fire. Null - * until the first. + * within its array (`move`), pair the dragged view beside a drop target in a new + * concat (`wrap`, the cross-container drag), or stack it against a whole container + * pulled out into a new full-span row/column (`wrap-container`, the frame-margin + * drag). Applied by the editor (which owns the undoable edit) as one ⌘Z step; the + * nonce makes a repeat re-fire. Null until the first. */ composeRequest: | { kind: 'move'; arrayPath: SpecPath; from: number; to: number; nonce: number } @@ -90,6 +91,15 @@ export interface AppState { side: 'before' | 'after'; nonce: number; } + | { + kind: 'wrap-container'; + containerPath: SpecPath; + sourcePath: SpecPath; + axis: DropAxis; + side: 'before' | 'after'; + nonce: number; + } + | { kind: 'simplify'; nonce: number } | null; setTheme: (theme: UiTheme) => void; @@ -121,6 +131,15 @@ export interface AppState { axis: DropAxis, side: 'before' | 'after', ) => void; + /** Ask the editor to pull a dragged view out into a new full-span row/column around a container (wireframe margin drag). */ + requestComposeWrapContainer: ( + containerPath: SpecPath, + sourcePath: SpecPath, + axis: DropAxis, + side: 'before' | 'after', + ) => void; + /** Ask the editor to collapse redundant single-child compositions (wireframe Simplify). */ + requestComposeSimplify: () => void; } export const useAppStore = create((set) => ({ @@ -163,4 +182,19 @@ export const useAppStore = create((set) => ({ nonce: (s.composeRequest?.nonce ?? 0) + 1, }, })), + requestComposeWrapContainer: (containerPath, sourcePath, axis, side) => + set((s) => ({ + composeRequest: { + kind: 'wrap-container', + containerPath, + sourcePath, + axis, + side, + nonce: (s.composeRequest?.nonce ?? 0) + 1, + }, + })), + requestComposeSimplify: () => + set((s) => ({ + composeRequest: { kind: 'simplify', nonce: (s.composeRequest?.nonce ?? 0) + 1 }, + })), })); diff --git a/src/core/spec-restructure.test.ts b/src/core/spec-restructure.test.ts index 94d9e9c..74c07ab 100644 --- a/src/core/spec-restructure.test.ts +++ b/src/core/spec-restructure.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { wrapViews } from './spec-restructure'; +import { simplifyStructure, wrapContainer, wrapViews } from './spec-restructure'; describe('wrapViews — wrap across the target axis', () => { it('pairs a view beside a sibling in a new perpendicular concat', () => { @@ -102,3 +102,128 @@ describe('wrapViews — degenerate drops', () => { expect(wrapViews(spec, [], ['vconcat', 1], 'horizontal', 'after')).toBeNull(); }); }); + +describe('wrapContainer — pull a view out into a new full-span row/column', () => { + it('pulls a view out of the root row into a new row above the rest', () => { + // The motivating case: hconcat[A,B,C,D], drop A on the frame's top margin → + // vconcat[A, hconcat[B,C,D]] — impossible with wrapViews (it pairs two views). + const spec = { + data: { name: 'd' }, + hconcat: [{ mark: 'a' }, { mark: 'b' }, { mark: 'c' }, { mark: 'd' }], + }; + const next = wrapContainer(spec, [], ['hconcat', 0], 'vertical', 'before'); + expect(next).toEqual({ + data: { name: 'd' }, + vconcat: [{ mark: 'a' }, { hconcat: [{ mark: 'b' }, { mark: 'c' }, { mark: 'd' }] }], + }); + expect(spec.hconcat).toHaveLength(4); // input untouched + }); + + it('keeps spec-level metadata on the wrapper when wrapping the root', () => { + const spec = { + $schema: 'https://vega.github.io/schema/vega-lite/v5.json', + data: { name: 'd' }, + config: { view: { stroke: null } }, + hconcat: [{ mark: 'a' }, { mark: 'b' }, { mark: 'c' }], + }; + const next = wrapContainer(spec, [], ['hconcat', 2], 'vertical', 'after'); + expect(next).toEqual({ + $schema: 'https://vega.github.io/schema/vega-lite/v5.json', + data: { name: 'd' }, + config: { view: { stroke: null } }, + vconcat: [{ hconcat: [{ mark: 'a' }, { mark: 'b' }] }, { mark: 'c' }], + }); + }); + + it('pulls a view out of a vconcat into a new column beside it', () => { + const spec = { vconcat: [{ mark: 'a' }, { mark: 'b' }, { mark: 'c' }] }; + const next = wrapContainer(spec, [], ['vconcat', 0], 'horizontal', 'before'); + expect(next).toEqual({ + hconcat: [{ mark: 'a' }, { vconcat: [{ mark: 'b' }, { mark: 'c' }] }], + }); + }); + + it('flattens into the parent when the new wrap matches the grandparent axis', () => { + // Drop A on the inner hconcat's top margin: the new vconcat around it is bare and + // same-orientation as the outer vconcat, so it flattens to a plain row above. + const spec = { + vconcat: [{ hconcat: [{ mark: 'a' }, { mark: 'b' }, { mark: 'c' }] }, { mark: 'x' }], + }; + const next = wrapContainer( + spec, + ['vconcat', 0], + ['vconcat', 0, 'hconcat', 0], + 'vertical', + 'before', + ); + expect(next).toEqual({ + vconcat: [{ mark: 'a' }, { hconcat: [{ mark: 'b' }, { mark: 'c' }] }, { mark: 'x' }], + }); + }); + + it('collapses a two-child container down to the lone survivor', () => { + // hconcat[A,B], pull A out above → the hconcat is left with one child, which + // collapses, so the result is the bare vconcat of the two. + const spec = { hconcat: [{ mark: 'a' }, { mark: 'b' }] }; + const next = wrapContainer(spec, [], ['hconcat', 0], 'vertical', 'before'); + expect(next).toEqual({ vconcat: [{ mark: 'a' }, { mark: 'b' }] }); + }); + + it('pins the source’s inherited data when pulled across data contexts', () => { + const spec = { + data: { name: 'root' }, + vconcat: [{ data: { name: 'd2' }, hconcat: [{ mark: 'a' }, { mark: 'b' }] }, { mark: 'c' }], + }; + // Pull A (which inherits d2) out to wrap the whole root vconcat in a column. A is + // pinned to d2 (it would otherwise rebind to root); B's old hconcat collapses to a + // bare unit that keeps d2. + const next = wrapContainer(spec, [], ['vconcat', 0, 'hconcat', 0], 'horizontal', 'before'); + expect(next).toEqual({ + data: { name: 'root' }, + hconcat: [ + { data: { name: 'd2' }, mark: 'a' }, + { vconcat: [{ data: { name: 'd2' }, mark: 'b' }, { mark: 'c' }] }, + ], + }); + }); + + it('returns null when the source is an ancestor of the container', () => { + const spec = { + vconcat: [ + { hconcat: [{ vconcat: [{ mark: 'a' }, { mark: 'b' }] }, { mark: 'c' }] }, + { mark: 'd' }, + ], + }; + // Drag the outer hconcat onto the inner vconcat's margin — the container is nested + // inside the source, a degenerate drop. + expect( + wrapContainer(spec, ['vconcat', 0, 'hconcat', 0], ['vconcat', 0], 'vertical', 'after'), + ).toBeNull(); + }); +}); + +describe('simplifyStructure — collapse redundant single-child wrappers', () => { + it('unwraps a single-child hconcat to the bare view', () => { + const spec = { data: { name: 'd' }, hconcat: [{ mark: 'bar' }] }; + expect(simplifyStructure(spec)).toEqual({ data: { name: 'd' }, mark: 'bar' }); + }); + + it('collapses nested single-child wrappers recursively', () => { + const spec = { vconcat: [{ hconcat: [{ mark: 'a' }] }, { mark: 'b' }] }; + expect(simplifyStructure(spec)).toEqual({ vconcat: [{ mark: 'a' }, { mark: 'b' }] }); + }); + + it('the lone child wins on key conflict, like unwrapSingleton', () => { + const spec = { width: 100, layer: [{ width: 200, mark: 'line' }] }; + expect(simplifyStructure(spec)).toEqual({ width: 200, mark: 'line' }); + }); + + it('returns null when there is nothing to simplify', () => { + expect(simplifyStructure({ hconcat: [{ mark: 'a' }, { mark: 'b' }] })).toBeNull(); + }); + + it('leaves a facet’s single child alone (one child by design)', () => { + const spec = { facet: { field: 'c', type: 'nominal' }, spec: { mark: 'bar' } }; + expect(simplifyStructure(spec)).toBeNull(); + }); +}); diff --git a/src/core/spec-restructure.ts b/src/core/spec-restructure.ts index 0deedcd..ec0ffb0 100644 --- a/src/core/spec-restructure.ts +++ b/src/core/spec-restructure.ts @@ -4,10 +4,14 @@ * drag-to-restructure, the complement to spec-insert (single-array reorder within * one container) and spec-transforms (whole-spec wrap/unwrap). * - * One operation does the work: `wrapViews(target, source, axis, side)` pairs the - * dragged `source` view beside the drop `target` in a new `hconcat`/`vconcat`, - * placed where the target was, and removes the source from where it came. Three - * correctness rules ride along (the traps that make this the hard part): + * Two operations do the work. `wrapViews(target, source, axis, side)` pairs the + * dragged `source` view beside the drop `target` *view* in a new `hconcat`/`vconcat`, + * placed where the target was, and removes the source from where it came. + * `wrapContainer(container, source, axis, side)` is the complement for a drop on a + * frame's *margin*: it stacks the source against the whole `container` (the root + * included) — pulling a view out into a new full-span row/column — rather than + * beside one sibling. Three correctness rules ride along (the traps that make this + * the hard part): * * - **Flatten** — a bare same-orientation concat nested directly in a concat is * redundant, so wrapping a view beside a sibling already in a matching-axis @@ -26,6 +30,7 @@ import { dataBindingAtPath } from './spec-data'; import { isJsonObject, type JsonObject } from './spec-config'; import { arrayAtPath, isPrefixPath, valueAtPath, type SpecPath } from './spec-insert'; +import { ARRAY_COMPOSITIONS, concatRootBeside } from './spec-transforms'; /** The orientation of the concat a drop creates. */ export type DropAxis = 'horizontal' | 'vertical'; @@ -184,3 +189,116 @@ export function wrapViews( flattenBareConcats(next); return next; } + +/** + * Collapse every single-child array-composition (`layer`/`concat`) to its lone + * child (the child wins on key conflict, as `unwrapSingleton`) and drop empty + * ones — recursively over the whole tree, bottom-up. The complement to + * `flattenBareConcats`: it both cleans up after `wrapContainer` pulls a view out + * (removing the source can leave a one- or zero-child composition along its old + * branch) and powers `simplifyStructure`'s global pass. Whole-tree rather than + * path-scoped (unlike `removeAndCollapse`) because re-slotting the container shifts + * the source's old path; the collapse is semantics-preserving, so the wider sweep + * is safe. + */ +function collapseSingletons(node: unknown): unknown { + if (Array.isArray(node)) { + return node + .map(collapseSingletons) + .filter((c) => !(isJsonObject(c) && Object.keys(c).length === 0)); + } + if (!isJsonObject(node)) return node; + const obj: Record = {}; + for (const k of Object.keys(node)) obj[k] = collapseSingletons(node[k]); + for (const op of ARRAY_COMPOSITIONS) { + const arr = obj[op]; + if (!Array.isArray(arr)) continue; + if (arr.length === 1 && isJsonObject(arr[0])) { + const child = arr[0]; + delete obj[op]; + return { ...obj, ...child }; // child wins, like unwrapSingleton + } + if (arr.length === 0) delete obj[op]; + } + return obj; +} + +/** + * Stack the `source` view against the *whole* container at `containerPath` in a new + * concat of `axis` — the frame-margin pull-out. The container (the root included) + * keeps its place; the source becomes a full-span sibling on `side`, removed from + * where it was. The complement to `wrapViews`: there the source pairs with a single + * sibling view, here with the container's entire contents. + * + * The source may sit *inside* the container (the motivating "pull a view out of its + * row into a new row above") or in a sibling subtree. Returns null for a degenerate + * drop (no source/container, or the source is the container or an ancestor of it). + * The root case lifts the spec-level metadata via core/spec-transforms; the input is + * not mutated. + */ +export function wrapContainer( + spec: JsonObject, + containerPath: SpecPath, + sourcePath: SpecPath, + axis: DropAxis, + side: 'before' | 'after', +): JsonObject | null { + // The source can be inside the container or in a sibling subtree, but never the + // container itself or an ancestor of it. + if (isPrefixPath(sourcePath, containerPath)) return null; + + const next = clone(spec); + const source = valueAtPath(next, sourcePath); + const container = valueAtPath(next, containerPath); + if (!isJsonObject(source) || !isJsonObject(container)) return null; + + // The array holding the source today — captured before re-slotting shifts its path. + const srcArr = arrayAtPath(next, sourcePath.slice(0, -1)); + if (!srcArr) return null; + + // Preserve the source's data before its context changes (reads the unmutated tree). + pinData(next, source, sourcePath, containerPath); + + const op = axis === 'horizontal' ? 'hconcat' : 'vconcat'; + let root: JsonObject; + if (containerPath.length === 0) { + // Wrapping the whole spec: shared metadata stays on top, only the view is wrapped. + root = concatRootBeside(container, source, axis === 'horizontal' ? 'h' : 'v', side); + } else { + const wrapper: JsonObject = { + [op]: side === 'before' ? [source, container] : [container, source], + }; + const cParent = valueAtPath(next, containerPath.slice(0, -1)); + const cKey = containerPath[containerPath.length - 1]; + if (Array.isArray(cParent)) cParent[cKey as number] = wrapper; + else if (isJsonObject(cParent)) cParent[cKey as string] = wrapper; + else return null; + root = next; + } + + // Remove the source from its old home (by reference — its path moved under the new + // wrapper), then collapse any container left one-/zero-child and flatten redundant + // same-orientation nesting the wrap produced (e.g. a new row inside its own column). + const si = srcArr.indexOf(source); + if (si >= 0) srcArr.splice(si, 1); + + // `collapseSingletons` runs whole-tree (the re-slot above shifts the source's path, so + // a path-scoped walk can't find it). A deliberate side effect: a pull-out also tidies + // any pre-existing redundant single-child wrapper elsewhere — the same cleanup the + // Simplify action offers. Semantics-preserving, so it's safe to fold into the one edit. + const collapsed = collapseSingletons(root) as JsonObject; + flattenBareConcats(collapsed); + return collapsed; +} + +/** + * Collapse every redundant single-child composition in the spec to its lone child — + * a `{hconcat: [oneView]}` is just that view. Recursive and semantics-preserving: the + * "simplify" the wireframe offers once it spots such wrappers. Returns the cleaned + * spec, or null when there is nothing to simplify. (`facet`/`repeat` hold a single + * child by design and aren't array-compositions, so they're left alone.) + */ +export function simplifyStructure(spec: JsonObject): JsonObject | null { + const next = collapseSingletons(spec) as JsonObject; + return JSON.stringify(next) === JSON.stringify(spec) ? null : next; +} diff --git a/src/core/spec-transforms.ts b/src/core/spec-transforms.ts index 732c5f3..b39ab93 100644 --- a/src/core/spec-transforms.ts +++ b/src/core/spec-transforms.ts @@ -85,6 +85,26 @@ export function wrapInConcat(spec: JsonObject, dir: ConcatDir): JsonObject { return { ...top, [key]: [view, placeholderView()] }; } +/** + * Wrap the whole spec's view in a concat of `dir` placed beside `other`, the + * pulled-out view, on `side` — the root case of the wireframe's frame-margin + * pull-out (core/spec-restructure). Shared metadata ($schema, data, config, …) + * stays on the wrapper exactly as the other whole-spec wraps; unlike them there is + * no placeholder — `other` is a real view the caller then removes from its old + * home and cleans up. The returned tree shares the source view's nested arrays, so + * the caller's removal of `other` from inside it takes effect here too. + */ +export function concatRootBeside( + spec: JsonObject, + other: JsonObject, + dir: ConcatDir, + side: 'before' | 'after', +): JsonObject { + const key = dir === 'h' ? 'hconcat' : 'vconcat'; + const { top, view } = partition(spec, SHARED_TOP); + return { ...top, [key]: side === 'before' ? [other, view] : [view, other] }; +} + /** * Wrap the view in a facet (small multiples across `field`). The caller resolves * a sensible field + type from the bound dataset; the field is left editable.