-
Notifications
You must be signed in to change notification settings - Fork 160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[lightbox-media-viewer]: Auto-open and play media from targeted link #11253
[lightbox-media-viewer]: Auto-open and play media from targeted link #11253
Conversation
Deploy preview created for package Built with commit: c17d244ab79970aa08421788fda42abf657cb37b |
Deploy preview created for package Built with commit: c17d244ab79970aa08421788fda42abf657cb37b |
Deploy preview created for package Built with commit: c17d244ab79970aa08421788fda42abf657cb37b |
I want to test this a little more thoroughly - my main concern at the moment is what happens if multiple CTAs share the same video. Do we get multiple lightboxes? |
Deploy preview created for package Built with commit: c17d244ab79970aa08421788fda42abf657cb37b |
That's a good question. I haven't explored that specifically, that may well be the case. Unless the modal logic has protections against that 🤞 . I can make a note to test that when I'm back in the office next week. |
It does seem to open all matching CTAs this way https://sandbox.andy-blum.com/ibm/auto-open-lightbox/multiple-ctas.html#cta-video-0_ibuqxqbe Edit: demo url should be public 😅 |
@andy-blum I addressed the multiple lightbox openings by only allowing the first in the page to open by that URL trigger. Feels like that should be sufficient. From a content editing perspective, I would imagine they would want to try to avoid duplicating video use for CTA's, otherwise you'd have really keen users running into the same video more than once. There could be use cases for that I guess, hopefully this heuristic would suffice. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One question about how this new behavior fits into the component lifecycle and one minor nit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most recent changes look good. Looking forward to seeing @kennylam & co's takes on this.
1f63335
into
carbon-design-system:main
…11412) ### Related Ticket(s) Closes #11213 [Jira ticket](https://jsw.ibm.com/browse/ADCMS-4360) [v2 PR](#11253) ### Description Enables the `VideoCTAMixin` to check the URL fragment for a particular pattern, `cta-video-[video-id]`, and if present, kick off the CTA trigger, which in the case of a video CTA, is opening the corresponding lightbox and auto-playing video. The effect is that visiting any page with a video CTA for a specific video embedded in it (eg. `0_ibuqxqbe`), with a URL fragment like `#cta-video-0_ibuqxqbe`, will automatically open the lightbox modal and play the video (provided [browser auto-play policies to not block it](https://developer.chrome.com/blog/autoplay/)). ### Testing instructions - [ ] Navigate to any page with a video CTA component. Examples include: CTA > Default - [ ] Set the CTA type (cta-type) to Video (video) - [ ] Take note of the configured Video ID (eg. `0_ibuqxqbe`). - [ ] Click Open canvas in new tab - [ ] The component should work without any regressions - [ ] Append a URL fragment to the URL like `#cta-video-[video_id]` - [ ] When loading the page with the URL fragment, the lightbox should automatically open and begin playback of the video (subject to [browser auto-play policies](https://developer.chrome.com/blog/autoplay/)). Note that mosy likely, the video _won't_ autoplay b/c you probably haven't built up enough of Media Engagement Index (Chrome speak) against the Storybook deploy preview URL. We don't have control over this. ### Changelog **New** - `VideoCTAMixin`, where `ctaType === CTA_TYPE.VIDEO`, will check for a specifically crafted URL fragment (`cta-video-[video-id]`) to trigger it's run action. <!-- React and Web Component deploy previews are enabled by default. --> <!-- To enable additional available deploy previews, apply the following --> <!-- labels for the corresponding package: --> <!-- *** "test: e2e": Codesandbox examples and e2e integration tests --> <!-- *** "package: services": Services --> <!-- *** "package: utilities": Utilities --> <!-- *** "RTL": React / Web Components (RTL) --> <!-- *** "feature flag": React / Web Components (experimental) -->
…arbon-design-system#11253) ### Related Ticket(s) Closes carbon-design-system#11213 [Jira ticket](https://jsw.ibm.com/browse/ADCMS-4360) ### Description Enables the `CTAMixin` to check the URL fragment for a particular pattern, `cta-video-[video-id]`, and if present, kick off the CTA trigger, which in the case of a video CTA, is opening the corresponding lightbox and auto-playing video. The effect is that visiting any page with a video CTA for a specific video embedded in it (eg. `0_ibuqxqbe`), with a URL fragment like `#cta-video-0_ibuqxqbe`, will automatically open the lightbox modal and play the video (provided [browser auto-play policies to not block it](https://developer.chrome.com/blog/autoplay/)) ### Testing instructions - [ ] Navigate to any page with a CTA component that allows you to configure it as a video component. Examples include: [Link with icon](https://ibmdotcom-webcomponents.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/11253/index.html?path=/story/components-link-with-icon--default) - [ ] Set the CTA type (cta-type) to Video (video) - [ ] Take note of the configured Video ID (eg. `0_ibuqxqbe`). - [ ] Click Open canvas in new tab - [ ] The component should work without any regressions - [ ] Append a URL fragment to the URL like `#cta-video-[video_id]` - [ ] When loading the page with the URL fragment, the lightbox should automatically open and begin playback of the video (subject to [browser auto-play policies](https://developer.chrome.com/blog/autoplay/)). Note that mosy likely, the video _won't_ autoplay b/c you probably haven't built up enough of Media Engagement Index (Chrome speak) against the Storybook deploy preview URL. We don't have control over this. ### Changelog **New** - `CTAMixin`, where `ctaType === CTA_TYPE.VIDEO`, will check for a specifically crafted URL fragment (`cta-video-[video-id]`) to trigger it's run action. <!-- React and Web Component deploy previews are enabled by default. --> <!-- To enable additional available deploy previews, apply the following --> <!-- labels for the corresponding package: --> <!-- *** "test: e2e": Codesandbox examples and e2e integration tests --> <!-- *** "package: services": Services --> <!-- *** "package: utilities": Utilities --> <!-- *** "RTL": React / Web Components (RTL) --> <!-- *** "feature flag": React / Web Components (experimental) -->
Related Ticket(s)
Closes #11213
Jira ticket
Description
Enables the
CTAMixin
to check the URL fragment for a particular pattern,cta-video-[video-id]
, and if present, kick off the CTA trigger, which in the case of a video CTA, is opening the corresponding lightbox and auto-playing video. The effect is that visiting any page with a video CTA for a specific video embedded in it (eg.0_ibuqxqbe
), with a URL fragment like#cta-video-0_ibuqxqbe
, will automatically open the lightbox modal and play the video (provided browser auto-play policies to not block it)Testing instructions
0_ibuqxqbe
).#cta-video-[video_id]
Changelog
New
CTAMixin
, wherectaType === CTA_TYPE.VIDEO
, will check for a specifically crafted URL fragment (cta-video-[video-id]
) to trigger it's run action.