fix(studio): put the timeline's portaled surfaces on the tier the other portals use

[vanceingalls]

Stacked on #3413. Part of restoring fixes stranded in the unreviewed #3363 — see #3413 for the full context.

What

The FX popover, the grouping dialog it swaps for, and the automation selection menu are all portaled to document.body, so they land in the root stacking context — where they sat at z-50.

main's own chrome occupies 60, 90, 91, 92, 94, 100 and 110, and every other portal that has to clear that chrome already uses z-[200]:

• components/ui/Tooltip.tsx
• components/sidebar/AssetContextMenu.tsx
• components/editor/InlineTextToolbar.tsx
• components/renders/RenderQueue.tsx

These three were the odd ones out.

Scoped honestly

The clipping in the original report is fixed by the height cap in #3413 — that is what actually cut the popover off at the timeline chrome.

This commit is tier consistency: it removes the standing risk of a portaled timeline surface losing to any of those seven higher tiers. It is not backed by a reproduced stacking failure on main. The original fix's message attributed the clash to "the ruler's sticky header at z-70", but z-70 is not a literal class in either tree — main's ruler is sticky top-0 flex with no z-index at all. Worth a hit test against a real window before claiming more than tier alignment.

Checks

tsc --noEmit clean · 14 tests across the three touched components pass · oxlint / oxfmt clean · fallow audit no issues.

🤖 Generated with Claude Code

[jrusso1020]

First review, no prior reviews or comments. Read the three changed files plus the four cited siblings, the dismiss hook, and every z- tier in packages/studio/src at e90f3c2e88.

Strengths

Every premise the body rests on holds, including the ones that would have quietly invalidated the change. All three surfaces really do portal to document.body (AutomationSelectionMenu.tsx:74, TimelineFxButton.tsx:98 and :139), so the root-stacking-context argument is real rather than assumed; all three are positioned (fixed in the class, or position: "fixed" in the style object), so z-index actually applies to them; and each of the four cited siblings carries z-[200] and no other tier. The seven chrome tiers are exactly right too — 60, 90, 91, 92, 94, 100 and 110 are all present.

The self-debunk is the best part of this PR and I would keep it verbatim. There is no z-70 literal anywhere in packages/studio at this head, so the original fix's attribution to "the ruler's sticky header at z-70" was wrong, and rather than inheriting a plausible-sounding rationale the body checks it and drops it. De-scoping to "tier consistency, not a reproduced failure" is the honest call, and it is what makes the change easy to review.

Important

This rung ran no test, build, typecheck, lint or format job, so its Checks section is a local result only. The full check list here is 13 entries: Detect changes ×3, Preflight (lint + format) ×1 pass and ×2 skipping, Preview parity, WIP, player-perf, preview-regression, regression, plus Perf and regression-shards skipping. #3413, which targets main, gets 45 — including Test, Build, Typecheck, Lint and Format. So "tsc --noEmit clean · 14 tests across the three touched components pass · oxlint / oxfmt clean" has not been re-derived by anything outside your machine.

Two things follow. regression reading pass here is not evidence: regression-shards reads skipping, so the aggregate is passing over shards that never ran. And this is the same shape of silent gap the stack exists to close — the fixes reverted once because nothing external was asserting them. Not your doing, it is how the stack's base and path gating works, but the checkmarks on this rung should not be read as coverage.

Notes

One tier sits above the convention: propertyPanelColor.tsx:331 uses z-[9999]. So "every other portal that has to clear that chrome already uses z-[200]" has an outlier, and 200 is not a ceiling. Nothing to change — it just means the colour picker paints over these three, which is presumably fine.

Checked in the other direction and found nothing, which is worth recording since the body asks for a hit test. Moving 50 → 200 also crosses the modal backdrops at z-[100] and z-[110] (AskAgentModal, LintModal, PromptPreviewModal, and ExternalFileConflictBanner.tsx:35 and :113), so in principle these three now paint over a modal instead of under it. It is not reachable in practice: those overlays only open from an onClick (setReviewing(true) and equivalents), and both the popover (TimelineFxPopover.tsx:81-86) and the automation menu (useContextMenuDismiss, capture-phase pointerdown + mousedown) dismiss on any outside press — so the very press that opens a modal closes the surface first. The one surface that appears with no pointer event, the conflict banner itself, is z-[94]: inside the chrome band you are deliberately clearing, and anchored at top-14 while these three anchor to the timeline.

Verdict: COMMENT
Reasoning: A three-line tier alignment whose every stated premise I verified, with an honestly de-scoped rationale and no reachable regression that I could find; the one substantive gap is that no test, typecheck or lint job ran on this rung, so its checkmarks carry less than they look like they do.

— Rames Jusso

[miguel-heygen]

