PDP: aspect ratio prop on gallery; suppress OOS card hover slideshow#248
Merged
PDP: aspect ratio prop on gallery; suppress OOS card hover slideshow#248
Conversation
Two related polish items on top of the ProductCard aspectRatio work in #233: 1. Apply aspectRatio to the PDP gallery. ProductDetailPage, ProductDetailSection, and ProductMedia now accept an optional aspectRatio: "landscape" | "portrait" | "square" (default "square"). The mobile carousel items, the desktop 2x2 grid items, and the color-image slot helpers pick the matching aspect-* class via the same data-[aspect-ratio=...] selector pattern as the product card. Page-level and color-slot Suspense skeletons also match the chosen aspect to avoid layout shift. The lightbox modal is intentionally left alone; it still fills the viewport with object-contain so the full image is always visible. 2. Skip the hover slideshow on out-of-stock product cards. The OOS overlay is bg-black/60 (translucent), so users hovering an out-of-stock card on lg+ saw images cycling underneath the dim "Sold out" tint. Suppressing ProductCardSlideshow rendering when outOfStock is true is the simplest fix and avoids the wasted next/image requests. aspectRatioClasses is exported from product-card/components.tsx so the PDP gallery and the page-level fallback skeleton reuse the same selector string instead of duplicating it. Adds a template-rollout-log entry for downstream forks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
5 tasks
…249) 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>
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.
Summary
Two related polish items on top of the ProductCard
aspectRatiowork in #233:aspectRatio.ProductDetailPage,ProductDetailSection, andProductMediatake an optionalaspectRatio: "landscape" | "portrait" | "square"(default"square"). The mobile carousel, the desktop 2×2 grid, and the color-image slot helpers pick the matchingaspect-*class via the samedata-[aspect-ratio=…]selector as the product card. Page-level and color-slot Suspense skeletons match the chosen aspect to avoid layout shift. The lightbox modal is intentionally left alone — it still fills the viewport withobject-contain.ProductCardImagepreviously renderedProductCardSlideshowregardless of stock. Because the OOS overlay isbg-black/60(translucent), users hovering an OOS card onlg+saw images cycling underneath the dim "Sold out" tint. Slideshow rendering is now skipped whenoutOfStockis true, which also avoids the wastednext/imagerequests.aspectRatioClassesis exported fromproduct-card/components.tsxso the PDP gallery and the page-level fallback skeleton reuse the same selector string instead of duplicating it.Includes a template-rollout-log entry for downstream forks.
Test plan
pnpm --filter template lintandpnpm --filter template buildcleansquareaspectRatio="portrait"to<ProductDetailPage>inapp/products/[handle]/page.tsx; the mobile carousel and desktop 2×2 grid switch to 3:4aspectRatio="portrait"; lightbox modal still fills the viewport (no aspect frame)lg+; no slideshow images cycle behind the dim overlay🤖 Generated with Claude Code