Skip to content

Hide video pillarboxing on web when the card fits#11118

Open
vineyardbovines wants to merge 1 commit into
mainfrom
app-2100
Open

Hide video pillarboxing on web when the card fits#11118
vineyardbovines wants to merge 1 commit into
mainfrom
app-2100

Conversation

@vineyardbovines

@vineyardbovines vineyardbovines commented Jul 9, 2026

Copy link
Copy Markdown
Member

On web, videos always rendered as a full-width card with black pillarbox bars around the video. This renders portrait videos at their own aspect ratio (matching native) when the resulting card is wide enough, and softens the bars when it isn't.

  • The card keeps the full-width pillarbox when it would be narrower than 280px (measured via onLayout), when the video is taller than 1:2, or when the aspect ratio is missing - a narrow card can't fit the overlay controls, and a ratio-fit card would still show bars for very tall videos (see Video crop on web tweaks #9371).
  • When the pillarbox is shown, a blurred cover-fit copy of the thumbnail fills the bars instead of flat black.

Supersedes #10303.

Tested on the web dev server: portrait 1052x1712 renders at its own ratio on desktop and flips to the blurred full-width card below the 280px floor (both directions on window resize), 381x800 (taller than 1:2) keeps the blurred full-width card, landscape unchanged.

before and after

CleanShot 2026-07-10 at 10 52 27@2x

@linear-code

linear-code Bot commented Jul 9, 2026

Copy link
Copy Markdown

APP-2100

@claude

claude Bot commented Jul 9, 2026

Copy link
Copy Markdown

Claude finished @vineyardbovines's task in 2m 5s —— View job


Review: Hide video pillarboxing on web when the card fits

  • Read .github/claude-review-prompt.md
  • Read PR diff
  • Analyze VideoEmbed/index.web.tsx + ConstrainedImage interaction
  • Post review

Traced the change against ConstrainedImage (src/components/images/AutoSizedImage.tsx:22-63) to confirm how fullBleed/aspectRatio drive the rendered card width. The fullBleed decision logic is sound: cardWidth = containerWidth * min(aspectRatio, 1) correctly reproduces the width that a ratio-fit card ({aspectRatio: constrained}) will actually render at, since the outer paddingTop box height for a portrait video collapses to containerWidth. Landscape/square videos still resolve to a full-width card (cardWidth === containerWidth), so "landscape unchanged" holds. The three fallback conditions (unknown ratio, taller than 1:2, sub-280px) all correctly retain the full-width pillarbox. Nice.

Two non-blocking observations, no blockers:

1. One-frame full-bleed flash on every ratio-fit video mount (non-blocking)

containerWidth starts at 0, so on first render cardWidth = 0 < MIN_CARD_WIDTH forces fullBleed = true. onLayout in react-native-web is backed by ResizeObserver, which fires after the first paint. So a qualifying portrait video (e.g. the 1052×1712 case from the PR description) paints once as a full-width blurred card, then snaps to the narrow ratio-fit card on the next frame.

Because the outer box height is driven by constrained (unchanged by fullBleed), this is a horizontal snap only — no vertical feed layout shift — so it's mild, and some initial guess is unavoidable when the decision depends on a measured width. Flagging it since it's reproducible on every scroll-in. If it reads as janky in practice, seeding containerWidth from a synchronous getBoundingClientRect() in a layout effect (or defaulting to the common case) would remove it.

2. backgroundImage: url(undefined) when thumbnail is absent (minor, pre-existing)

The always-rendered contain layer (index.web.tsx:141-150) is not guarded by embed.thumbnail, unlike the blur backdrop above it. This produces url(undefined), but it matches the prior behavior (the old outer div did the same) and browsers no-op an invalid background URL, so it's not a regression — worth a cheap embed.thumbnail && guard for symmetry with the backdrop if you're already touching this block.

Verified the blur backdrop is clipped correctly (parent card has a.rounded_md + a.overflow_hidden, so scale(1.2) doesn't bleed past the card), aria-hidden is set on the decorative backdrop, and this is web-only (native has its own file). LGTM.

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor
Old size New size Diff
7.87 MB 7.87 MB 604 B (0.01%)

constrained = Math.max(aspectRatio, ratio)
}

const [containerWidth, setContainerWidth] = useState(0)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a nit, but can we use the aspect ratio to calculate width (constrained by a max height, which I think we do on native) so that we don't need to do a FOUC? Could use a heuristic min window width and fall back to letterbox in those cases.

Comment on lines +126 to +140
{fullBleed && embed.thumbnail && (
<div
aria-hidden
style={{
position: 'absolute',
inset: 0,
backgroundImage: `url(${embed.thumbnail})`,
backgroundSize: 'cover',
backgroundPosition: 'center',
filter: 'blur(32px)',
// hide the transparent fade the blur creates at the edges
transform: 'scale(1.2)',
}}
/>
)}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Love this, looks great imo

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