feat(ScriptVimeoPlayer): add ratio prop#624
Conversation
|
@AndColla is attempting to deploy a commit to the Nuxt Team on Vercel. A member of the Team first needs to authorize it. |
commit: |
📝 WalkthroughWalkthroughThe changes add a new Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/content/scripts/vimeo-player.md (1)
90-90: Consider aligning the TS props snippet with the newratioprop (or clarify snippet scope).The new bullet documents
ratio, but theVimeoPlayerPropsexample below doesn’t show it. Adding it (or labeling that snippet as SDK/embed options only) would avoid ambiguity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/scripts/vimeo-player.md` at line 90, The docs mention a new `ratio` prop but the `VimeoPlayerProps` TypeScript snippet is missing it; update the `VimeoPlayerProps` example to include `ratio?: string` (or the appropriate type) with its default comment, or alternatively add a clarifying note above the snippet that it only shows SDK/embed options and not all component props; ensure references to `ratio` and `VimeoPlayerProps` are consistent so readers aren’t confused.src/runtime/components/ScriptVimeoPlayer.vue (1)
55-59: Consider normalizingratiobefore style application.
ratiois a public string and is used directly for both root and iframe aspect-ratio paths. A malformed value can break layout sizing. Normalizing once with a fallback ('16/9') would make the component more resilient.Proposed refactor
const props = withDefaults(defineProps<{ @@ ratio?: string }>(), { trigger: 'mousedown', ratio: '16/9', }) +const normalizedRatio = computed(() => { + const value = props.ratio?.trim() || '' + return /^\d+(\.\d+)?\/\d+(\.\d+)?$/.test(value) ? value : '16/9' +}) @@ 'style': { - '--vimeo-ratio': props.ratio, + '--vimeo-ratio': normalizedRatio.value, maxWidth: '100%', width: `auto`, height: 'auto', - aspectRatio: props.ratio, + aspectRatio: normalizedRatio.value, position: 'relative', backgroundColor: 'black', },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/components/ScriptVimeoPlayer.vue` around lines 55 - 59, The component uses the public prop ratio directly when applying styles to the root and iframe; add a single normalized value (e.g., a computed property like normalizedRatio) that validates the format (accept only "N/M" or numeric CSS acceptable values) and falls back to '16/9' when invalid or missing, then use normalizedRatio wherever ratio is used for aspect-ratio styling in ScriptVimeoPlayer.vue (root container and iframe style bindings) to prevent malformed inputs from breaking layout.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/content/scripts/vimeo-player.md`:
- Line 90: The docs mention a new `ratio` prop but the `VimeoPlayerProps`
TypeScript snippet is missing it; update the `VimeoPlayerProps` example to
include `ratio?: string` (or the appropriate type) with its default comment, or
alternatively add a clarifying note above the snippet that it only shows
SDK/embed options and not all component props; ensure references to `ratio` and
`VimeoPlayerProps` are consistent so readers aren’t confused.
In `@src/runtime/components/ScriptVimeoPlayer.vue`:
- Around line 55-59: The component uses the public prop ratio directly when
applying styles to the root and iframe; add a single normalized value (e.g., a
computed property like normalizedRatio) that validates the format (accept only
"N/M" or numeric CSS acceptable values) and falls back to '16/9' when invalid or
missing, then use normalizedRatio wherever ratio is used for aspect-ratio
styling in ScriptVimeoPlayer.vue (root container and iframe style bindings) to
prevent malformed inputs from breaking layout.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 79ecfbbc-9238-4513-8329-1e7cf15b80a4
📒 Files selected for processing (2)
docs/content/scripts/vimeo-player.mdsrc/runtime/components/ScriptVimeoPlayer.vue
ratio prop
|
Nice one, thanks. |
❓ Type of change
📚 Description
Adds the props aspect ratio to ScriptVimeoPlayer to allow embedding videos in any aspect ratio and removes the hardcoded 16:9 value.