✨(frontend) detect and embed YouTube/Vimeo/Loom in video block#2260
✨(frontend) detect and embed YouTube/Vimeo/Loom in video block#2260MashdorDev wants to merge 5 commits intosuitenumerique:mainfrom
Conversation
Detects YouTube, Vimeo, Loom, and Dailymotion URLs and returns iframe-ready src URLs. URLs that look like direct video files (or are unrecognised) fall back to native <video> rendering, matching the behaviour of BlockNote's default video block so existing documents keep working unchanged. Refs suitenumerique#464 Signed-off-by: MashdorDev <dorzairi@ymail.com>
Override BlockNote's default video block with a React-based one that detects URLs from known platforms (YouTube, Vimeo, Loom, Dailymotion) and renders an <iframe> with appropriate sandbox and allow attributes. Direct video file URLs continue to render through <video>, preserving backward compatibility with existing documents. Iframes use a 16:9 aspect ratio so they scale correctly inside the ResizableFileBlockWrapper instead of being squished into a tall narrow box on smaller container widths. Empty URLs fall through to BlockNote's built-in Upload/URL/Embed picker rather than showing a premature "invalid" warning; the warning is now reserved for the case where a non-empty URL fails the isSafeUrl check. The Embed tab in the file panel now produces working YouTube/Vimeo embeds instead of triggering "no video with supported format and mime type found". Closes suitenumerique#464 Signed-off-by: MashdorDev <dorzairi@ymail.com>
Refs suitenumerique#464 Signed-off-by: MashdorDev <dorzairi@ymail.com>
WalkthroughAdds a new VideoBlock custom BlockNote component and registers it in the editor schema to render video content. Implements parseEmbedUrl utility (with exported types) to normalize YouTube, Vimeo, Loom, Dailymotion links into iframe embed URLs or return direct-video sources. VideoBlock renders either a sandboxed, lazy-loaded iframe or a native HTML5 video element, validates URLs (using existing isSafeUrl), shows a localized warning for invalid URLs, accepts Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Line 14: Update the changelog entry string that currently reads "- ✨(frontend)
detect and embed YouTube/Vimeo/Loom in video block" to include Dailymotion as
well (e.g. add "/Dailymotion" or ", Dailymotion") so the entry accurately
reflects the implemented/tested support for Dailymotion.
In `@src/frontend/apps/impress/src/utils/embed.ts`:
- Around line 15-17: YOUTUBE_RE currently only matches watch?v=... and misses
URLs where v= is not the first query param (e.g., watch?si=...&v=...), so update
the YOUTUBE_RE pattern to accept a YouTube watch URL when the v= parameter
appears anywhere in the query string (e.g., match watch followed by any query
that contains v= or match a fragment like /embed/, /v/, /shorts/, or youtu.be/),
and apply the same fix to the other identical regex instance in this file so
both uses correctly extract the 11-character video ID.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 006306d1-1c40-4ebb-a2b3-a705c98b6130
📒 Files selected for processing (6)
CHANGELOG.mdsrc/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteEditor.tsxsrc/frontend/apps/impress/src/features/docs/doc-editor/components/custom-blocks/VideoBlock.tsxsrc/frontend/apps/impress/src/features/docs/doc-editor/components/custom-blocks/index.tssrc/frontend/apps/impress/src/utils/__tests__/embed.test.tsxsrc/frontend/apps/impress/src/utils/embed.ts
Replace YOUTUBE_RE with a URL-parser-based getYouTubeId that uses
searchParams.get('v'), so YouTube share URLs where v= is not the first
query param (e.g. watch?si=...&v=..., watch?feature=share&si=...&v=...&t=...)
are detected correctly. Add tests covering both orderings.
Signed-off-by: MashdorDev <dorzairi@ymail.com>
Dailymotion was added to parseEmbedUrl alongside YouTube, Vimeo and Loom but the changelog entry only listed the first three. Signed-off-by: MashdorDev <dorzairi@ymail.com>
|
Pushed two small fixes from the review. Added Dailymotion to the changelog line, it was already supported in the code and tests, just missing from the text. For the YouTube one, the old check missed share links like |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/frontend/apps/impress/src/utils/embed.ts`:
- Around line 56-57: The DAILYMOTION_RE constant in embed.ts doesn't match URLs
with hash fragments (e.g. `#start`=30); update the regex DAILYMOTION_RE to include
'#' in the optional trailing group so hashes are accepted (e.g. change the
trailing character class from [?_] to include # such as [?_#] or [?#] as
appropriate) so URLs like https://www.dailymotion.com/video/x9zyxwv#start=30
correctly match and return the video id.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 92be4d41-5b56-4ae8-9902-5d1b9bf2b522
📒 Files selected for processing (3)
CHANGELOG.mdsrc/frontend/apps/impress/src/utils/__tests__/embed.test.tsxsrc/frontend/apps/impress/src/utils/embed.ts
| const DAILYMOTION_RE = | ||
| /^(?:https?:\/\/)?(?:www\.)?(?:dailymotion\.com\/(?:video|embed\/video)|dai\.ly)\/([a-zA-Z0-9]+)(?:[?_].*)?$/; |
There was a problem hiding this comment.
Dailymotion regex silently fails for URLs with hash fragments.
(?:[?_].*)?$ handles query strings (?) and title slugs (_) but not #. A browser-copied URL like https://www.dailymotion.com/video/x9zyxwv#start=30 won't match — it falls through to { kind: 'video', src: url }, causing a silent broken <video> attempting to load an HTML page.
🐛 Proposed fix
- /^(?:https?:\/\/)?(?:www\.)?(?:dailymotion\.com\/(?:video|embed\/video)|dai\.ly)\/([a-zA-Z0-9]+)(?:[?_].*)?$/;
+ /^(?:https?:\/\/)?(?:www\.)?(?:dailymotion\.com\/(?:video|embed\/video)|dai\.ly)\/([a-zA-Z0-9]+)(?:[?_#].*)?$/;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const DAILYMOTION_RE = | |
| /^(?:https?:\/\/)?(?:www\.)?(?:dailymotion\.com\/(?:video|embed\/video)|dai\.ly)\/([a-zA-Z0-9]+)(?:[?_].*)?$/; | |
| const DAILYMOTION_RE = | |
| /^(?:https?:\/\/)?(?:www\.)?(?:dailymotion\.com\/(?:video|embed\/video)|dai\.ly)\/([a-zA-Z0-9]+)(?:[?_#].*)?$/; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/frontend/apps/impress/src/utils/embed.ts` around lines 56 - 57, The
DAILYMOTION_RE constant in embed.ts doesn't match URLs with hash fragments (e.g.
`#start`=30); update the regex DAILYMOTION_RE to include '#' in the optional
trailing group so hashes are accepted (e.g. change the trailing character class
from [?_] to include # such as [?_#] or [?#] as appropriate) so URLs like
https://www.dailymotion.com/video/x9zyxwv#start=30 correctly match and return
the video id.
Purpose
#464 — pasting a YouTube URL via the Embed tab fails with
no video with supported format and mime type foundbecause the default video block always renders<video>. This adds a custom block that renders<iframe>for known embed providers.Proposal
parseEmbedUrlURL parser detecting YouTube, Vimeo, Loom, Dailymotion (with unit tests)defaultBlockSpecs.videowith a custom React block — iframe for embed URLs (16:9 aspect, sandbox, allow),<video>for direct file URLs (preserves backward compat with existing documents)External contributions
Thank you for your contribution! 🎉
Please ensure the following items are checked before submitting your pull request:
General requirements
Skip the checkbox below 👇 if you're fixing an issue or adding documentation
CI requirements
git commit --signoff(DCO compliance)git commit -S)<gitmoji>(type) title description## [Unreleased]section (if noticeable change)AI requirements
Skip the checkboxes below 👇 If you didn't use AI for your contribution