Skip to content

feat(studio): NLE shell components — context provider, panes, overlays (unwired)#2204

Open
ukimsanov wants to merge 1 commit into
studio-dnd/pr12-canvas-nudge-mathfrom
studio-dnd/pr13-nle-shell-components
Open

feat(studio): NLE shell components — context provider, panes, overlays (unwired)#2204
ukimsanov wants to merge 1 commit into
studio-dnd/pr12-canvas-nudge-mathfrom
studio-dnd/pr13-nle-shell-components

Conversation

@ukimsanov

@ukimsanov ukimsanov commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

What

the NLE shell layer, unwired: NLEContext (provider extracted from NLELayout), PreviewPane, AssetPreviewOverlay + assetPreviewStore, TimelineOverlays (menus/modal/hint extracted from Timeline), timelineClipChildren, timelineClipDragTypes, useTimelinePlayerLoop (playback loop extracted from useTimelinePlayer).

Why

the extracted replacements for NLELayout/StudioPreviewArea and the oversized Timeline/useTimelinePlayer internals, reviewable standalone.

How

new files only, tsc-clean against main. They intentionally duplicate blocks of their still-alive originals (duplication is warn-level in fallow; the swap PRs delete the originals). Dead-file findings covered by TEMP(studio-dnd) entry registrations, removed at the app-shell swap.

Test plan

tsc --noEmit; bunx vitest run (suite unchanged); fallow audit clean.


Stack position 13/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.

@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.

R1 — Batch C (unwired chrome / assembly)

Verdict: 🟢 approve

Reviewed at ba130cb9. Focus per Phase-2 recon: NLEContext extraction from NLELayout must be behavior-preserving; new components must be truly unwired at merge; fallow entries must be tracked for #2213 removal.

Extraction correctness — 🟢

NLEContext.tsx is a duplicate-then-swap extraction, not an in-place refactor. NLELayout.tsx is untouched at this SHA (confirmed via file-changed set: 10 files, none of them NLELayout.tsx). StudioPreviewArea.tsx:382 continues to mount NLELayout; the pre-swap render path is unchanged. Extraction cannot regress pre-swap consumers because their code path is untouched.

The extracted NLEProvider faithfully copies the provider-shape state from NLELayout: player hook, useCompositionStack, timelineH persistence + restore clamp, composition-loading gate, compIdToSrc fetch/patch, iframe onLoad + MotionPathPlugin preload, previewCompositionSize. The bits NLELayout owns and this provider omits (fullscreen useSyncExternalStore, usePreviewBlockDrop stage-ref, expanded-clip local-coordinate wrappers, Escape-to-pop keyboard, timelineVisible toggle) are picked up downstream — verified expandedParentStart + trackStudioExpandedClipEdit live in #2209's TimelinePane.tsx, and containerRef + Escape handler live in #2209's EditorShellBody.

Unwired assertion — 🟢

Grepped packages/studio/src/**/*.{ts,tsx} at this SHA for consumers of every new symbol; all imports resolve inside the new subtree only:

  • NLEContext / NLEProvider / useNLEContext — consumed only by new PreviewPane.tsx (sibling); no live-tree consumer.
  • AssetPreviewOverlay — consumed only by new PreviewPane.tsx.
  • PreviewPane — no live-tree consumer.
  • TimelineOverlays — no consumer at all at this SHA (wired later).
  • renderClipChildren (from new timelineClipChildren.tsx) — TimelineCanvas.tsx:197 defines a locally-scoped renderClipChildren arrow; the new module-scoped export is not imported by TimelineCanvas. Name shadow, no live consumption.
  • timelineClipDragTypes — no consumer.
  • useTimelinePlayerLoop — no consumer.
  • assetPreviewStore — consumed only by new AssetPreviewOverlay.

Fallow lifecycle — 🟢

Added to .fallowrc.jsonc:

  • unusedExports block: NLEContext.tsx, PreviewPane.tsx, AssetPreviewOverlay.tsx, TimelineOverlays.tsx, timelineClipChildren.tsx, timelineClipDragTypes.ts, useTimelinePlayerLoop.ts, assetPreviewStore.ts — 8 entries, all with the shared TEMP(studio-dnd) comment pointing at studio-dnd/pr22 for removal.
  • complexity ignore block: TimelineOverlays.tsx (1 entry, same TEMP comment).

All 9 entries are on files created in this PR and are removable in one motion at #2213.

Notes for Phase 3 cross-check

🟡 NLEProvider sets DEFAULT_TIMELINE_H = 340 (line 22), while NLELayout uses DEFAULT_TIMELINE_H = 220 (line 80). Both are gated by the persisted readStudioUiPreferences().timelineHeight, so returning users see identical behavior. First-load-after-cutover for users without a persisted preference will jump from 220 to 340. Not a blocker on this unwired PR; flagging for Phase-3 so the #2213 review can confirm this is intentional (CapCut-style default per the comment) vs. an accidental drift.

R1 by Via

@ukimsanov ukimsanov force-pushed the studio-dnd/pr13-nle-shell-components branch from ba130cb to d8c95b9 Compare July 11, 2026 02:56
@ukimsanov ukimsanov force-pushed the studio-dnd/pr12-canvas-nudge-math branch from f45f759 to a15be5a Compare July 11, 2026 02:56
@ukimsanov

Copy link
Copy Markdown
Collaborator Author

Confirming the 340 vs 220 question: intentional. 340 is the CapCut-style taller timeline default the NLE shell was designed and feel-tested with; 220 was the legacy layout's value. First-load-after-cutover users without a persisted preference get the new default deliberately.

@ukimsanov ukimsanov force-pushed the studio-dnd/pr12-canvas-nudge-math branch from a15be5a to f20cc98 Compare July 11, 2026 08:18
@ukimsanov ukimsanov force-pushed the studio-dnd/pr13-nle-shell-components branch from d8c95b9 to 0ab5336 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/pr12-canvas-nudge-math branch from f20cc98 to f3e5bb2 Compare July 11, 2026 10:49
@ukimsanov ukimsanov force-pushed the studio-dnd/pr13-nle-shell-components branch from 0ab5336 to 5d3b082 Compare July 11, 2026 10:49
…s (unwired)

What: the NLE shell layer, unwired: NLEContext (provider extracted from
NLELayout), PreviewPane, AssetPreviewOverlay + assetPreviewStore,
TimelineOverlays (menus/modal/hint extracted from Timeline),
timelineClipChildren, timelineClipDragTypes, useTimelinePlayerLoop
(playback loop extracted from useTimelinePlayer).

Why: the extracted replacements for NLELayout/StudioPreviewArea and the
oversized Timeline/useTimelinePlayer internals, reviewable standalone.

How: new files only, tsc-clean against main. They intentionally duplicate
blocks of their still-alive originals (duplication is warn-level in fallow;
the swap PRs delete the originals). Dead-file findings covered by
TEMP(studio-dnd) entry registrations, removed at the app-shell swap.

Test plan: tsc --noEmit; bunx vitest run (suite unchanged); fallow audit
clean.
@ukimsanov ukimsanov force-pushed the studio-dnd/pr12-canvas-nudge-math branch from f3e5bb2 to b72f457 Compare July 11, 2026 12:51
@ukimsanov ukimsanov force-pushed the studio-dnd/pr13-nle-shell-components branch from 5d3b082 to 17b6c3b Compare July 11, 2026 12:51
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.

3 participants