Add config support to wistia-video-element#215
Add config support to wistia-video-element#215comanzanares wants to merge 1 commit intomuxinc:mainfrom
Conversation
|
@comanzanares is attempting to deploy a commit to the Mux Team on Vercel. A member of the Team first needs to authorize it. |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
ronald-urbina
left a comment
ronald-urbina
left a comment
There was a problem hiding this comment.
Hey! Thanks for putting this together. We really appreciate the time and effort you put into it. The work looks great overall, I just left a couple of small comments with minor changes before we can approve it. Let me know if anything is unclear!
| export type WistiaPluginConfig = Record<string, unknown>; | ||
|
|
||
| export type WistiaEmbedOptions = { | ||
| autoPlay?: boolean; |
There was a problem hiding this comment.
| autoPlay?: boolean; | |
| autoPlay?: boolean; | |
| chromeless?: boolean; |
can you add this since is being used by the component and tests but was missing from the type
| fakeFullscreen?: boolean; | ||
| fitStrategy?: 'contain' | 'cover' | 'fill' | 'none'; | ||
| fullscreenButton?: boolean; | ||
| muted?: boolean; |
There was a problem hiding this comment.
| muted?: boolean; | |
| muted?: boolean; | |
| keyMoments?: boolean; |
can you add this one since is present in the official documentation
| const id = this.src.match(MATCH_SRC)[1]; | ||
|
|
||
| const options = { | ||
| autoPlay: this.autoplay, |
There was a problem hiding this comment.
| autoPlay: this.autoplay, | |
| silentAutoPlay: this.autoplay ? 'allow' : undefined, |
Added silentAutoPlay: this.autoplay ? 'allow' : undefined to enable Wistia's auto-mute fallback for autoplay on mobile
| autoPlay: this.autoplay, | ||
| preload: this.preload ?? 'metadata', | ||
| playsinline: this.playsInline, | ||
| endVideoBehavior: this.loop && 'loop', |
There was a problem hiding this comment.
| endVideoBehavior: this.loop ? 'loop' : undefined, |
this.loop && 'loop' evaluates to false when loop is not set, which passes endVideoBehavior: false, an invalid value (Wistia only accepts 'default' | 'reset' | 'loop'). Changed to a ternary so the key is omitted entirely when loop is off, which is what Wistia expects.
| as={WistiaVideo} | ||
| src="https://wesleyluyten.wistia.com/medias/oifkgmxnkb" | ||
| config={{ | ||
| playerColor: '#f5f6f7', |
There was a problem hiding this comment.
| playerColor: '#f5f6f7', | |
| playerColor: '#afe5a4', |
Consider using a more vibrant color here to improve control visibility, the current shade may be too subtle
2 participants
Adds a config property to to pass Wistia embed options via
_wq.push({ options }), consistent with other elements like Vimeo, HLS, etc.Merges config into Wistia
optionswith user config taking precedence over attribute-derived defaults.Added params based on Wistia embed options docs.
Updates the example to demonstrate passing config to Wistia works.
Adds a test to ensure config is forwarded into Wistia options.