Skip to content

feat(studio): canvas context menu and z-order actions (unwired)#2202

Merged
miguel-heygen merged 1 commit into
studio-dnd/pr10-gesture-characterization-testsfrom
studio-dnd/pr11-canvas-zorder-menu
Jul 12, 2026
Merged

feat(studio): canvas context menu and z-order actions (unwired)#2202
miguel-heygen merged 1 commit into
studio-dnd/pr10-gesture-characterization-testsfrom
studio-dnd/pr11-canvas-zorder-menu

Conversation

@ukimsanov

@ukimsanov ukimsanov commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

What

CanvasContextMenu (right-click menu for canvas selections) and canvasContextMenuZOrder (tie-aware bring-forward/send-backward z-order patch computation) with its test suite. Shipped unwired.

Why

the z-order rules are the substance; mounting is one line in the later overlay swap.

How

new files, compiled against current main. Nothing mounts the menu yet, so .fallowrc.jsonc gains TEMP(studio-dnd) entries (entry registration + ignoreExports) — removed by the app-shell swap PR that wires everything.

Test plan

bunx vitest run canvasContextMenuZOrder.test.ts; tsc --noEmit; fallow audit clean.


Stack position 11/25 — studio NLE overhaul (CapCut-parity timeline + canvas). Graphite manages bases; merge from #2192 upward. Temporary TEMP(studio-dnd) fallow entries (unwired-component windows) are all removed by #2213 (app-shell swap), whose tree is byte-identical to the fully-verified integration branch.

ukimsanov commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@ukimsanov ukimsanov force-pushed the studio-dnd/pr11-canvas-zorder-menu branch from 9d0fa02 to 78c55d1 Compare July 11, 2026 00:55
@ukimsanov ukimsanov force-pushed the studio-dnd/pr10-gesture-characterization-tests branch 2 times, most recently from 3c08ade to ae82816 Compare July 11, 2026 01:15
@ukimsanov ukimsanov force-pushed the studio-dnd/pr11-canvas-zorder-menu branch from 78c55d1 to ad7cd13 Compare July 11, 2026 01:15

@miga-heygen miga-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed as part of the 25-PR NLE overhaul stack. Full stack review on #2205 (the keystone). No blockers on this PR. — Miga

@vanceingalls vanceingalls left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: LGTM (green) — pure math, unwired, correctly gated

A textbook z-order resolver: parse effective z, sort into render order (z asc, DOM asc), move the target one step or to an end, realize with the minimal patch (single between-value on the distinct-z fast path; renumber to 0..n-1 on the ties path). Every dispatch branch is a pure function of the inputs.