Strengths: TimelineFxPopover.tsx:108, AutomationSelectionMenu.tsx:44, and TimelineFxButton.tsx:82 use the same z-[200] tier as the established body portals, so the selected tier is coherent.

Blocker — the same contract is still violated by three sibling timeline portals: this change is framed as moving the timeline body portals off z-50, but packages/studio/src/player/components/ClipContextMenu.tsx:51, TrackGapContextMenu.tsx:78, and KeyframeDiamondContextMenu.tsx:99 all still call createPortal(..., document.body) and remain at z-50. They share the exact root-stacking precondition this PR uses to justify the three edits, and all can open over the timeline chrome. The restored fix therefore covers only half of the timeline body portals. Either move the complete set to the common portal tier or provide a concrete stacking reason those three are intentionally below app chrome.

Verdict: REQUEST CHANGES
Reasoning: The tier choice is sound, but the all-sites audit finds three remaining production paths with the same failure class.

— Magi

[jrusso1020]

Additive to @miguel-heygen's blocker — the all-sites audit is right, and it is one site short.

There are four createPortal(…, document.body) surfaces still on z-50, not three. The fourth is packages/studio/src/components/editor/CanvasContextMenu.tsx:215:

• ClipContextMenu.tsx:51 — fixed z-50 …, body portal at :92
• TrackGapContextMenu.tsx:78 — :114
• KeyframeDiamondContextMenu.tsx:99 — :184
• CanvasContextMenu.tsx:215 — :212 / :276

Two things make it belong in the same bucket rather than being a judgement call. Its className is byte-identical to ClipContextMenu.tsx:51 (fixed z-50 bg-neutral-900 border border-neutral-700 rounded-md shadow-lg py-1 min-w-[180px]), and its own header comment at :4-5 says it "Mirrors the look, positioning, and dismiss behavior of player/components/ClipContextMenu.tsx — portaled to document.body". So it is the same contract by the file's own account.

It is easy to miss because it is the only one of the four outside player/components/ — a sweep scoped to the timeline directory finds exactly the three already cited. Worth using createPortal + document.body as the predicate rather than the directory, which also turns up BlocksTab.tsx carrying no z- class at all — probably fine, but the same one-line check settles it.

Whichever way the blocker is resolved, the set to move (or to justify leaving behind) is four.

Verdict: COMMENT
Reasoning: Additive completion of an existing blocker's site list; no separate finding of my own, and nothing here changes that blocker's disposition.

— Rames Jusso

[vanceingalls]

Follow-up so this PR isn't credited with more than it does.

A user report of "the grouping button did nothing" looked at first like it might be this z-order issue — the group-creation dialog is one of the three surfaces raised here. It isn't. Traced in #3417: that dialog was positioning itself raw at top: anchorRect.bottom + 4 with no flip, and the timeline sits at the bottom of the window, so it opened past the viewport edge. Off-screen, not behind chrome.

So this PR's framing stands unchanged: tier consistency, not a reproduced stacking failure. It removes the standing risk of a portaled timeline surface losing to the seven higher chrome tiers; it is not the fix for that report. #3417 is.

[vanceingalls]

Blocker fixed at 6a42079c. The audit was right and it was four sites, not three — thanks for completing the list.

Using createPortal(…, document.body) as the predicate rather than the timeline directory, all four are now z-[200]:

• player/components/ClipContextMenu.tsx:51
• player/components/TrackGapContextMenu.tsx:78
• player/components/KeyframeDiamondContextMenu.tsx:99
• components/editor/CanvasContextMenu.tsx:215

CanvasContextMenu belongs to the set by its own account, as noted: className byte-identical to ClipContextMenu, and a header comment saying it mirrors that file portaled to document.body.

Two body portals deliberately left behind, with the reason:

• sidebar/BlocksTab.tsx:125 portals PromptPreviewModal, which carries its own z-[100]/z-[110] modal tier. A z- class on the portal wrapper would be dead weight.
• RenderQueue.tsx:235 is already z-[200].

And two that look like the set but are not: FileTree.tsx:336 and FileTreeNodes.tsx:103 are fixed z-50 but are not portaled — they render inside the sidebar's own stacking context, so the root-context argument does not reach them and raising them would be an unrelated change. (propertyPanelColor.tsx:331 at z-[9999] stays the outlier above the convention; noted, not touched.)

Crossing the modal backdrops is unreachable for the same reason it was for the first three: all four dismiss on an outside pointerdown, so the press that opens a modal closes the menu first.

One test needed updating: CanvasContextMenu.test.tsx:95 asserted .fixed.z-50 was null to prove the menu did not render. Left as-is it would have passed vacuously against any tier, so it now names the new class and still fails if the menu renders.

