Dev#16
Conversation
There was a problem hiding this comment.
Code Review
This pull request replaces the webm-writer dependency with mediabunny to support both WebM and MP4 video exports, rewriting the video exporter and recorder to utilize native WebCodecs. It also updates the overlay UI with advanced video settings, refactors the examples gallery to load from a dynamic manifest, and modularizes several TypeDoc plugins. Feedback on the changes highlights a potential TypeError in the examples gallery if an entry's title is missing, and points out duplicate error progress reporting between VideoRecorder and VideoExporter.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| .map((entry) => { | ||
| const name = escapeHtml(entry.name); | ||
| const title = escapeHtml(entry.title); | ||
| const description = escapeHtml(entry.description); | ||
| return `<a href="${escapeHtml(getExampleHref(entry.path))}" class="entry-badge" data-entry="${escapeHtml(entry.name.toLowerCase())}" data-entry-name="${name}" data-entry-title="${escapeHtml(entry.title.toLowerCase())}" data-entry-description="${description}" aria-expanded="false" title="${title}">${name}</a>`; | ||
| }) |
There was a problem hiding this comment.
If entry.title is undefined (for example, if an entry in the manifest is missing a title field), calling entry.title.toLowerCase() will throw a TypeError and crash the rendering of the examples gallery. We should safely handle potentially missing titles by defaulting to an empty string before calling toLowerCase().
| .map((entry) => { | |
| const name = escapeHtml(entry.name); | |
| const title = escapeHtml(entry.title); | |
| const description = escapeHtml(entry.description); | |
| return `<a href="${escapeHtml(getExampleHref(entry.path))}" class="entry-badge" data-entry="${escapeHtml(entry.name.toLowerCase())}" data-entry-name="${name}" data-entry-title="${escapeHtml(entry.title.toLowerCase())}" data-entry-description="${description}" aria-expanded="false" title="${title}">${name}</a>`; | |
| }) | |
| .map((entry) => { | |
| const name = escapeHtml(entry.name); | |
| const titleText = entry.title || ''; | |
| const title = escapeHtml(titleText); | |
| const description = escapeHtml(entry.description); | |
| return `<a href="${escapeHtml(getExampleHref(entry.path))}" class="entry-badge" data-entry="${escapeHtml(entry.name.toLowerCase())}" data-entry-name="${name}" data-entry-title="${escapeHtml(titleText.toLowerCase())}" data-entry-description="${description}" aria-expanded="false" title="${title}">${name}</a>`; | |
| }) |
| } catch (error) { | ||
| const exportError = this._normalizeError(error); | ||
| onProgress?.({ state: 'error', message: exportError.message }); | ||
| throw exportError; | ||
| } |
There was a problem hiding this comment.
This catch block calls onProgress with the error state. However, VideoExporter.ts also catches any error thrown by $record and calls onProgress with the error state. This results in duplicate error progress events being emitted. We should simplify this catch block to only normalize and rethrow the error, leaving the high-level progress reporting to VideoExporter.ts.
} catch (error) {
throw this._normalizeError(error);
}
No description provided.