The pointed queries

  • canvasContextMenuZOrder is a pure function. packages/studio/src/components/editor/canvasContextMenuZOrder.ts:228–:268resolveZOrderChange(target, action) performs no side effect: it reads the DOM (via getFamily/readEffectiveZIndex/getBoundingClientRect) and returns a patch list. Actual style mutation is the caller's responsibility. Every helper (:45, :52, :98, :156, :192) is a query, not a command. Test-friendly by design.
  • Unwired at merge point. Verified against the tree at head SHA ad7cd1: DomEditOverlay.tsx at this ref does NOT import CanvasContextMenu, canvasContextMenuZOrder, or resolveZOrderChange (grep clean). The .fallowrc.jsonc at :53–:55 also honestly marks CanvasContextMenu.tsx as TEMP(studio-dnd): shipped unwired ahead of the NLE integration; the app-shell swap PR (studio-dnd/pr22) wires the consumers and removes this block. The unwired assertion holds. Batch-C reviewer will separately verify the assertion survives its post-batch state; my scope is the math itself.
  • Render-order sort is deterministic. :156 — (a.zIndex - b.zIndex) || (a.domIndex - b.domIndex). Both keys stable functions of the input; no mutated-output tie-break (avoiding the same oscillation trap #2199 fixed). Test at :242–:258 pins the tie-break for a three-way z=0 case.
  • DOM-order ties handled correctly. The comment at :13–:26 is precise about CSS paint reality (later-in-DOM paints on top at equal z) and the resolver at :78–:82 (paintsAbove) and :156 respects it. The tie-repro tests at :196–:240 cover all four actions × {earlier-in-DOM, later-in-DOM} — every ambiguous case has a deterministic outcome.
  • Non-painting sibling hygiene. :85 excludes <audio>/<script>/<style>/<link>/<meta> from the family; test at :263–:307 pins the invariant using an audio sibling with a leaked z=2 that would otherwise pad the renumber. Nice defensive touch — the qa-clean audio artifact would otherwise skew results.
  • Cross-realm HTMLElement check. :72–:76 uses node.ownerDocument?.defaultView to compare against the element's own realm's HTMLElement, with a nodeType === 1 fallback. Documented at :64–:71 as the fix for iframe-realm silent-empty-family bug. Good.
  • z clamped ≥ 0. :174 (Math.max(0, above.zIndex - 1)) and the renumber path :215–:218 (i starts at 0) both keep the invariant. renumber emits patches only for elements whose z actually changes at :242–:258 confirms.

Nits (non-blocking)

  • readEffectiveZIndex at :52 is exported but never consumed at this SHA — correctly exempted via .fallowrc.jsonc ignoreExports at :90–:99 with a TEMP marker. Would be removed by studio-dnd/pr22. Fine.

R1 by Via

@ukimsanov ukimsanov force-pushed the studio-dnd/pr10-gesture-characterization-tests branch from ae82816 to bd0c221 Compare July 11, 2026 02:56
@ukimsanov ukimsanov force-pushed the studio-dnd/pr11-canvas-zorder-menu branch from ad7cd13 to 97977b2 Compare July 11, 2026 02:56
@ukimsanov ukimsanov force-pushed the studio-dnd/pr10-gesture-characterization-tests branch from bd0c221 to b372711 Compare July 11, 2026 08:18
@ukimsanov ukimsanov force-pushed the studio-dnd/pr11-canvas-zorder-menu branch from 97977b2 to 143e573 Compare July 11, 2026 08:18
@ukimsanov ukimsanov marked this pull request as ready for review July 11, 2026 09:06
@ukimsanov ukimsanov force-pushed the studio-dnd/pr11-canvas-zorder-menu branch from 143e573 to a087215 Compare July 11, 2026 10:49
@ukimsanov ukimsanov force-pushed the studio-dnd/pr10-gesture-characterization-tests branch from b372711 to 267d84c Compare July 11, 2026 10:49
@ukimsanov ukimsanov force-pushed the studio-dnd/pr11-canvas-zorder-menu branch 2 times, most recently from 802df15 to 4d17f3c Compare July 11, 2026 21:08
@ukimsanov ukimsanov force-pushed the studio-dnd/pr10-gesture-characterization-tests branch from 34fba38 to 39720e7 Compare July 11, 2026 21:08
@ukimsanov ukimsanov force-pushed the studio-dnd/pr11-canvas-zorder-menu branch from 4d17f3c to 138ede6 Compare July 11, 2026 22:38
@ukimsanov ukimsanov force-pushed the studio-dnd/pr10-gesture-characterization-tests branch from 39720e7 to 04a063c Compare July 11, 2026 22:38

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current required CI is not clean: preview-regression is failing at head 138ede6. Please rerun/fix and re-request review once green.

@ukimsanov ukimsanov force-pushed the studio-dnd/pr10-gesture-characterization-tests branch from 04a063c to 0afc5ec Compare July 12, 2026 01:03
@ukimsanov ukimsanov force-pushed the studio-dnd/pr11-canvas-zorder-menu branch from 138ede6 to 0d1397b Compare July 12, 2026 01:03
What: CanvasContextMenu (right-click menu for canvas selections) and
canvasContextMenuZOrder (tie-aware bring-forward/send-backward z-order patch
computation) with its test suite. Shipped unwired.

Why: the z-order rules are the substance; mounting is one line in the later
overlay swap.

How: new files, compiled against current main. Nothing mounts the menu yet,
so .fallowrc.jsonc gains TEMP(studio-dnd) entries (entry registration +
ignoreExports) — removed by the app-shell swap PR that wires everything.

Test plan: bunx vitest run canvasContextMenuZOrder.test.ts; tsc --noEmit;
fallow audit clean.
@ukimsanov ukimsanov force-pushed the studio-dnd/pr10-gesture-characterization-tests branch from 0afc5ec to 74d5592 Compare July 12, 2026 01:22
@ukimsanov ukimsanov force-pushed the studio-dnd/pr11-canvas-zorder-menu branch from 0d1397b to 8229961 Compare July 12, 2026 01:22

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed current head and verified the addressed feedback and required behavior. Approval is for this exact head; Graphite mergeability may still be pending.

@miguel-heygen miguel-heygen dismissed their stale review July 12, 2026 02:38

Superseded by current-head re-review; findings verified addressed.

@miguel-heygen miguel-heygen merged commit 711859b into studio-dnd/pr10-gesture-characterization-tests Jul 12, 2026
14 of 15 checks passed
@miguel-heygen miguel-heygen deleted the studio-dnd/pr11-canvas-zorder-menu branch July 12, 2026 02:56
@miguel-heygen miguel-heygen restored the studio-dnd/pr11-canvas-zorder-menu branch July 12, 2026 04:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants