Willem/feat: add YouTube video component #4116
Conversation
WalkthroughAdds a new YouTube Video designer component with full props/interfaces, rendering and iframe event handling, responsive sizing, thumbnail resolution, settings UI, styles, and toolbox registration. Changes
Sequence Diagram(s)sequenceDiagram
participant Designer
participant Component as YouTubeComponent
participant Iframe as YouTubeIframe
participant Window as Window(postMessage)
participant Actions as ConfiguredActions
Designer->>Component: Render (props, videoId)
Component->>Component: Compute embed URL, styles, thumbnail
Component->>Iframe: Insert iframe (enablejsapi if needed) / or show thumbnail placeholder
Iframe->>Window: postMessage onReady / stateChange
Window->>Component: Receive message (onReady/onStateChange)
Component->>Component: Update playback state / watched progress
Component->>Actions: Trigger onReady/onPlay/onPause/onEnd handlers with context
alt watchCompletionRequired
Component->>Designer: Expose validation state (warning until complete)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related issues
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
shesha-reactjs/src/designer-components/youtubeVideo/index.tsx(1 hunks)shesha-reactjs/src/designer-components/youtubeVideo/interfaces.ts(1 hunks)shesha-reactjs/src/designer-components/youtubeVideo/settingsForm.ts(1 hunks)shesha-reactjs/src/designer-components/youtubeVideo/styles.module.scss(1 hunks)shesha-reactjs/src/providers/form/defaults/toolboxComponents.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3312
File: shesha-reactjs/src/hooks/formComponentHooks.ts:183-192
Timestamp: 2025-05-28T07:55:21.036Z
Learning: In shesha-reactjs formComponentHooks.ts, the background style initialization deliberately sets valid values from properties panel immediately to provide better UX, rather than waiting for the useDeepCompareEffect to run. This intentional pattern prevents a flash of no background and shows the background image immediately when the component renders.
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3461
File: shesha-reactjs/src/designer-components/charts/settingsFormIndividual.ts:1280-1280
Timestamp: 2025-06-26T19:59:22.872Z
Learning: In shesha-reactjs chart settings forms, the styling panels (border, shadow, background) should not include buttonType-based hidden conditions since charts are not button components. The border configuration should follow the standard component pattern without button-specific restrictions.
📚 Learning: 2025-10-28T14:21:55.782Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4084
File: shesha-reactjs/src/components/quickView/styles/styles.ts:41-94
Timestamp: 2025-10-28T14:21:55.782Z
Learning: In shesha-reactjs/src/components/quickView/styles/styles.ts, the quickViewContent styles intentionally use multiple `!important` flags and broad selectors (including spans, form items, inputs, links, and labels) to ensure proper targeting and prevent layout breakage in the quick view component.
Applied to files:
shesha-reactjs/src/designer-components/youtubeVideo/styles.module.scss
📚 Learning: 2025-06-26T20:01:48.838Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3461
File: shesha-reactjs/src/designer-components/charts/settingsFormIndividual.ts:171-192
Timestamp: 2025-06-26T20:01:48.838Z
Learning: In shesha-reactjs chart settings forms, duplicate input configurations may be intentionally included as part of UI/UX experiments and should be flagged for review during UI/UX review phases rather than immediately removed as errors.
Applied to files:
shesha-reactjs/src/designer-components/youtubeVideo/settingsForm.ts
📚 Learning: 2025-06-26T19:59:22.872Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3461
File: shesha-reactjs/src/designer-components/charts/settingsFormIndividual.ts:1280-1280
Timestamp: 2025-06-26T19:59:22.872Z
Learning: In shesha-reactjs chart settings forms, the styling panels (border, shadow, background) should not include buttonType-based hidden conditions since charts are not button components. The border configuration should follow the standard component pattern without button-specific restrictions.
Applied to files:
shesha-reactjs/src/designer-components/youtubeVideo/settingsForm.ts
📚 Learning: 2025-06-12T16:55:57.638Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3397
File: shesha-reactjs/src/designer-components/charts/bar.tsx:49-52
Timestamp: 2025-06-12T16:55:57.638Z
Learning: For the chart components’ migrators (e.g., BarChartComponent in shesha-reactjs/src/designer-components/charts/bar.tsx), the version 5 step intentionally spreads `{ ...prev, ...defaultConfigFiller }` so that values from `defaultConfigFiller` override any existing properties in `prev`. This reset to new defaults is by design and should not be flagged as an issue.
Applied to files:
shesha-reactjs/src/providers/form/defaults/toolboxComponents.ts
🧬 Code graph analysis (2)
shesha-reactjs/src/designer-components/youtubeVideo/index.tsx (5)
shesha-reactjs/src/providers/form/models.ts (1)
FormMode(32-32)shesha-reactjs/src/interfaces/formDesigner.ts (1)
IToolboxComponent(68-179)shesha-reactjs/src/designer-components/youtubeVideo/interfaces.ts (1)
IYoutubeVideoComponentProps(4-58)shesha-reactjs/src/utils/object.ts (1)
removeUndefinedProps(171-173)shesha-reactjs/src/designer-components/youtubeVideo/settingsForm.ts (1)
getSettings(7-744)
shesha-reactjs/src/designer-components/youtubeVideo/settingsForm.ts (2)
shesha-reactjs/src/providers/form/models.ts (1)
FormMarkupWithSettings(383-386)shesha-reactjs/src/designer-components/_settings/utils/border/utils.tsx (2)
getBorderInputs(136-242)getCornerInputs(251-303)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-attempt
…illemBarendKruger/shesha-framework into willem/featureYouTubeComponent
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
shesha-reactjs/src/designer-components/youtubeVideo/index.tsx (2)
103-123: Remove unnecessary semicolon.The dimension handling logic is correct, but there's an unnecessary semicolon after the closing brace at line 122.
Apply this diff to remove the semicolon:
width: 560, height: 315, }; - }; + } }
204-210: Remove unused catch variable.The catch block defines
ebut never uses it. While this is harmless, it triggers linter warnings.Apply this diff to fix the linter warning:
let data; try { data = typeof event.data === 'string' ? JSON.parse(event.data) : event.data; - } catch (e) { + } catch { // Ignore malformed messages return; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
shesha-reactjs/src/designer-components/youtubeVideo/index.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3312
File: shesha-reactjs/src/hooks/formComponentHooks.ts:183-192
Timestamp: 2025-05-28T07:55:21.036Z
Learning: In shesha-reactjs formComponentHooks.ts, the background style initialization deliberately sets valid values from properties panel immediately to provide better UX, rather than waiting for the useDeepCompareEffect to run. This intentional pattern prevents a flash of no background and shows the background image immediately when the component renders.
🧬 Code graph analysis (1)
shesha-reactjs/src/designer-components/youtubeVideo/index.tsx (5)
shesha-reactjs/src/providers/form/models.ts (1)
FormMode(32-32)shesha-reactjs/src/interfaces/formDesigner.ts (1)
IToolboxComponent(68-179)shesha-reactjs/src/designer-components/youtubeVideo/interfaces.ts (1)
IYoutubeVideoComponentProps(4-58)shesha-reactjs/src/utils/object.ts (1)
removeUndefinedProps(171-173)shesha-reactjs/src/designer-components/youtubeVideo/settingsForm.ts (1)
getSettings(7-744)
🪛 GitHub Check: build-attempt
shesha-reactjs/src/designer-components/youtubeVideo/index.tsx
[failure] 207-207:
'e' is defined but never used
🔇 Additional comments (7)
shesha-reactjs/src/designer-components/youtubeVideo/index.tsx (7)
1-20: LGTM! Clean imports and interface setup.The imports are well-organized and the
IYoutubeVideoCalculatedValuesinterface appropriately captures the calculated form mode.
22-68: LGTM! Component metadata and setup are well-structured.The component is correctly configured as both input and output, and the Factory function properly destructures all configuration properties with sensible defaults.
71-92: LGTM! Thumbnail resolution logic handles backward compatibility well.The function correctly prioritizes the new
thumbnailSourceproperty while maintaining backward compatibility withcustomThumbnail.
94-100: LGTM! Percentage conversion helper works correctly.The regex-based extraction handles various input formats (px, plain numbers, percentages) and appropriately returns undefined for invalid inputs.
135-180: LGTM! YouTube URL construction is comprehensive and secure.The function correctly builds the embed URL with all necessary parameters, properly handles privacy mode, and appropriately enables the JS API only when event handlers or watch completion tracking is needed. The origin parameter ensures secure cross-origin messaging.
300-452: LGTM! Render implementation is comprehensive and well-structured.The component correctly handles:
- Designer mode placeholder when videoId is missing
- Responsive vs. non-responsive layout with appropriate styling
- Custom thumbnail overlay with play functionality
- YouTube iframe with proper event listener registration (both
onStateChangeandonReadyare now subscribed, addressing the previous review feedback)- Watch completion warning when required
The responsive mode implementation using aspect-ratio padding is a solid approach for maintaining 16:9 ratio.
454-480: LGTM! Component lifecycle methods are correctly implemented.The
calculateModel,initModel, andmigratorfunctions follow the framework patterns. The default initialization withhideLabel: trueis appropriate for a video component, and the migration chain properly handles legacy configurations.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
shesha-reactjs/src/designer-components/youtubeVideo/index.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-12T16:55:57.638Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3397
File: shesha-reactjs/src/designer-components/charts/bar.tsx:49-52
Timestamp: 2025-06-12T16:55:57.638Z
Learning: For the chart components’ migrators (e.g., BarChartComponent in shesha-reactjs/src/designer-components/charts/bar.tsx), the version 5 step intentionally spreads `{ ...prev, ...defaultConfigFiller }` so that values from `defaultConfigFiller` override any existing properties in `prev`. This reset to new defaults is by design and should not be flagged as an issue.
Applied to files:
shesha-reactjs/src/designer-components/youtubeVideo/index.tsx
🧬 Code graph analysis (1)
shesha-reactjs/src/designer-components/youtubeVideo/index.tsx (5)
shesha-reactjs/src/providers/form/models.ts (1)
FormMode(32-32)shesha-reactjs/src/interfaces/formDesigner.ts (1)
IToolboxComponent(68-179)shesha-reactjs/src/designer-components/youtubeVideo/interfaces.ts (1)
IYoutubeVideoComponentProps(4-58)shesha-reactjs/src/utils/object.ts (1)
removeUndefinedProps(171-173)shesha-reactjs/src/designer-components/youtubeVideo/settingsForm.ts (1)
getSettings(7-744)
🪛 Biome (2.1.2)
shesha-reactjs/src/designer-components/youtubeVideo/index.tsx
[error] 209-209: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-attempt
|
Hi @WillemBarendKruger. This is a well-structured YouTube Video component with good separation of concerns and comprehensive feature set. The main issues relate to TypeScript best practices as defined in the project guidelines (avoiding
|
… performance, and implement type guards for event data validation.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
shesha-reactjs/src/designer-components/youtubeVideo/index.tsx(1 hunks)shesha-reactjs/src/designer-components/youtubeVideo/interfaces.ts(1 hunks)shesha-reactjs/src/designer-components/youtubeVideo/styles.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-10-28T14:21:55.782Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4084
File: shesha-reactjs/src/components/quickView/styles/styles.ts:41-94
Timestamp: 2025-10-28T14:21:55.782Z
Learning: In shesha-reactjs/src/components/quickView/styles/styles.ts, the quickViewContent styles intentionally use multiple `!important` flags and broad selectors (including spans, form items, inputs, links, and labels) to ensure proper targeting and prevent layout breakage in the quick view component.
Applied to files:
shesha-reactjs/src/designer-components/youtubeVideo/styles.ts
📚 Learning: 2025-05-28T07:55:21.036Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3312
File: shesha-reactjs/src/hooks/formComponentHooks.ts:183-192
Timestamp: 2025-05-28T07:55:21.036Z
Learning: In shesha-reactjs formComponentHooks.ts, the background style initialization deliberately sets valid values from properties panel immediately to provide better UX, rather than waiting for the useDeepCompareEffect to run. This intentional pattern prevents a flash of no background and shows the background image immediately when the component renders.
Applied to files:
shesha-reactjs/src/designer-components/youtubeVideo/styles.ts
📚 Learning: 2025-06-12T16:55:57.638Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3397
File: shesha-reactjs/src/designer-components/charts/bar.tsx:49-52
Timestamp: 2025-06-12T16:55:57.638Z
Learning: For the chart components’ migrators (e.g., BarChartComponent in shesha-reactjs/src/designer-components/charts/bar.tsx), the version 5 step intentionally spreads `{ ...prev, ...defaultConfigFiller }` so that values from `defaultConfigFiller` override any existing properties in `prev`. This reset to new defaults is by design and should not be flagged as an issue.
Applied to files:
shesha-reactjs/src/designer-components/youtubeVideo/index.tsx
🧬 Code graph analysis (2)
shesha-reactjs/src/designer-components/youtubeVideo/index.tsx (7)
shesha-reactjs/src/designer-components/youtubeVideo/interfaces.ts (3)
IYouTubeEventData(64-67)IYoutubeVideoComponentProps(4-58)IYoutubeVideoCalculatedValues(60-62)shesha-reactjs/src/designer-components/youtubeVideo/styles.ts (1)
useStyles(3-99)shesha-reactjs/src/providers/configurableActionsDispatcher/index.tsx (1)
useConfigurableActionDispatcher(269-269)shesha-reactjs/src/providers/form/index.tsx (1)
useForm(162-162)shesha-reactjs/src/utils/object.ts (1)
removeUndefinedProps(171-173)shesha-reactjs/src/providers/globalState/globalState.ts (1)
globalState(14-16)shesha-reactjs/src/designer-components/youtubeVideo/settingsForm.ts (1)
getSettings(7-744)
shesha-reactjs/src/designer-components/youtubeVideo/interfaces.ts (1)
shesha-reactjs/src/providers/form/models.ts (1)
FormMode(32-32)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-attempt
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
shesha-reactjs/src/designer-components/youtubeVideo/index.tsx (1)
101-125: Verify if unclamped percentage conversion is intentional or apply suggested fixes.The
toPercentagefunction converts pixel values like "560px" to "560%" without clamping. Commit 519dfb3 ("better handeling of dimensions") improved validation but did not address the clamping gap. The settingsForm.ts tooltip explicitly documents this: "500px → 500%", which would apply 560% width to the CSS—5.6× the container width, causing overflow in responsive mode.At line 137, the fallback
|| '100%'only handles undefined values, not over-100 percentages, so unclamped values pass directly to the width property.Option 1: Reject px-suffixed values and clamp bare numbers
// Extract numeric part from "500px", "500", etc. - const match = strValue.match(/^(\d+(?:\.\d+)?)/); + // Only accept bare numbers, reject px-suffixed values + const match = strValue.match(/^(\d+(?:\.\d+)?)$/); if (!match) { return undefined; } const numeric = parseFloat(match[1]); if (!Number.isFinite(numeric) || numeric < 0) { return undefined; } - return `${numeric}%`; + // Clamp to reasonable percentage range + return `${Math.min(numeric, 100)}%`;Option 2: Return undefined for non-percentage inputs
// If already a percentage, return as-is if (strValue.endsWith('%')) { return strValue; } - // Extract numeric part from "500px", "500", etc. - const match = strValue.match(/^(\d+(?:\.\d+)?)/); - if (!match) { - return undefined; - } - - const numeric = parseFloat(match[1]); - if (!Number.isFinite(numeric) || numeric < 0) { - return undefined; - } - - return `${numeric}%`; + // Reject non-percentage values in responsive mode + return undefined;
🧹 Nitpick comments (2)
shesha-reactjs/src/designer-components/youtubeVideo/index.tsx (2)
140-148: Remove unnecessary semicolon.Line 147 has an unnecessary semicolon after the closing brace.
finalDimensionStyles = { ...finalDimensionStyles, width: 560, height: 315, }; - }; + }
78-98: Consider memoizing thumbnail URL resolution.The
resolvedThumbnail(line 98) is recalculated on every render, andthumbnailStyle(lines 237-258) depends on it. While thegetThumbnailUrlfunction is lightweight, wrapping it inuseMemowould prevent unnecessary style object recreation when thumbnail-related props haven't changed.- const getThumbnailUrl = (): string | null => { + const resolvedThumbnail = useMemo((): string | null => { // Backward compatibility: use old customThumbnail if new properties not set if (!thumbnailSource && customThumbnail) { return customThumbnail; } switch (thumbnailSource) { case 'url': return thumbnailUrl || null; case 'base64': return thumbnailBase64 || null; case 'storedFile': return thumbnailStoredFileId ? `/api/StoredFile/Download?id=${encodeURIComponent(thumbnailStoredFileId)}` : null; default: return null; } - }; - - const resolvedThumbnail = getThumbnailUrl(); + }, [thumbnailSource, customThumbnail, thumbnailUrl, thumbnailBase64, thumbnailStoredFileId]);Also applies to: 237-258
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
shesha-reactjs/src/designer-components/youtubeVideo/index.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-06-12T16:55:57.638Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3397
File: shesha-reactjs/src/designer-components/charts/bar.tsx:49-52
Timestamp: 2025-06-12T16:55:57.638Z
Learning: For the chart components’ migrators (e.g., BarChartComponent in shesha-reactjs/src/designer-components/charts/bar.tsx), the version 5 step intentionally spreads `{ ...prev, ...defaultConfigFiller }` so that values from `defaultConfigFiller` override any existing properties in `prev`. This reset to new defaults is by design and should not be flagged as an issue.
Applied to files:
shesha-reactjs/src/designer-components/youtubeVideo/index.tsx
📚 Learning: 2025-11-05T07:46:22.035Z
Learnt from: Lihlu
Repo: shesha-io/shesha-framework PR: 4132
File: shesha-reactjs/src/components/layerEditor/modal.tsx:55-57
Timestamp: 2025-11-05T07:46:22.035Z
Learning: In shesha-reactjs layer editor modal (src/components/layerEditor/modal.tsx), the useDeepCompareEffect that syncs items to the parent via onChange should only depend on [items], not [items, onChange]. Adding onChange to the dependency array causes the effect to fire on every parent render and breaks functionality. The guard for optional onChange is needed but the function reference should not be in the dependency array.
Applied to files:
shesha-reactjs/src/designer-components/youtubeVideo/index.tsx
🧬 Code graph analysis (1)
shesha-reactjs/src/designer-components/youtubeVideo/index.tsx (5)
shesha-reactjs/src/designer-components/youtubeVideo/interfaces.ts (3)
IYouTubeEventData(64-67)IYoutubeVideoComponentProps(4-58)IYoutubeVideoCalculatedValues(60-62)shesha-reactjs/src/interfaces/formDesigner.ts (1)
IToolboxComponent(68-179)shesha-reactjs/src/designer-components/youtubeVideo/styles.ts (1)
useStyles(3-99)shesha-reactjs/src/utils/object.ts (1)
removeUndefinedProps(171-173)shesha-reactjs/src/designer-components/youtubeVideo/settingsForm.ts (1)
getSettings(7-744)
🔇 Additional comments (7)
shesha-reactjs/src/designer-components/youtubeVideo/index.tsx (7)
18-25: LGTM! Type guard properly implemented.The dedicated type guard function validates YouTube event data with appropriate runtime checks, narrowing the type safely for downstream usage.
160-205: LGTM! Secure and comprehensive URL construction.The YouTube embed URL is built safely using
URLSearchParams, with appropriate privacy mode handling, comprehensive parameter mapping, and security-conscious origin parameter inclusion for the JS API.
269-355: LGTM! Secure and robust message handler.The YouTube iframe message handler implements comprehensive security checks (origin and source validation), safe error handling, type guard validation, and proper event routing. All concerns from previous reviews have been addressed.
362-376: LGTM! Promise-based validation correctly implemented.The custom validator uses Promise-based syntax (
Promise.resolve()/Promise.reject(new Error(...))) which aligns with Ant Design 5 and the framework's validation expectations. This properly addresses the previous review feedback.
437-463: LGTM! YouTube iframe API initialization complete.The
onLoadhandler correctly subscribes to bothonStateChangeandonReadyevents via postMessage, addressing the concern from the previous review. The iframe API setup follows YouTube's documented patterns with proper channel and event structure.
482-495: LGTM! Sensible default initialization.The
initModelfunction provides appropriate defaults for the YouTube video component, including responsive mode, controls visibility, andhideLabel: truewhich is a good default for a media component.
497-503: LGTM! Standard component configuration.The migrator, settings form, and validation configuration follow the framework's established patterns and utilize the appropriate utilities from the common migration library.
2 participants
#2671
Added YouTube video component with customizable settings and event handling.
Note: some of the requirements are deprecated:
Summary by CodeRabbit
New Features
Settings
Style
Chores