On the CI point — correct, and it stands. This rung's checkmarks are still base/path-gated; the studio suite (4378 tests) and both touched test files were run locally on the stack tip, and #3413 is what carries the main-targeting gate.

[miguel-heygen]

Strengths: The all-sites fix now uses the actual contract boundary. CanvasContextMenu.tsx:215, ClipContextMenu.tsx:51, TrackGapContextMenu.tsx:78, and KeyframeDiamondContextMenu.tsx:99 join the three original timeline body portals at z-[200]. An exact-head grep confirms every createPortal(..., document.body) surface is now either on that tier, the color pickers deliberate z-[9999], or a modal that owns its own z-[100]/z-[110] backdrop.

CanvasContextMenu.test.tsx:95 also updates the negative selector, so the no-menu assertion no longer passes vacuously against the tier change. I ran the exact stack tip locally after building workspace dependencies: 29/29 targeted popover/context-menu/caret tests pass.

Verdict: APPROVE
Reasoning: The prior incomplete-sweep blocker is closed at every sibling body portal, and the affected regression surface passes locally at the restacked head.

— Magi

[jrusso1020]

Re-review at 6a42079c22. The all-sites gap is closed — CanvasContextMenu.tsx:215 is now z-[200], and its test selector moved with it (CanvasContextMenu.test.tsx:95), which is the part that is easy to leave behind: that assertion was querying .fixed.z-50 to prove no menu opened, so leaving it would have made it pass for the wrong reason forever.

Rather than check the four sites off a list, I re-derived the predicate at this head. Every createPortal in packages/studio/src — 11 call sites — with its target and tier:

portal → document.body	tier
CanvasContextMenu.tsx:215	z-[200] ✓ this push
ClipContextMenu.tsx:51	z-[200] ✓ this push
KeyframeDiamondContextMenu.tsx:99	z-[200] ✓ this push
TrackGapContextMenu.tsx:78	z-[200] ✓ this push
AutomationSelectionMenu.tsx:44, TimelineFxButton.tsx:82	z-[200] (earlier in this PR)
RenderQueue.tsx:235, Tooltip.tsx:107	z-[200] already — the tier being matched
propertyPanelColor.tsx:331	z-[9999] — pre-existing outlier, above the tier
BlocksTab.tsx:131	no z- on the portaled node

git grep z-50 -- packages/studio/src now returns zero portal-to-body surfaces (11 hits remain, all either absolute — so scoped to their own stacking ancestor — or fixed but not portaled). So the contract in the title is now actually satisfied, which it wasn't at the previous head.

I also checked the thing raising a tier usually breaks, since CanvasContextMenu is newly promoted and the full-screen backdrops sit at z-[100]/z-[110] — i.e. now below these menus. Not reachable: all four use useContextMenuDismiss, which registers pointerdown and mousedown on the capture phase (useContextMenuDismiss.ts:42-44, with matching removal at :46-48), so any press that could open a modal dismisses the menu first. No menu-over-backdrop state exists to see.

nit, out of this PR's contract. FileTreeNodes.tsx:103 is a context menu whose className is nearly identical to the four here (fixed z-50 bg-neutral-900 border border-neutral-700 rounded-md shadow-lg py-1, only min-w-[160px] differs) but it is rendered inline rather than portaled, so it is out of scope for "portaled surfaces" and its z-50 may not even resolve against these — it depends on its ancestors' stacking contexts. Flagging it only so the next person grepping z-50 for this same contract knows it was looked at and deliberately left.

Verdict: COMMENT
Reasoning: The site my last review added is fixed along with its stale test selector, the predicate re-derives to zero remaining portal sites, and the tier promotion introduces no reachable stacking regression.

— Rames Jusso

The base branch was changed.

[vanceingalls]

Restack note, not a code change: #3413 merged as 277fe0fa while this was in flight, so both branches were rebased onto the post-merge main. Rames' comment-interval nit from #3413 (y = 8..40 → y = 8..168 in TimelineFxPopover.test.tsx) missed that merge, so it now rides here as 836922a1 — the merged PR's assertions were already correct, only the comment interval was wrong.

Heads: #3414 836922a1, #3415 3dcb44cf (restacked onto #3414's current tip). Targeted studio tests pass at both.

[miguel-heygen]

Exact-head restack verified: 836922a1 carries the previously approved all-portal tier fix unchanged on current main; the only new PR-local delta corrects the popover regression comment from y=8..40 to y=8..168, matching the already-correct assertions.

Every body-portal site remains on its intended tier, the affected 29-test local surface was green in the prior pass, and all required current-head checks — including Test, Typecheck, Build, both Windows jobs, and the timeline gate — are now green.

Verdict: APPROVE
Reasoning: The restack preserves the reviewed behavior, fixes the sole comment nit, and the current main-targeting matrix is fully green.

— Magi