feat: add link embed previews (mention, URL, embed)#9188
feat: add link embed previews (mention, URL, embed)#9188stexz01 wants to merge 3 commits intoTriliumNext:mainfrom
Conversation
When pasting a URL, a floating popup offers three conversion options: - Mention: inline chip with favicon + page title - URL: plain clickable link (default, no conversion needed) - Embed: YouTube iframe player or OpenGraph card preview Features: - Server-side metadata API using YouTube oEmbed + OpenGraph scraping - CKEditor plugin with paste detection and keyboard navigation (arrows + enter) - Popup appears at cursor position, first option highlighted by default - Converts the exact pasted URL instance (not first-found duplicate) - Bouncing dots loading animation while fetching metadata - Read-only and share/published note rendering - SSRF protection on server endpoint - 14 unit tests for URL parsing utilities New files: 9 | Modified files: 12 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
dosubot
bot
added
the
size:XXL
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive "Link Embed" feature, enabling users to insert links as inline mentions, standard URLs, or rich embeds like YouTube iframes and OpenGraph cards. The implementation includes a client-side service, a new server-side metadata fetching API with security validations, and a CKEditor 5 plugin featuring a paste-to-convert popup. Feedback highlights opportunities for code cleanup, such as removing unused parameters, refactoring complex JSX logic into mapping objects, improving error logging for metadata fetching, and centralizing duplicated regex patterns.
| // Embed renderer (YouTube = iframe, otherwise = card) | ||
| // --------------------------------------------------------------------------- | ||
|
|
||
| export async function renderEmbed(url: string, embedType: string, $container: JQuery<HTMLElement>) { |
There was a problem hiding this comment.
The embedType parameter is unused within the renderEmbed function. The function re-determines if the URL is for a YouTube video by calling extractYouTubeVideoId. To simplify the code, you can remove the embedType parameter from this function's signature. You will also need to update its call site in renderPreview.
| export async function renderEmbed(url: string, embedType: string, $container: JQuery<HTMLElement>) { | |
| export async function renderEmbed(url: string, $container: JQuery<HTMLElement>) { |
| if (embedType === "mention") { | ||
| await renderMention(url, $container); | ||
| } else { | ||
| await renderEmbed(url, embedType, $container); |
There was a problem hiding this comment.
To accompany the removal of the unused embedType parameter from renderEmbed, update this call to no longer pass it.
| await renderEmbed(url, embedType, $container); | |
| await renderEmbed(url, $container); |
| className={`btn btn-sm ${mode === m ? "btn-primary" : "btn-outline-secondary"}`} | ||
| onClick={() => setMode(m)} | ||
| > | ||
| {m === "mention" ? "@ Mention" : m === "url" ? "URL" : "Embed"} |
There was a problem hiding this comment.
This chain of ternary operators for labels can be hard to read and maintain. Consider defining a mapping object from mode to label outside the JSX for better clarity and scalability.
For example:
const MODE_LABELS: Record<LinkPasteMode, string> = {
mention: "@ Mention",
url: "URL",
embed: "Embed"
};
// ... then in your JSX
{MODE_LABELS[m]}| if (faviconEl) { | ||
| const href = faviconEl.getAttribute("href"); | ||
| if (href) { | ||
| try { favicon = new URL(href, url).toString(); } catch { /* ignore */ } |
There was a problem hiding this comment.
Silently ignoring errors in a catch block can hide potential issues. While a failure to construct a favicon URL might not be critical, logging the error would be beneficial for debugging.
| try { favicon = new URL(href, url).toString(); } catch { /* ignore */ } | |
| try { favicon = new URL(href, url).toString(); } catch (e) { log.info(`Could not construct favicon URL from href '${href}' with base '${url}': ${e}`); } |
| } | ||
| } | ||
| if (!favicon) { | ||
| try { favicon = `${new URL(url).origin}/favicon.ico`; } catch { /* ignore */ } |
There was a problem hiding this comment.
Similar to the previous point, silently ignoring errors here can hide issues. It would be better to log the error for debugging purposes.
| try { favicon = `${new URL(url).origin}/favicon.ico`; } catch { /* ignore */ } | |
| try { favicon = `${new URL(url).origin}/favicon.ico`; } catch (e) { log.info(`Could not construct fallback favicon URL for '${url}': ${e}`); } |
| function extractYouTubeVideoIdForShare(url: string): string | null { | ||
| const match = url.match(/(?:youtube\.com\/watch\?.*v=|youtu\.be\/|youtube\.com\/embed\/|youtube\.com\/shorts\/)([a-zA-Z0-9_-]{11})/); | ||
| return match ? match[1] : null; | ||
| } |
There was a problem hiding this comment.
The YouTube regular expression is duplicated in several places (apps/client/src/services/link_embed.ts, apps/server/src/routes/api/link_embed.ts, and here). To improve maintainability and ensure consistency, consider defining this regex in a shared location, for example, in a constants file within the @triliumnext/commons package, and importing it where needed.
| let parentEl = root as ReturnType<typeof root.getChild>; | ||
| for (const idx of location.parentPath) { | ||
| if (!parentEl || typeof (parentEl as any).getChild !== 'function') return; | ||
| parentEl = (parentEl as any).getChild(idx); | ||
| } | ||
| if (!parentEl || !parentEl.is('element')) return; |
There was a problem hiding this comment.
This loop to resolve the parent element is a bit hard to follow due to type assertions (as any) and late validation. You can make it more robust and readable by performing checks inside the loop.
| let parentEl = root as ReturnType<typeof root.getChild>; | |
| for (const idx of location.parentPath) { | |
| if (!parentEl || typeof (parentEl as any).getChild !== 'function') return; | |
| parentEl = (parentEl as any).getChild(idx); | |
| } | |
| if (!parentEl || !parentEl.is('element')) return; | |
| let parentEl: Element | RootElement = root; | |
| for (const idx of location.parentPath) { | |
| const child = parentEl.getChild(idx); | |
| // The path should always point to an element. | |
| if (!child || !child.is('element')) { | |
| return; | |
| } | |
| parentEl = child; | |
| } |
|
|
2 participants
Summary
Adds Notion-style link embed previews to Trilium Notes. When a user pastes a URL into a text note, a floating popup appears offering three conversion options:
How it works
Technical details
GET /api/link-embed/metadata): Fetches OpenGraph metadata + favicon. Uses YouTube oEmbed API for reliable video titles. SSRF protection (blocks private IPs).linkembed.ts): Two model elements —linkEmbed(block widget) andlinkMention(inline widget). Paste detection viachange:datalistener with position tracking.<section>and<span>withdata-*attributes (already allowed).Files changed
Test plan
pnpm test:parallel→ all 141 tests pass