Skip to content

feat(studio): let an embedder draw the preview edge to edge with fillBox - #4408

Merged
miguel-heygen merged 2 commits into
mainfrom
fix/studio-preview-fill
Sep 24, 2026
Merged

miguel-heygen merged 2 commits into
mainfrom
fix/studio-preview-fill

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator

What

NLEPreview gets an optional fillBox prop. When it is on, the picture is drawn edge to edge in the preview's box instead of 8px in from each side on a dark band. Without the prop nothing changes.

Why

An app that embeds NLEPreview and sizes its box to the film's shape saw the film smaller than the box, framed by a black band. In a 640x360 box with a 16:9 film, the picture was 611.5x344. There was no way to turn the band off: the inset lived in two places (the inner container's inset-2 and a 16px subtraction in resolvePreviewStageSize), and neither was reachable from props.

How

  • One constant owns the per-side inset (PREVIEW_STAGE_INSET_PX = 8). fillBox sets it to 0.
  • The same value drives the inner container's inset and the stage-size math, so the layout and the sizing cannot drift apart.
  • resolvePreviewStageSize takes the inset as a parameter that defaults to the constant, so existing callers keep their numbers.
  • The viewport's p-2 is removed: every child of the viewport is absolutely positioned, so the padding never affected layout.

Test plan

  • New test in NLEPreview.test.ts: in a 640x360 box, the default stays 611.5556x344 with an 8px inset, and fillBox gives a 640x360 stage with no inset. It passes on this branch (11 passed, exit 0). It fails when fillBox is ignored (exit 1, only this test red).
  • tsc --noEmit on packages/studio, oxlint and oxfmt --check on the changed files: clean.
  • A page that mounts NLEPreview in a 640x360 box (pink outline) with a 16:9 registry example, same URL both times. Measured in the browser: main draws 611.5x344 inside the box; this branch with fillBox draws 640x360. With the prop off, this branch's screenshot is pixel-identical to main.

Before

main: the picture sits 8px in from the box on a black band.

main: 611x344 picture on a black band in a 640x360 box

After

this branch with fillBox: the picture fills the box.

fillBox: 640x360 picture fills the 640x360 box

Review

Independent adversarial review of this head: blocking none, should fix none. Notes: the new test covers a same-shape landscape box without the ruler gutter; the gutter and letterbox paths reuse the same arithmetic (checked by hand, 2 * (inset + gutter) equals the old single subtraction when the inset is 8). Every consumer of the stage geometry (pan and zoom clamp, ruler, deselect outside the frame, drop coordinates) reads live rects, so none assumes the inset.

NLEPreview always drew its picture 8px in from each side on a dark band, so an embedder that sizes the box to the film saw a smaller picture in a black frame. fillBox drops that inset from both the layout and the stage-size math. Default look is unchanged.

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

Approve at 8809db94.

  • Default sizing is unchanged: old viewport - 16 - 2·gutter, new viewport - 2·(8 + gutter), identical for every gutter. resolvePreviewStageSize and PREVIEW_STAGE_INSET_PX have no other callers in packages/, so halving the constant's meaning (per side instead of both sides) touches nothing else.
  • p-2 removal is inert: all three viewport children (the stage container, the zoom HUD, the reset button) are absolute, and absolute offsets resolve against the padding box, so the padding never moved them.
  • One source for the inset: layout (style={{ inset: insetPx }}) and sizing now share it. A small bonus: the old inset-2 was 0.5rem, so in a host app whose root font size is not 16px the layout and the 16px JS subtraction already disagreed. Now they can't.
  • The ResizeObserver effect re-subscribes on insetPx, so toggling fillBox re-measures.

Tests: NLEPreview.test.ts passes, 11/11. Two mutations, each failing only the new test:

  • ignoring fillBox (insetPx always 8)
  • decoupling the layout inset from the sizing inset (style={{ inset: 8 }} with fillBox on)

— Rames

@miguel-heygen
miguel-heygen merged commit 326df8d into main Sep 24, 2026
54 checks passed
@miguel-heygen
miguel-heygen deleted the fix/studio-preview-fill branch September 24, 2026 11:22
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.

2 participants