-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat: Add support for Video type in View table to display videos
#3061
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
Conversation
|
🚀 Thanks for opening this pull request! |
📝 WalkthroughWalkthroughAdds Video as a new view type to the dashboard. Updates README documentation with Video section detailing rendering with video tags, controls, sizing, and URL sanitization examples. Extends Views component to recognize and render Video-type values with sanitized URLs, optional width/height, object-fit scaling, and error handling. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
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 |
✅ 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. |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/dashboard/Data/Views/Views.react.js (1)
497-533: Consider strengthening URL sanitization.The current regex-based URL sanitization using
url.match(/javascript/i)andurl.match(/<script/i)can be bypassed with various encoding techniques (e.g.,java\nscript:, HTML entities, URL encoding). While this matches the existing Image implementation, consider using a more robust approach for both.A safer approach would be to validate the URL scheme explicitly:
- if ( - !url || - url.match(/javascript/i) || - url.match(/<script/i) - ) { - url = '#'; - } + try { + const parsedUrl = new URL(url, window.location.origin); + if (!['http:', 'https:', 'data:'].includes(parsedUrl.protocol)) { + url = '#'; + } + } catch { + url = '#'; + }This would apply to the Image type as well (lines 464-471) for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(2 hunks)src/dashboard/Data/Views/Views.react.js(3 hunks)
⏰ 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: Docker linux/amd64
🔇 Additional comments (4)
src/dashboard/Data/Views/Views.react.js (2)
276-277: LGTM!The Video type detection during column building follows the established pattern used for other types like Image and Link.
411-412: LGTM!The Video type detection in row rendering mirrors the column-building logic, maintaining consistency.
README.md (2)
102-102: LGTM!Table of contents entry correctly added for the new Video section.
1637-1654: LGTM!Documentation is well-structured, follows the established pattern from the Image section, and includes appropriate security warnings. The JSON example clearly shows the expected structure with
__type,url,width, andheightproperties.
# [8.2.0-alpha.4](8.2.0-alpha.3...8.2.0-alpha.4) (2025-12-10) ### Features * Add support for `Video` type in View table to display videos ([#3061](#3061)) ([bd4aa4f](bd4aa4f))
|
🎉 This change has been released in version 8.2.0-alpha.4 |
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.