feat(markdown-renderer): add copy code button for code blocks#1812
feat(markdown-renderer): add copy code button for code blocks#1812Killusions wants to merge 3 commits intomainfrom
Conversation
Add max-inline-size constraint to message bubble and wrapper to prevent overflow. Fix attachment preview backdrop click behavior. Use index-based tracking for message list and add scroll-to-bottom on message send. Update example action labels for clarity.
…line and caching Rewrite the markdown renderer to use a recursive processing pipeline with placeholder-based architecture. Add SSR support via optional doc/isBrowser parameters. Introduce LRU caching for code blocks and tables. Improve table rendering with proper thead/th support. Add code-wrapper structure with language labels. Update SCSS for new structural layout including code-wrapper, table-wrapper, and table-scroll-container.
![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 refactors the markdown renderer to support Server-Side Rendering (SSR), implements LRU caching for code blocks and tables, and adds a copy-to-clipboard feature for code snippets. It also updates chat message action labels to align with UX guidelines. Feedback highlights a duplicate code block with conflicting definitions, a caching issue where locale changes are not reflected in translated labels, and potential SSR errors or memory leaks caused by direct event listener attachments.
| import { getCachedOrCreateElement } from './markdown-renderer-helpers'; | ||
|
|
||
| const CACHE_SIZE = 100; | ||
|
|
||
| export type MarkdownRendererOptions = Record<string, never>; | ||
|
|
||
| interface ProcessOptions { | ||
| allowCodeBlocks?: boolean; | ||
| allowBlockquotes?: boolean; | ||
| allowTables?: boolean; | ||
| allowInlineCode?: boolean; | ||
| allowLinks?: boolean; | ||
| } |
There was a problem hiding this comment.
This block of code is a duplicate of lines 10-33 and contains conflicting definitions. Specifically, MarkdownRendererOptions is redefined as Record<string, never>, which will break the component implementation that expects the interface defined earlier. Please remove this entire duplicate block.
| * Create cache key for code block | ||
| */ | ||
| const createCodeBlockCacheKey = (language: string, content: string): string => { | ||
| return `${language}|||${content}|||${options?.copyCodeButton ?? ''}`; |
There was a problem hiding this comment.
The cache key for code blocks should include the current language or a locale identifier. Since the copyCodeButton label is translated inside the createHtml callback (which only runs on a cache miss), switching the application language will not update the labels for code blocks already present in the cache. Including a locale key in the cache key ensures that translations are correctly updated when the language changes.
| if (isInBrowser) { | ||
| div.querySelectorAll('.copy-code-btn').forEach(btn => { | ||
| btn.addEventListener('click', e => { | ||
| const button = e.target as HTMLButtonElement; | ||
| const codeId = button.getAttribute('data-code-id'); | ||
| if (!codeId) return; | ||
|
|
||
| const codeElement = div.querySelector(`#${codeId}`); | ||
| if (!codeElement) return; | ||
|
|
||
| const code = codeElement.textContent ?? ''; | ||
| navigator.clipboard.writeText(code).catch(() => { | ||
| console.warn('Failed to copy code to clipboard'); | ||
| }); | ||
| }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Adding event listeners directly to DOM elements inside the renderer can lead to memory leaks and will cause errors during Server-Side Rendering (SSR) as addEventListener is not available on the server. It is safer to use event delegation on the container div and ensure these browser-specific operations are wrapped in an isPlatformBrowser check. Given this is a standalone renderer function, consider adding a single listener to the root div that handles clicks for all copy buttons via data attributes.
References
- When using browser-dependent UI features, ensure they are only executed in a browser environment. Use a check like isPlatformBrowser to prevent errors during Server-Side Rendering (SSR).
Killusions
force-pushed
the
feat/markdown-renderer-copy-button
branch
from
8e56fac to
1d5aaf3
Compare
1 participant
Merge after #1810 and #1811
Adds a button to copy code blocks to the rendered markdown (with a translatable label)
Documentation.
Examples.
Dashboards Demo.
Playwright report.
Coverage Reports: