-
Notifications
You must be signed in to change notification settings - Fork 3.3k
feat(note-block): expand media embed support with tuned aspect ratios #3016
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
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
ae69b5f to
ad53a7e
Compare
ad53a7e to
9f8b4a4
Compare
Greptile OverviewGreptile SummaryExpanded media embed support from YouTube-only to 20+ platforms including Spotify, Vimeo, TikTok, Twitch, Instagram, Twitter/X, SoundCloud, Apple Music, and Bandcamp. Added platform-specific aspect ratios and iframe scaling (0.78x) to properly fit embeds within narrow note blocks. Implemented markdown table rendering support.
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant NoteBlock
participant NoteMarkdown
participant getEmbedInfo
participant ReactMarkdown
participant Browser
User->>NoteBlock: Views note with URL
NoteBlock->>NoteMarkdown: Renders content
NoteMarkdown->>ReactMarkdown: Process markdown
ReactMarkdown->>getEmbedInfo: Check if URL is embeddable
alt URL matches platform pattern
getEmbedInfo->>getEmbedInfo: Extract ID/parameters from URL
getEmbedInfo->>getEmbedInfo: Determine embed type (iframe/video/audio)
getEmbedInfo->>getEmbedInfo: Set platform-specific aspect ratio
getEmbedInfo-->>ReactMarkdown: Return EmbedInfo object
alt Embed type is iframe
ReactMarkdown->>Browser: Render iframe with scaling (0.78x)
Browser->>Browser: Apply aspect ratio and transform
Browser-->>User: Display embedded media
else Embed type is video
ReactMarkdown->>Browser: Render video element
Browser-->>User: Display video player
else Embed type is audio
ReactMarkdown->>Browser: Render audio element
Browser-->>User: Display audio player
end
else URL is not embeddable
getEmbedInfo-->>ReactMarkdown: Return null
ReactMarkdown->>Browser: Render as regular link
Browser-->>User: Display clickable link
end
|
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.
4 files reviewed, 4 comments
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/note-block/note-block.tsx
Outdated
Show resolved
Hide resolved
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/note-block/note-block.tsx
Outdated
Show resolved
Hide resolved
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/note-block/note-block.tsx
Show resolved
Hide resolved
Additional Comments (1)
Context Used: Context from Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/note-block/note-block.tsx
Line: 368:390
Comment:
Uses `any` type for component props - violates TypeScript conventions. Use proper React component prop types like `React.HTMLAttributes<HTMLElement>` or define specific interfaces
**Context Used:** Context from `dashboard` - TypeScript conventions and type safety ([source](https://app.greptile.com/review/custom-context?memory=b4f0be8d-a787-4d5a-9098-a66b1449df25))
How can I resolve this? If you propose a fix, please make it concise. |
9f8b4a4 to
9ed0fae
Compare
9ed0fae to
e477bc8
Compare
|
@greptile |
Include the artist subdomain in Bandcamp track and album embed URLs to ensure proper embed resolution.
ed4a335 to
4ce7c4e
Compare
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.
2 files reviewed, 2 comments
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/note-block/note-block.tsx
Outdated
Show resolved
Hide resolved
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/note-block/note-block.tsx
Outdated
Show resolved
Hide resolved
HTML spec requires track elements to have a src attribute.
ffbd43d to
0e99cd5
Compare
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/note-block/note-block.tsx
Show resolved
Hide resolved
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/note-block/note-block.tsx
Outdated
Show resolved
Hide resolved
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/note-block/note-block.tsx
Outdated
Show resolved
Hide resolved
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/note-block/note-block.tsx
Show resolved
Hide resolved
- Fix YouTube regex to handle v= anywhere in query params - Fix Twitch channel match to exclude /clip/ URLs - Remove Mux support (HLS not supported in most browsers) - Remove Bandcamp support (requires numeric IDs, not slugs) Co-Authored-By: Claude Opus 4.5 <[email protected]>
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.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| const directUrl = url | ||
| .replace('www.dropbox.com', 'dl.dropboxusercontent.com') | ||
| .replace('?dl=0', '') | ||
| return { url: directUrl, type: 'video' } |
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.
Dropbox URL replacement can create malformed URLs
Low Severity
The Dropbox URL transformation using replace('?dl=0', '') creates malformed URLs when additional query parameters exist. For example, a URL like dropbox.com/file.mp4?dl=0&ref=123 becomes dropbox.com/file.mp4&ref=123 with an orphaned & instead of a ?, resulting in an invalid URL that the video element cannot load correctly.
| height: EMBED_INVERSE_SCALE, | ||
| transform: `scale(${EMBED_SCALE})`, | ||
| }} | ||
| /> |
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.
Iframe sandbox attribute removal reduces security posture
Medium Severity
The previous YouTube embed included sandbox='allow-scripts allow-same-origin allow-presentation allow-popups' which restricted iframe capabilities. The new generic iframe rendering removes this attribute entirely for all 20+ embed providers. Without the sandbox, embedded content gains unrestricted capabilities including top-frame navigation, form submission, and modal dialogs. The referrerPolicy attribute was also removed.
Summary
Type of Change
Testing
Tested manually with real embed URLs
Checklist