PDP: split image priority from loading=eager; account for color slot#249
Merged
laugharn merged 1 commit intopdp-aspect-ratio-and-oos-slideshowfrom May 5, 2026
Merged
Conversation
Two priority bugs in ProductMedia:
1. Mobile carousel under-prioritized in color-partitioned mode. The
shared-items map used priority={!children && idx === 0}. Whenever
a mobileSlot was passed (the resolved-color-images Suspense node),
children was always truthy, so no shared item was marked priority
even if the resolved color images turned out to be empty. The
mobile LCP candidate was lazy-loaded.
2. Desktop grid double-prioritized. GridItem used priority={idx < 2}
independently for color images and shared items. With color
partitioning on, the first two color images were priority and the
first two shared items at visual positions 3/4 also got priority,
so four <link rel=preload> ended up competing for a single PDP.
Fix:
- Separate priority (the single LCP candidate; fetchPriority high +
preload) from loading=eager (above-the-fold but not LCP; loads now
without fighting the LCP for bandwidth).
- Compute hasColorSlot once in ProductMedia and pass it down.
- Color slot present: colorImage[0] is priority, colorImage[1] is
eager, shared[0] is eager (covers the case where color resolves to
0 or 1 images).
- No color slot: shared[0] is priority, shared[1] is eager.
MediaImage now takes an explicit eager prop so loading semantics
aren't conflated with priority. MediaVideo's preview image keeps the
combined priority||eager signal — small cost, and the preview is
what affects perceived load.
Adds a template-rollout-log entry.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacks on #248. Targets
pdp-aspect-ratio-and-oos-slideshowso the diff stays scoped to image-loading changes; GitHub will auto-retarget tomainonce #248 merges.Summary
Two priority bugs in
ProductMedia:priority={!children && idx === 0}. Whenever amobileSlotwas passed,childrenwas always truthy, so no shared item was marked priority — even if the resolved color images turned out to be empty. The mobile LCP candidate was lazy-loaded.GridItemusedpriority={idx < 2}independently for color images and shared items, so in color-partitioned mode four<link rel=preload>ended up competing for a single PDP.Fix separates
priority(the single LCP candidate;fetchPriority="high"+ preload) fromloading="eager"(above-the-fold but not LCP; loads now without fighting the LCP for bandwidth), and computes both centrally inProductMediabased on ahasColorSlotflag.Rules:
colorImage[0]priority,colorImage[1]eager,shared[0]eager (covers 0- or 1-color-image cases).shared[0]priority,shared[1]eager.MediaImagenow takes an expliciteagerprop so loading semantics aren't conflated with priority.MediaVideo's preview image keeps the combinedpriority||eagersignal — small cost, and the preview is what drives perceived load.Includes a template-rollout-log entry.
Test plan
pnpm --filter template lintandtsc --noEmitcleanfetchpriority=highrequest with a<link rel=preload>in<head>, second isloading=eager(no preload), rest lazyeager; rest lazyeager)🤖 Generated with Claude Code