feat(product): display product videos on the PDP#3044
Conversation
|
Someone is attempting to deploy a commit to the BigCommerce Platform Team on Vercel. A member of the Team first needs to authorize it. |
🦋 Changeset detectedLatest commit: fc32eeb The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
9d8ed7e to
9d4cf20
Compare
|
|
||
| // YouTube ids are short alphanumeric tokens; reject anything else so a malformed | ||
| // path tail or encoded query material can't leak into a built URL. | ||
| const YOUTUBE_ID = /^[\w-]{6,}$/; |
There was a problem hiding this comment.
Is there a library we could potentially use instead of rolling our own embed logic?
There was a problem hiding this comment.
Good question. Quick context first on why some transform is unavoidable, then the library option.
The Storefront API returns a YouTube watch URL, which can't be iframed directly — YouTube blocks /watch in frames, only /embed/<id> is frameable. So inline playback always needs a watch-URL → embed-id step regardless of approach; the real question is whether we own that logic or delegate it.
First-party option: @next/third-parties/google's YouTubeEmbed (it wraps lite-youtube-embed under the hood). It would replace the embed-URL/poster construction here, the click-to-play facade in product-gallery.tsx, and the i.ytimg.com next/image remote pattern — and removes the hand-built iframe src/thumbnail URLs entirely. We'd keep only a small getYouTubeId() parser, since the component takes a videoid and the API gives us a watch URL.
Honestly, neither route is clearly ideal, so this feels like your call on which risk is more acceptable for core:
@next/third-parties— first-party and removes most of the custom/security-sensitive URL building, but it's still marked experimental by Next and adds a runtime dependency tovibes/soul.- Roll our own (current) — keeps
vibes/souldependency-free, but we maintain (and you review) the parsing/embed logic. - Middle ground — depend on
lite-youtube-embeddirectly (stable, not experimental) and wrap the web component ourselves: stable dep, a bit more glue.
A tiny ID parser stays either way. Which trade-off would you prefer — experimental first-party dep, stable third-party dep, or keeping it in-house? Happy to implement whichever you pick.
There was a problem hiding this comment.
I'd go with either @next/third-parties or lite-youtube-embed instead of rolling our own code. I just don't want to roll my own custom video code in Catalyst that we have to maintain.
There was a problem hiding this comment.
Went with lite-youtube-embed directly. The custom embed/facade is gone — the gallery now renders <lite-youtube>, which owns the poster, click-to-play, and the youtube-nocookie iframe. The only thing left is a small getYouTubeId(), since the Storefront API returns watch URLs and the element needs the bare id.
Chose it over @next/third-parties because that wrapper is still flagged experimental, whereas lite-youtube-embed is stable and zero-dependency (and is what @next/third-parties wraps anyway). It's registered client-side only so SSR doesn't trip on the custom element, and each <lite-youtube> remounts when its carousel slide is deselected so playback stops on navigate-away. Scope is YouTube-only, matching BC product-video support.
Pushed.
f35766b to
dfe13a9
Compare
|
Is there a deployment url I can use to test this? |
Product videos were unsupported on the Catalyst PDP even though the Storefront
GraphQL API exposes them on Product.videos (a { title, url } pair — a YouTube
watch URL). This adds the query and renders videos in a dedicated "Videos"
section below the primary product content (mirroring the Stencil/Cornerstone
product_below_content layout), not inside the image gallery.
- page-data.ts: fetch videos(first: 25) on the PDP product query
- page.tsx: stream videos independently of gallery images; mount <ProductVideos>
below ProductDetail (before related products / reviews)
- ProductVideos: featured lite-youtube player + thumbnail strip that swaps the
featured video, with a Hide/Show toggle (YouTube-only, BC's supported provider)
- lite-youtube: client-only facade (poster + play button; injects the
youtube-nocookie iframe only on click) — no heavy player on PDP view
- video-embed: getYouTubeId() defensive watch-URL parser + poster URL helper
- next.config: allow i.ytimg.com poster thumbnails through next/image
- i18n: videosTitle / hideVideos / showVideos / playVideo / viewVideo (en source)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dfe13a9 to
f8ee729
Compare
Preview deployment: https://adam-native-catalyst.catalyst-sandbox.store/zz-plant-wqxt - Updated PR to reflect that videos will load in their own section rather than in the image gallery. |
| @@ -0,0 +1,5 @@ | |||
| --- | |||
There was a problem hiding this comment.
Can we include a migration section similar to other changesets from the past?
There was a problem hiding this comment.
Added a Migration section to the changeset in fc32eeb.
|
Looks good, just left a minor comment and we need to make sure the link/typecheck passes. |
Address review feedback (changeset should include a migration section like prior changesets). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Pushed On the failing checks: "Lint, Typecheck, and gql.tada" and both bundle jobs fail at |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
What / Why
Product videos were unsupported on the Catalyst PDP even though the Storefront
GraphQL API already exposes them on
Product.videos. The API returns a{ title, url }pair (a YouTube watch URL — YouTube is BigCommerce's supportedproduct-video provider), so two pieces were missing: the query never requested
the field, and there was no layer to turn a watch URL into an embeddable player.
This PR adds both and renders videos in a dedicated section below the primary
product content — mirroring the Stencil/Cornerstone
product_below_contentlayout — rather than mixing them into the image gallery.
Layout
A "Videos" section sits directly below
ProductDetail, before Related Productsand Reviews:
It renders a featured player + horizontal thumbnail strip (clicking a
thumbnail swaps the featured video) with a Hide/Show toggle. This matches the
Cornerstone PDP video structure. The featured player is full section width
(
max-w-screen-2xl) as a starting point.Changes
core/app/.../product/[slug]/page-data.tsvideos(first: 25) { edges { node { title url } } }to the PDP product querycore/app/.../product/[slug]/page.tsxstreamableVideos(independent of the gallery image stream); mount<ProductVideos>belowProductDetailcore/vibes/soul/sections/product-detail/product-videos.tsxlite-youtubeplayer + title, thumbnail strip that swaps the featured videocore/vibes/soul/sections/product-detail/lite-youtube.tsx<lite-youtube>facade wrapper (poster + play button; injects the iframe only on click)core/vibes/soul/sections/product-detail/video-embed.tsgetYouTubeId()(defensive watch-URL parser) +getYouTubePosterUrl()core/vibes/soul/sections/product-detail/lite-youtube-embed.d.tslite-youtube-embedships no types and is imported only for its custom-element side effectcore/messages/en.jsonvideosTitle,hideVideos,showVideos,playVideo,viewVideo(English source)core/next.config.tsi.ytimg.com/vi/**poster thumbnails throughnext/imagecore/package.jsonlite-youtube-embed ^0.3.4.changeset/product-videos-pdp.mdminorbump to@bigcommerce/catalyst-coreDesign notes
(image-only), matches the long-standing Stencil/Cornerstone PDP convention, and
lets videos scale without competing with image pagination / load-more.
lite-youtube-embed: a tiny, dependency-free third-party facade — shows aposter + play button and injects the
youtube-nocookieiframe only on click, sono heavy player loads on PDP view. Registered client-side only (it subclasses
HTMLElementat import, which breaks SSR); inert markup server-side, upgrades onhydration.
getYouTubeId()validates protocol + host + id shape anddrops anything that isn't a YouTube URL, matching BC's supported provider. The
section renders nothing when there are no usable YouTube URLs.
<lite-youtube>is keyed by video id, soswapping videos remounts the player and stops the previous one.
Accessibility
aria-expanded+aria-controlswired to the panel viauseId().aria-pressedfor selected state and labelledaria-labels; theplay control has a visually-hidden label.
Testing
Verified on current
canary(base98d5b87) with a store product carrying 3videos:
toggle hides/shows; thumbnails swap the featured video.
getYouTubeId()verification matrixcore/has no unit-test runner today (tests are Playwright e2e), so the parser iscovered by this manual matrix (a unit test is a ready follow-up — the function is
pure):
https://www.youtube.com/watch?v=dQw4w9WgXcQdQw4w9WgXcQhttps://youtu.be/dQw4w9WgXcQdQw4w9WgXcQhttps://www.youtube.com/embed/dQw4w9WgXcQdQw4w9WgXcQhttps://www.youtube.com/shorts/dQw4w9WgXcQdQw4w9WgXcQhttps://m.youtube.com/watch?v=dQw4w9WgXcQdQw4w9WgXcQhttps://vimeo.com/123456null(non-YouTube host)https://youtube.com.evil.com/watch?v=xnull(host not whitelisted)javascript:alert(1)null(non-http(s) protocol)not a urlnull(parse failure)https://www.youtube.com/watch?v=null(empty id)Migration / risk
Additive — the gallery, image load-more, and the review-form image consumer are
unchanged. CSP isn't affected (Catalyst sets no
frame-src, so the YouTube iframeloads). Branch is based on current
canary(98d5b87); canary's tax-displaylogic is inherited untouched.