chore(docs): add component preview to card list - #202
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughThe default gauge example now shows one 66% gauge. The elements documentation now renders lazily loaded interactive component previews with generated templates, mocks, preview metadata, and responsive styling. ChangesInteractive component previews
Gauge example update
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant IntersectionObserver
participant PreviewLoader
participant ComponentModule
IntersectionObserver->>PreviewLoader: observe preview card
PreviewLoader->>ComponentModule: import required module
PreviewLoader->>ComponentModule: initialize preview data
ComponentModule-->>PreviewLoader: component update
PreviewLoader-->>IntersectionObserver: mark preview ready or error
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
projects/core/src/gauge/gauge.examples.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. projects/site/src/docs/elements/index.11ty.jsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@projects/site/src/docs/elements/index.11ty.js`:
- Line 38: Update stripScripts to allow optional whitespace before the closing
angle bracket in </script> end tags, and repeatedly apply the replacement until
the template output no longer changes. Preserve the existing trimming behavior
and ensure residual or overlapping <script fragments are removed once the result
stabilizes.
- Around line 338-340: Update the generated script value in renderPreviewScript,
specifically the component tag and import entrypoint serialization around
component.imports.map, to escape the HTML-sensitive `</script` sequence after
JSON.stringify. Use a small shared helper for safe inline-script serialization
and apply it to both serialized values without changing the generated
module-loading behavior.
- Around line 431-443: Update the .component-preview[data-preview-ready]
.component-preview-content rule to reset the initial scale transform to full
size, and extend the .component-preview-content transition to include transform
alongside opacity.
- Around line 409-415: Add a .component-card-link:focus-visible rule that
applies a clear visible outline to the overlay link, ensuring keyboard focus
remains distinguishable while preserving the existing card-link positioning and
default non-focused appearance.
- Around line 54-57: Add a concise comment immediately above renderMonacoMock
documenting that tags containing “diff” return the two-pane Changes markup, tags
containing “problems” return the Problems list markup, and other tags return the
default pipeline.ts markup; keep the existing substring-based selection logic
unchanged.
- Around line 356-358: Guard the nve-sparkline lookup in the preview
initialization block with optional chaining before assigning its data, so a
missing element is skipped without throwing. Keep the existing sparkline data
and allow the remaining preview setup to continue normally.
- Line 324: Update the heading interpolation in the component rendering template
to pass component.title through the existing escapeHtml helper, matching the
escaping already used for the description and aria-label values.
- Around line 208-249: Add CSS rules in the <style> block for the
data-preview-size values produced by getPreviewSize, overriding
.component-preview’s fixed height so large previews (including nve-page and
nve-grid) are not clipped and wide previews receive their intended sizing. Keep
renderComponentCard’s data-preview-size attribute and the existing largePreviews
and widePreviews classifications intact.
- Around line 287-306: Collapse the chained maps in the component entry
construction into a single map. Within that map, call
getPreviewTemplate(component.tag) and construct each object once with
getPreviewImports(preview), preview, and getPreviewSize(component.tag), while
preserving the existing fields and final sortByTitle ordering.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 9487593d-b539-4fd9-87dd-5d7f81d41efa
📒 Files selected for processing (2)
projects/core/src/gauge/gauge.examples.tsprojects/site/src/docs/elements/index.11ty.js
| const renderMonacoMock = tag => { | ||
| const isDiff = tag.includes('diff'); | ||
| const isProblems = tag.includes('problems'); | ||
| const title = isProblems ? 'Problems' : isDiff ? 'Changes' : 'pipeline.ts'; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Document the tag-substring contract of renderMonacoMock.
The variant selection depends on substrings of tag, not on an explicit parameter. A reader cannot tell from the signature that nve-monaco-diff-input produces the two-pane variant and that nve-monaco-problems produces the problems list. Add a short comment that states the accepted tags and the returned markup.
As per coding guidelines: "Document agent capabilities, constraints, and expected inputs/outputs in code comments or docstrings".
📝 Proposed doc comment
+/**
+ * Builds a static Monaco preview mock for a documentation card.
+ *
+ * `@param` {string} tag Component tag. A tag that contains `diff` returns a two-pane
+ * view. A tag that contains `problems` returns a problems list. Any other tag
+ * returns a single code pane.
+ * `@returns` {string} Preview markup.
+ */
const renderMonacoMock = tag => {📝 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 renderMonacoMock = tag => { | |
| const isDiff = tag.includes('diff'); | |
| const isProblems = tag.includes('problems'); | |
| const title = isProblems ? 'Problems' : isDiff ? 'Changes' : 'pipeline.ts'; | |
| /** | |
| * Builds a static Monaco preview mock for a documentation card. | |
| * | |
| * `@param` {string} tag Component tag. A tag that contains `diff` returns a two-pane | |
| * view. A tag that contains `problems` returns a problems list. Any other tag | |
| * returns a single code pane. | |
| * `@returns` {string} Preview markup. | |
| */ | |
| const renderMonacoMock = tag => { | |
| const isDiff = tag.includes('diff'); | |
| const isProblems = tag.includes('problems'); | |
| const title = isProblems ? 'Problems' : isDiff ? 'Changes' : 'pipeline.ts'; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@projects/site/src/docs/elements/index.11ty.js` around lines 54 - 57, Add a
concise comment immediately above renderMonacoMock documenting that tags
containing “diff” return the two-pane Changes markup, tags containing “problems”
return the Problems list markup, and other tags return the default pipeline.ts
markup; keep the existing substring-based selection logic unchanged.
Source: Coding guidelines
| const largePreviews = new Set([ | ||
| 'nve-card', | ||
| 'nve-grid', | ||
| 'nve-markdown', | ||
| 'nve-menu', | ||
| 'nve-page', | ||
| 'nve-panel', | ||
| 'nve-tree' | ||
| ]); | ||
|
|
||
| const widePreviews = new Set([ | ||
| 'nve-accordion', | ||
| 'nve-alert', | ||
| 'nve-breadcrumb', | ||
| 'nve-button-group', | ||
| 'nve-chat-message', | ||
| 'nve-codeblock', | ||
| 'nve-combobox', | ||
| 'nve-control', | ||
| 'nve-dropzone', | ||
| 'nve-input', | ||
| 'nve-input-group', | ||
| 'nve-page-header', | ||
| 'nve-pagination', | ||
| 'nve-preferences-input', | ||
| 'nve-progress-bar', | ||
| 'nve-progressive-filter-chip', | ||
| 'nve-radio', | ||
| 'nve-search', | ||
| 'nve-select', | ||
| 'nve-skeleton', | ||
| 'nve-sparkline', | ||
| 'nve-star-rating', | ||
| 'nve-steps', | ||
| 'nve-tabs', | ||
| 'nve-textarea', | ||
| 'nve-time', | ||
| 'nve-toolbar', | ||
| 'nve-week' | ||
| ]); | ||
|
|
||
| const getPreviewSize = tag => (largePreviews.has(tag) ? 'large' : widePreviews.has(tag) ? 'wide' : 'default'); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
previewSize is computed and serialized, but no CSS consumes it.
getPreviewSize classifies each tag as large, wide, or default. renderComponentCard writes the result to data-preview-size at line 316. The <style> block at lines 398-626 contains no [data-preview-size] selector, so .component-preview keeps a fixed height: 180px for every card. Large previews such as nve-page (260px tall) and nve-grid are then clipped by overflow: hidden, and the two Sets are dead configuration.
Add rules for the two size variants, or remove largePreviews, widePreviews, getPreviewSize, and the data-preview-size attribute.
🎨 Proposed CSS rules for the size variants
Add inside the <style> block, after the .component-preview rule:
+ .component-preview[data-preview-size='wide'] {
+ height: 200px;
+ }
+
+ .component-preview[data-preview-size='large'] {
+ height: 280px;
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@projects/site/src/docs/elements/index.11ty.js` around lines 208 - 249, Add
CSS rules in the <style> block for the data-preview-size values produced by
getPreviewSize, overriding .component-preview’s fixed height so large previews
(including nve-page and nve-grid) are not clipped and wide previews receive
their intended sizing. Keep renderComponentCard’s data-preview-size attribute
and the existing largePreviews and widePreviews classifications intact.
| imports: [], | ||
| packageName: element.package, | ||
| preview: '', | ||
| previewSize: 'default', | ||
| tag: element.name, | ||
| title: doc.data.title, | ||
| version: element.version | ||
| }; | ||
| }) | ||
| .map(component => { | ||
| const preview = getPreviewTemplate(component.tag); | ||
|
|
||
| return { | ||
| ...component, | ||
| imports: getPreviewImports(preview), | ||
| preview, | ||
| previewSize: getPreviewSize(component.tag) | ||
| }; | ||
| }) | ||
| .sort(sortByTitle); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Collapse the two .map passes into one.
The first .map sets imports: [], preview: '', and previewSize: 'default'. The second .map overwrites all three unconditionally. The placeholders are never read, and the spread allocates a second object per component. Build the entry once.
♻️ Proposed single-pass construction
.map(element => {
const doc = docsByTag.get(element.name);
+ const preview = getPreviewTemplate(element.name);
return {
description: element.manifest?.description?.trim() ?? `Documentation for ${doc.data.title}.`,
href: doc.url,
- imports: [],
+ imports: getPreviewImports(preview),
packageName: element.package,
- preview: '',
- previewSize: 'default',
+ preview,
+ previewSize: getPreviewSize(element.name),
tag: element.name,
title: doc.data.title,
version: element.version
};
})
- .map(component => {
- const preview = getPreviewTemplate(component.tag);
-
- return {
- ...component,
- imports: getPreviewImports(preview),
- preview,
- previewSize: getPreviewSize(component.tag)
- };
- })
.sort(sortByTitle);📝 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.
| imports: [], | |
| packageName: element.package, | |
| preview: '', | |
| previewSize: 'default', | |
| tag: element.name, | |
| title: doc.data.title, | |
| version: element.version | |
| }; | |
| }) | |
| .map(component => { | |
| const preview = getPreviewTemplate(component.tag); | |
| return { | |
| ...component, | |
| imports: getPreviewImports(preview), | |
| preview, | |
| previewSize: getPreviewSize(component.tag) | |
| }; | |
| }) | |
| .sort(sortByTitle); | |
| .map(element => { | |
| const doc = docsByTag.get(element.name); | |
| const preview = getPreviewTemplate(element.name); | |
| return { | |
| description: element.manifest?.description?.trim() ?? `Documentation for ${doc.data.title}.`, | |
| href: doc.url, | |
| imports: getPreviewImports(preview), | |
| packageName: element.package, | |
| preview, | |
| previewSize: getPreviewSize(element.name), | |
| tag: element.name, | |
| title: doc.data.title, | |
| version: element.version | |
| }; | |
| }) | |
| .sort(sortByTitle); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@projects/site/src/docs/elements/index.11ty.js` around lines 287 - 306,
Collapse the chained maps in the component entry construction into a single map.
Within that map, call getPreviewTemplate(component.tag) and construct each
object once with getPreviewImports(preview), preview, and
getPreviewSize(component.tag), while preserving the existing fields and final
sortByTitle ordering.
| const imports = component.imports.map(entrypoint => `import(${JSON.stringify(entrypoint)})`).join(', '); | ||
|
|
||
| return `${JSON.stringify(component.tag)}: () => ${imports ? `Promise.all([${imports}])` : 'Promise.resolve()'}`; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
JSON.stringify does not neutralize </script inside an inline module.
renderPreviewScript embeds the generated object literal in a <script type="module"> block at line 345. JSON.stringify escapes quotes and backslashes, but it leaves </script intact. A tag or entrypoint that contains that sequence would close the script element early and inject markup. The current values come from siteData, so this is not exploitable today. CodeQL flags line 340, and the fix is a one-line helper.
🔒 Proposed fix
+const toScriptLiteral = value => JSON.stringify(value).replace(/</g, '\\u003c');
+
const renderPreviewScript = components => {
const loaders = components
.map(component => {
- const imports = component.imports.map(entrypoint => `import(${JSON.stringify(entrypoint)})`).join(', ');
+ const imports = component.imports.map(entrypoint => `import(${toScriptLiteral(entrypoint)})`).join(', ');
- return `${JSON.stringify(component.tag)}: () => ${imports ? `Promise.all([${imports}])` : 'Promise.resolve()'}`;
+ return `${toScriptLiteral(component.tag)}: () => ${imports ? `Promise.all([${imports}])` : 'Promise.resolve()'}`;
})🧰 Tools
🪛 GitHub Check: CodeQL
[warning] 340-340: Improper code sanitization
Code construction depends on an improperly sanitized value.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@projects/site/src/docs/elements/index.11ty.js` around lines 338 - 340, Update
the generated script value in renderPreviewScript, specifically the component
tag and import entrypoint serialization around component.imports.map, to escape
the HTML-sensitive `</script` sequence after JSON.stringify. Use a small shared
helper for safe inline-script serialization and apply it to both serialized
values without changing the generated module-loading behavior.
Source: Linters/SAST tools
| if (tag === 'nve-sparkline') { | ||
| preview.querySelector('nve-sparkline').data = [18, 22, 20, 24, 19, 28, 25, 30]; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard the nve-sparkline lookup against a missing element.
querySelector returns null when the preview markup contains no nve-sparkline. That happens when no Default example exists for the tag, because getPreviewTemplate then falls back to <code nve-text="code">nve-sparkline</code>. The property assignment throws, the catch at line 365 marks data-preview-error, and the failure is visible only in the console. Use optional chaining so the rest of the preview still initializes.
🛡️ Proposed fix
if (tag === 'nve-sparkline') {
- preview.querySelector('nve-sparkline').data = [18, 22, 20, 24, 19, 28, 25, 30];
+ const sparkline = preview.querySelector('nve-sparkline');
+
+ if (sparkline) sparkline.data = [18, 22, 20, 24, 19, 28, 25, 30];
}📝 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.
| if (tag === 'nve-sparkline') { | |
| preview.querySelector('nve-sparkline').data = [18, 22, 20, 24, 19, 28, 25, 30]; | |
| } | |
| if (tag === 'nve-sparkline') { | |
| const sparkline = preview.querySelector('nve-sparkline'); | |
| if (sparkline) sparkline.data = [18, 22, 20, 24, 19, 28, 25, 30]; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@projects/site/src/docs/elements/index.11ty.js` around lines 356 - 358, Guard
the nve-sparkline lookup in the preview initialization block with optional
chaining before assigning its data, so a missing element is skipped without
throwing. Keep the existing sparkline data and allow the remaining preview setup
to continue normally.
| .component-card-link { | ||
| position: absolute; | ||
| z-index: 2; | ||
| inset: 0; | ||
| border-radius: var(--nve-ref-border-radius-lg); | ||
| cursor: pointer; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add a visible focus indicator to the overlay link.
.component-card-link covers the whole card and is the only focusable target. The rule sets no focus style, and the default outline is drawn on a transparent, zero-content element that sits above the card, so it is easy to lose. A keyboard user cannot tell which card is focused. Add a :focus-visible outline.
♿ Proposed fix
.component-card-link {
position: absolute;
z-index: 2;
inset: 0;
border-radius: var(--nve-ref-border-radius-lg);
cursor: pointer;
}
+
+ .component-card-link:focus-visible {
+ outline: var(--nve-ref-border-width-lg) solid var(--nve-sys-interaction-emphasis-background);
+ outline-offset: var(--nve-ref-space-xs);
+ }📝 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.
| .component-card-link { | |
| position: absolute; | |
| z-index: 2; | |
| inset: 0; | |
| border-radius: var(--nve-ref-border-radius-lg); | |
| cursor: pointer; | |
| } | |
| .component-card-link { | |
| position: absolute; | |
| z-index: 2; | |
| inset: 0; | |
| border-radius: var(--nve-ref-border-radius-lg); | |
| cursor: pointer; | |
| } | |
| .component-card-link:focus-visible { | |
| outline: var(--nve-ref-border-width-lg) solid var(--nve-sys-interaction-emphasis-background); | |
| outline-offset: var(--nve-ref-space-xs); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@projects/site/src/docs/elements/index.11ty.js` around lines 409 - 415, Add a
.component-card-link:focus-visible rule that applies a clear visible outline to
the overlay link, ensuring keyboard focus remains distinguishable while
preserving the existing card-link positioning and default non-focused
appearance.
| .component-preview-content { | ||
| display: grid; | ||
| place-items: center; | ||
| width: 100%; | ||
| height: 100%; | ||
| opacity: 0; | ||
| transform: scale(0.82); | ||
| transition: opacity var(--nve-ref-animation-duration-150); | ||
| } | ||
|
|
||
| .component-preview[data-preview-ready] .component-preview-content { | ||
| opacity: 1; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The scale(0.82) transform is never reset, so every preview stays shrunk.
Line 437 applies transform: scale(0.82). Line 438 transitions opacity only. The ready rule at lines 441-443 changes opacity alone. No rule removes the transform, so each preview renders permanently at 82% scale instead of animating to full size. Reset the transform in the ready state and include it in the transition.
🐛 Proposed fix
opacity: 0;
transform: scale(0.82);
- transition: opacity var(--nve-ref-animation-duration-150);
+ transition:
+ opacity var(--nve-ref-animation-duration-150),
+ transform var(--nve-ref-animation-duration-150);
}
.component-preview[data-preview-ready] .component-preview-content {
opacity: 1;
+ transform: scale(1);
}📝 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.
| .component-preview-content { | |
| display: grid; | |
| place-items: center; | |
| width: 100%; | |
| height: 100%; | |
| opacity: 0; | |
| transform: scale(0.82); | |
| transition: opacity var(--nve-ref-animation-duration-150); | |
| } | |
| .component-preview[data-preview-ready] .component-preview-content { | |
| opacity: 1; | |
| } | |
| .component-preview-content { | |
| display: grid; | |
| place-items: center; | |
| width: 100%; | |
| height: 100%; | |
| opacity: 0; | |
| transform: scale(0.82); | |
| transition: | |
| opacity var(--nve-ref-animation-duration-150), | |
| transform var(--nve-ref-animation-duration-150); | |
| } | |
| .component-preview[data-preview-ready] .component-preview-content { | |
| opacity: 1; | |
| transform: scale(1); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@projects/site/src/docs/elements/index.11ty.js` around lines 431 - 443, Update
the .component-preview[data-preview-ready] .component-preview-content rule to
reset the initial scale transform to full size, and extend the
.component-preview-content transition to include transform alongside opacity.
- add small inline preview of the component to the card - large components like monaco have small mock html/css inlined visuals Signed-off-by: Cory Rylan <crylan@nvidia.com>
670501c to
d628c13
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@projects/site/src/docs/elements/index.11ty.js`:
- Around line 359-380: Bound the custom element definition wait in the
preview-loading flow by adding a timeout or equivalent settled fallback around
customElements.whenDefined(name), so an unregistered nve-* tag cannot block
Promise.all indefinitely. Ensure timeout failures are handled by the existing
error path and preview.dataset.previewReady is always set, including when
definition waiting fails or times out.
- Around line 36-57: Remove the redundant `xlink:href` entry from
`previewUrlAttributes`; `sanitizePreviewNode` should continue handling
namespaced xlink attributes through the existing `href` name check and
`isSafePreviewUrl` validation.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 55593e3b-d155-4e9b-9ee7-578fc3a54f37
📒 Files selected for processing (2)
projects/core/src/gauge/gauge.examples.tsprojects/site/src/docs/elements/index.11ty.js
| const previewUrlAttributes = new Set(['action', 'formaction', 'href', 'poster', 'src', 'xlink:href']); | ||
| const safePreviewProtocols = new Set(['http:', 'https:', 'mailto:', 'tel:']); | ||
|
|
||
| const isSafePreviewUrl = value => { | ||
| try { | ||
| return safePreviewProtocols.has(new URL(value, 'https://elements.invalid').protocol); | ||
| } catch { | ||
| return false; | ||
| } | ||
| }; | ||
|
|
||
| const sanitizePreviewNode = node => { | ||
| if (!Array.isArray(node.childNodes)) return; | ||
| node.childNodes = node.childNodes.filter(child => !disallowedPreviewElements.has(child.nodeName)); | ||
| node.childNodes.forEach(child => { | ||
| if (Array.isArray(child.attrs)) { | ||
| child.attrs = child.attrs.filter(attribute => { | ||
| const name = attribute.name.toLowerCase(); | ||
| if (name.startsWith('on') || name === 'srcdoc') return false; | ||
| if (previewUrlAttributes.has(name)) return isSafePreviewUrl(attribute.value); | ||
| return true; | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial | 💤 Low value
🧩 Analysis chain
🌐 Web query:
parse5 7 attribute prefix namespace xlink:href attrs representation
💡 Result:
In parse5, attributes are represented as objects within an array [1][2]. The Attribute interface includes four primary properties: name (string), value (string), and two optional properties for namespace-aware handling: namespace (string) and prefix (string) [3][4]. For foreign elements such as those in SVG or MathML, attributes like xlink:href are typically represented with these fields populated [5]. For example, an xlink:href attribute is often represented with the name set to href, the prefix set to xlink, and the namespace set to the corresponding URI (e.g., http://www.w3.org/1999/xlink) [5]. This structure is consistent across the parse5 ecosystem, where the tree adapter's createElement and getAttrList methods utilize this Attribute interface to handle attributes, specifically noting that foreign attributes may contain namespace and prefix fields [6][2][7]. It is important to note that parse5 is an HTML5-compliant parser, not an XML parser; consequently, its handling of namespaces is governed by the HTML5 specification's foreign content parsing rules [8][5].
Citations:
- 1: https://github.com/inikulin/parse5/blob/master/packages/parse5/lib/tree-adapters/default.ts
- 2: https://parse5.js.org/interfaces/parse5.TreeAdapter.html
- 3: https://parse5.js.org/interfaces/parse5.Token.Attribute.html
- 4: https://gitea.rs.h3.ink/actions/setup-python/src/commit/04b88679183d4f500054e12d8bc671140dae29e8/node_modules/parse5/lib/index.d.ts
- 5: consider namespaces cburgmer/xmlserializer#11
- 6: https://github.com/inikulin/parse5/blob/cf1b2df71efd487f35dcbeb3d07bff58c0c83131/packages/parse5/lib/tree-adapters/interface.ts
- 7: https://github.com/inikulin/parse5/blob/0d56627fc924d40f560fd260ade0e1a935e2369c/packages/parse5/lib/tree-adapters/interface.ts
- 8: Incorrect image element tag parsing inikulin/parse5#243
Remove the redundant xlink:href entry. parse5 reports namespaced attributes as name: 'href' with prefix: 'xlink', so the existing href check still rejects unsafe protocols.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@projects/site/src/docs/elements/index.11ty.js` around lines 36 - 57, Remove
the redundant `xlink:href` entry from `previewUrlAttributes`;
`sanitizePreviewNode` should continue handling namespaced xlink attributes
through the existing `href` name check and `isSafePreviewUrl` validation.
| const customElementNames = new Set( | ||
| [...preview.querySelectorAll('*')] | ||
| .map(element => element.localName) | ||
| .filter(name => name.startsWith('nve-')) | ||
| ); | ||
| await Promise.all([...customElementNames].map(name => customElements.whenDefined(name))); | ||
|
|
||
| if (tag === 'nve-sparkline') { | ||
| preview.querySelector('nve-sparkline').data = [18, 22, 20, 24, 19, 28, 25, 30]; | ||
| } | ||
|
|
||
| const updates = [...preview.querySelectorAll('*')] | ||
| .map(element => element.updateComplete) | ||
| .filter(Boolean); | ||
| await Promise.all(updates); | ||
| await new Promise(resolve => requestAnimationFrame(() => requestAnimationFrame(resolve))); | ||
| } catch (error) { | ||
| console.error('Unable to load component preview', { error, tag }); | ||
| preview.dataset.previewError = ''; | ||
| } | ||
|
|
||
| preview.dataset.previewReady = ''; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift
A never-registered nve-* tag leaves the card permanently blank.
customElements.whenDefined(name) never settles until the name is registered. It does not reject and has no timeout. The generated script no longer imports any component entrypoint, so registration depends entirely on the page bundle. If one preview contains a tag that the bundle does not register, await Promise.all(...) at line 364 never resolves, preview.dataset.previewReady at line 380 is never set, and .component-preview-content stays at opacity: 0. The catch never runs, so no error is logged.
Bound the wait and always set the ready flag.
🛡️ Proposed fix to bound the definition wait
+ const withTimeout = (promise, ms) =>
+ Promise.race([promise, new Promise(resolve => setTimeout(resolve, ms))]);
+
const loadPreview = async preview => {
const tag = preview.dataset.componentPreview;
try {
const customElementNames = new Set(
[...preview.querySelectorAll('*')]
.map(element => element.localName)
.filter(name => name.startsWith('nve-'))
);
- await Promise.all([...customElementNames].map(name => customElements.whenDefined(name)));
+ await withTimeout(
+ Promise.all([...customElementNames].map(name => customElements.whenDefined(name))),
+ 2000
+ );
if (tag === 'nve-sparkline') {
preview.querySelector('nve-sparkline').data = [18, 22, 20, 24, 19, 28, 25, 30];
}
const updates = [...preview.querySelectorAll('*')]
.map(element => element.updateComplete)
.filter(Boolean);
- await Promise.all(updates);
+ await withTimeout(Promise.all(updates), 2000);
await new Promise(resolve => requestAnimationFrame(() => requestAnimationFrame(resolve)));
} catch (error) {
console.error('Unable to load component preview', { error, tag });
preview.dataset.previewError = '';
+ } finally {
+ preview.dataset.previewReady = '';
}
-
- preview.dataset.previewReady = '';
};Run the following script to check which entrypoints the page bundle registers:
#!/bin/bash
# Description: Find what the docs layout imports and whether every nve-* element is registered on this page.
set -uo pipefail
fd -t f 'docs.ts' projects/site/src | while IFS= read -r f; do
echo "=== $f ==="
rg -n "^\s*import" "$f"
done
# Look for a barrel/entrypoint that registers all elements.
fd -t f -e ts -e js . projects/site/src --exec rg -ln 'customElements\.define|defineCustomElement' {} \; | head -50🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@projects/site/src/docs/elements/index.11ty.js` around lines 359 - 380, Bound
the custom element definition wait in the preview-loading flow by adding a
timeout or equivalent settled fallback around customElements.whenDefined(name),
so an unregistered nve-* tag cannot block Promise.all indefinitely. Ensure
timeout failures are handled by the existing error path and
preview.dataset.previewReady is always set, including when definition waiting
fails or times out.
Summary by CodeRabbit
New Features
Updates