chore(docs): add component preview to card list - #202
Open
coryrylan wants to merge 1 commit into
Open
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughThe documentation site now renders lazily loaded interactive component previews with sanitized templates, mocks, metadata, and responsive styling. Navigation consolidates Code, Monaco, and Markdown entries under Elements. Component descriptions and examples receive content updates. ChangesDocumentation previews and navigation
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
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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
Comment on lines
+431
to
+443
| .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.
Suggested change
| .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.
coryrylan
force-pushed
the
topic-docs-improvements
branch
from
July 31, 2026 17:08
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
coryrylan
force-pushed
the
topic-docs-improvements
branch
from
July 31, 2026 21:27
d628c13 to
db101b8
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/core/src/drawer/drawer.ts`:
- Line 21: Update the JSDoc description for the Drawer symbol to use
grammatically correct singular wording and a direct description of its purpose,
preserving the existing examples of notifications, navigation, and settings.
In `@projects/core/src/notification/notification.ts`:
- Line 23: Update the JSDoc `@description` for the notification symbol to use
“real-time” as the modifier for “updates” and simplify the sentence according to
the repository’s technical writing style, while preserving the meaning that
notifications communicate important messages or status without interrupting the
user’s workflow.
In `@projects/core/src/tooltip/tooltip.ts`:
- Line 20: Update the JSDoc description for the tooltip component to state its
behavior directly with grammatically parallel wording, covering that it appears
on hover, focus, or tap and cannot contain interactive elements. Keep the change
limited to the description text and follow the repository’s technical writing
style.
In `@projects/site/src/docs/elements/index.11ty.js`:
- Around line 387-407: Update the preview sizing around
.component-preview-content and its corresponding styles near the additional
occurrence so .preview-page and .preview-editor mocks fit entirely within their
.component-preview containers, including narrow single-column and two-column
spans. Increase the dedicated container height or apply sufficient scaling based
on each mock’s dimensions, while preserving the existing centered layout and
avoiding clipping.
- Around line 339-356: The IntersectionObserver lifecycle and fallback loading
need adjustment in the preview initialization flow. Update the observer callback
around observer.unobserve and loadPreview so it disconnects once all preview
targets have been handled, and change the fallback branch to schedule preview
loads in chunks rather than invoking loadPreview for every preview in one task;
preserve loading every preview when IntersectionObserver is unavailable.
- Around line 315-320: Bound the custom-element readiness wait in the preview
initialization flow around customElementNames and Promise.all so unregistered
nve-* elements cannot keep the preview pending indefinitely. Add a finite
timeout that rejects or otherwise releases the wait, while preserving immediate
completion for elements resolved by customElements.whenDefined(name).
🪄 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: 26e58148-e8c8-4338-b5d1-2a4129b4545f
📒 Files selected for processing (9)
projects/core/src/dialog/dialog.tsprojects/core/src/drawer/drawer.tsprojects/core/src/gauge/gauge.examples.tsprojects/core/src/notification/notification.tsprojects/core/src/toast/toast.tsprojects/core/src/tooltip/tooltip.tsprojects/markdown/src/markdown/markdown.examples.tsprojects/site/src/_11ty/layouts/common.jsprojects/site/src/docs/elements/index.11ty.js
| /** | ||
| * @element nve-drawer | ||
| * @description Drawer are to display content that is out of context of the rest of the page (notifications, navigation, settings). Or use [Panel](./docs/elements/panel/) inline as its content couples with or closely relates to the content on the page (details, extra actions/options). [MDN Popover API](https://developer.mozilla.org/en-US/docs/Web/API/Popover_API) | ||
| * @description Drawer are to display content that is out of context of the rest of the page (notifications, navigation, settings). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the drawer description grammar.
Drawer are is incorrect. Use a singular subject and a direct description.
As per coding guidelines, TypeScript JSDoc comments are linted by Vale and should follow the repository's technical writing style.
Proposed wording
- * `@description` Drawer are to display content that is out of context of the rest of the page (notifications, navigation, settings).
+ * `@description` A drawer displays content separate from the rest of the page, such as notifications, navigation, or settings.📝 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.
Suggested change
| * @description Drawer are to display content that is out of context of the rest of the page (notifications, navigation, settings). | |
| * `@description` A drawer displays content separate from the rest of the page, such as notifications, navigation, or settings. |
🤖 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/core/src/drawer/drawer.ts` at line 21, Update the JSDoc description
for the Drawer symbol to use grammatically correct singular wording and a direct
description of its purpose, preserving the existing examples of notifications,
navigation, and settings.
Source: Coding guidelines
Comment on lines
+315
to
+320
| const customElementNames = new Set( | ||
| [...preview.querySelectorAll('*')] | ||
| .map(element => element.localName) | ||
| .filter(name => name.startsWith('nve-')) | ||
| ); | ||
| await Promise.all([...customElementNames].map(name => customElements.whenDefined(name))); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find the docs page entrypoint imports and every registered custom element name.
set -uo pipefail
fd -t f 'docs.ts' projects/site/src --exec rg -n '^\s*import' {} \;
fd -t f -e ts -e js . projects/site/src --exec rg -ln 'customElements\.define|`@customElement`' {} \;Repository: NVIDIA/elements
Length of output: 1451
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- target file context ---'
sed -n '270,355p' projects/site/src/docs/elements/index.11ty.js
printf '%s\n' '--- docs.ts candidates and imports ---'
fd -t f -i 'docs.ts' projects/site/src --exec sh -c 'echo "### $1"; sed -n "1,180p" "$1"' sh {}
printf '%s\n' '--- package and site entrypoint references ---'
rg -n --glob 'package.json' --glob '*.11ty.js' --glob '*.js' --glob '*.ts' \
'docs\.ts|index\.11ty|component-preview|componentPreview|customElements\.whenDefined|customElements\.define|`@customElement`' \
projects/site package.json 2>/dev/null | head -300Repository: NVIDIA/elements
Length of output: 15920
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- docs layout and generated script inclusion ---'
sed -n '1,120p' projects/site/src/_11ty/layouts/docs.11ty.js
sed -n '1,90p' projects/site/src/_11ty/layouts/docs.ts
sed -n '330,430p' projects/site/src/docs/elements/index.11ty.js
printf '%s\n' '--- preview generation and component metadata ---'
sed -n '1,275p' projects/site/src/docs/elements/index.11ty.js
rg -n --glob '*.js' --glob '*.ts' --glob '*.json' \
'component\.tag|tag:\s*['"'"'"]nve-|elements.*entry|custom-elements|`@nvidia-elements`|register' \
projects/site package.json
printf '%s\n' '--- relevant site/build configuration ---'
rg -n --glob 'package.json' --glob '*.js' --glob '*.ts' --glob '*.json' \
'docs\.11ty|layouts/docs|rollup|vite|entry|bundle|components' \
projects/site package.json | head -300Repository: NVIDIA/elements
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- Eleventy component entrypoint selection ---'
sed -n '45,145p' projects/site/eleventy.config.js
printf '%s\n' '--- base head and component scripts ---'
rg -n -A35 -B10 'function renderBaseHead|const renderBaseHead|renderBaseHead|entrypoints|componentModules|define\.js|bundles/index' \
projects/site/src/_11ty/layouts/common.js projects/site/eleventy.config.js
printf '%s\n' '--- component names and package metadata sources ---'
rg -n --glob '*.json' --glob '*.js' --glob '*.ts' \
'"name":\s*"nve-|customElements|custom-elements|metadata.*entrypoint|entrypoint.*define' \
packages projects 2>/dev/null | head -400
printf '%s\n' '--- exact preview tag universe ---'
python3 - <<'PY'
from pathlib import Path
import re
p = Path('projects/site/src/docs/elements/index.11ty.js').read_text()
for m in re.finditer(r"'(nve-[^']+)'", p):
print(m.group(1))
PYRepository: NVIDIA/elements
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -u
LOG=/tmp/coderabbit-shell-logs/shell-output-T0lemJ
printf '%s\n' '--- concise entrypoint configuration ---'
sed -n '1,115p' "$LOG"
printf '%s\n' '--- concise base-head matches ---'
rg -n -A25 -B8 'renderBaseHead|componentModules|entrypoints|bundles/index|define\.js' "$LOG" | head -240
printf '%s\n' '--- exact preview tags ---'
rg -n '^nve-' "$LOG" | tail -100Repository: NVIDIA/elements
Length of output: 22955
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- base head body ---'
sed -n '17,115p' projects/site/src/_11ty/layouts/common.js
printf '%s\n' '--- page runtime imports ---'
rg -n '^import|^void import|customElements\.whenDefined' \
projects/site/src/_11ty/layouts/docs.ts projects/site/src/_internal/canvas/canvas.ts \
projects/site/src/_internal/search/search.ts
printf '%s\n' '--- metadata shape and registered entrypoint coverage ---'
python3 - <<'PY'
import json
from pathlib import Path
path = Path('projects/internals/metadata/static/projects.json')
data = json.loads(path.read_text())
print(type(data).__name__)
if isinstance(data, dict):
print('top-level keys:', sorted(data.keys()))
projects = data.get('projects', data.get('data', []))
else:
projects = data
print('project count:', len(projects) if isinstance(projects, list) else 'not-list')
for project in projects if isinstance(projects, list) else []:
if project.get('name') == '`@nvidia-elements/core`':
print('core keys:', sorted(project.keys()))
elements = project.get('elements', project.get('manifest', {}).get('elements', []))
print('core element count:', len(elements) if isinstance(elements, list) else 'not-list')
for element in elements if isinstance(elements, list) else []:
name = element.get('name') or element.get('tag') or element.get('customElement')
meta = element.get('manifest', {}).get('metadata', element.get('metadata', {}))
entry = meta.get('entrypoint') if isinstance(meta, dict) else None
if name and name.startswith('nve-'):
print(f'{name}\t{entry}')
PY
printf '%s\n' '--- standalone promise behavior probe ---'
node - <<'JS'
const pending = new Promise(() => {});
const timeout = new Promise(resolve => setTimeout(() => resolve('timeout'), 5));
Promise.race([pending, timeout]).then(value => console.log(value));
setTimeout(() => console.log('underlying promise remains pending:', true), 10);
JSRepository: NVIDIA/elements
Length of output: 8597
🏁 Script executed:
#!/bin/bash
set -u
python3 - <<'PY'
from pathlib import Path
import re
runtime = '\n'.join(Path(p).read_text() for p in [
'projects/site/src/_11ty/layouts/docs.ts',
'projects/site/src/_internal/canvas/canvas.ts',
'projects/site/src/_internal/search/search.ts',
])
imports = set(re.findall(r"`@nvidia-elements/`[^'\" ]+/([^/'\" ]+)/define\.js", runtime))
runtime_tags = sorted(set(re.findall(r"`@nvidia-elements/`[^'\" ]+/([^/'\" ]+)/define\.js", runtime)))
print('runtime define entrypoints:', runtime_tags)
preview_source = Path('projects/site/src/docs/elements/index.11ty.js').read_text()
preview_tags = sorted(set(re.findall(r"'(nve-[a-z0-9-]+)'", preview_source)))
print('literal preview override tags:', preview_tags)
print('preview tags with no matching runtime entrypoint name:', [
tag for tag in preview_tags
if tag.removeprefix('nve-') not in imports
])
print('docs browser module imports:')
for line in Path('projects/site/src/_11ty/layouts/docs.11ty.js').read_text().splitlines():
if 'import ' in line and '<script' not in line:
print(line.strip())
PY
node - <<'JS'
const definitions = new Map([['nve-progress-ring', Promise.resolve()]]);
const customElements = {
whenDefined(name) {
return definitions.get(name) ?? new Promise(() => {});
}
};
const names = ['nve-grid', 'nve-progress-ring'];
const wait = Promise.all(names.map(name => customElements.whenDefined(name)));
const result = await Promise.race([
wait.then(() => 'all-defined'),
new Promise(resolve => setTimeout(() => resolve('timeout'), 5))
]);
console.log({ result });
JSRepository: NVIDIA/elements
Length of output: 1619
Bound the customElements.whenDefined wait.
The docs browser entrypoint does not register preview elements such as nve-grid or nve-tree. If a preview contains an unregistered nve-* element, Promise.all(...) remains pending, so the preview never becomes ready. Enforce a timeout without retaining an unbounded wait.
🤖 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 315 - 320, Bound
the custom-element readiness wait in the preview initialization flow around
customElementNames and Promise.all so unregistered nve-* elements cannot keep
the preview pending indefinitely. Add a finite timeout that rejects or otherwise
releases the wait, while preserving immediate completion for elements resolved
by customElements.whenDefined(name).
Comment on lines
+339
to
+356
| const previews = document.querySelectorAll('[data-component-preview]'); | ||
|
|
||
| if ('IntersectionObserver' in globalThis) { | ||
| const observer = new IntersectionObserver( | ||
| entries => { | ||
| entries.forEach(entry => { | ||
| if (!entry.isIntersecting) return; | ||
| observer.unobserve(entry.target); | ||
| void loadPreview(entry.target); | ||
| }); | ||
| }, | ||
| { rootMargin: '300px 0px' } | ||
| ); | ||
|
|
||
| previews.forEach(preview => observer.observe(preview)); | ||
| } else { | ||
| previews.forEach(preview => void loadPreview(preview)); | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
The observer is never disconnected, and the fallback branch loads every preview at once.
Line 353 observes every card. Line 346 unobserves each entry after it intersects, so the observer stays alive with zero targets for the page lifetime. That is acceptable, but a disconnect() after the last target keeps the intent explicit.
The fallback at line 355 starts loadPreview for every card in the same task. Each call queries the whole subtree twice and awaits two animation frames. On a page with the full component catalog this creates one long task. Consider chunking the fallback, or accept it because IntersectionObserver is available in every supported browser.
🤖 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 339 - 356, The
IntersectionObserver lifecycle and fallback loading need adjustment in the
preview initialization flow. Update the observer callback around
observer.unobserve and loadPreview so it disconnects once all preview targets
have been handled, and change the fallback branch to schedule preview loads in
chunks rather than invoking loadPreview for every preview in one task; preserve
loading every preview when IntersectionObserver is unavailable.
- 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>
coryrylan
force-pushed
the
topic-docs-improvements
branch
from
August 4, 2026 14:49
db101b8 to
8c2126b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (4)
projects/site/src/docs/elements/index.11ty.js:76
flattenNestedContainerschecksattributes.includes('container='), which can produce false positives (e.g.,data-container=) and skip addingcontainer="flat"when the actualcontainerattribute is absent. Use an attribute-boundary regex instead so only a realcontainer=attribute prevents injection.
const flattenNestedContainers = template =>
template.replace(/<(nve-(?:grid|accordion-group|accordion))(?=[\s>])([^>]*)>/g, (match, tag, attributes) =>
attributes.includes('container=') ? match : `<${tag} container="flat"${attributes}>`
);
projects/core/src/tooltip/tooltip.ts:21
- The updated tooltip description sentence is ungrammatical (“Trigger tooltips by … and cannot …”), which reads like it’s missing a subject for the second clause. Rephrase to keep “tooltips” as the subject for both clauses.
* @element nve-tooltip
* @description A contextual popup that displays a plaintext description. Trigger tooltips by hovering, focusing, or tapping an element and cannot have interactive elements within them.
* @documentation https://nvidia.github.io/elements/docs/elements/tooltip/
projects/core/src/drawer/drawer.ts:22
- The drawer description uses incorrect subject/verb agreement (“Drawer are”). Since this line was edited, it’s a good opportunity to correct the grammar.
* @element nve-drawer
* @description Drawer are to display content that is out of context of the rest of the page (notifications, navigation, settings).
* @documentation https://nvidia.github.io/elements/docs/elements/drawer/
projects/core/src/notification/notification.ts:24
- “Real time” should be hyphenated as a compound adjective (“real-time updates”) in this description.
* @element nve-notification
* @description Displays real time updates without interrupting the user's workflow to communicate an important message or status.
* @documentation https://nvidia.github.io/elements/docs/elements/notification/
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
projects/site/src/docs/elements/index.11ty.js (1)
260-280: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBuild each catalog entry in one
.mappass.The first
.mapsetspreview: ''. The second.mapoverwrites it unconditionally and allocates a second object per component. The placeholder is never read.♻️ Proposed single-pass construction
.map(element => { const doc = docsByTag.get(element.name); return { description: element.manifest?.description?.trim() ?? `Documentation for ${doc.data.title}.`, href: doc.url, packageName: element.package, - preview: '', + preview: getPreviewTemplate(element.name), tag: element.name, title: doc.data.title, version: element.version }; }) - .map(component => { - const preview = getPreviewTemplate(component.tag); - - return { - ...component, - preview - }; - }) .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 260 - 280, Combine the two transformation stages into a single `.map` callback that sets `preview` directly to `getPreviewTemplate(element.name)` while constructing each catalog entry. Remove the placeholder `preview: ''` and the subsequent `.map(component => ...)`, preserving the existing fields and values.
♻️ Duplicate comments (2)
projects/site/src/docs/elements/index.11ty.js (2)
322-324: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard the
nve-sparklinelookup.
querySelectorreturnsnullwhen the preview markup contains nonve-sparkline.getPreviewTemplatefalls back to<code nve-text="code">nve-sparkline</code>when noDefaultexample exists for the tag. The assignment then throws, and thecatchblock marksdata-preview-errorinstead of finishing initialization. Use optional chaining.🛡️ 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]; }🤖 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 322 - 324, Guard the nve-sparkline element lookup in the tag initialization block by using optional chaining before assigning its data, so missing preview markup does not throw and trigger the error path.
315-320: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound the
customElements.whenDefinedwait.
customElements.whenDefined(name)never settles until the name is registered. It does not reject and has no timeout. The docs page bundle does not register every preview tag, soPromise.allat line 320 can stay pending forever. The steps after it then never run: the sparkline seed at line 323 is skipped,updateCompleteis never awaited, andpreview.dataset.previewReadyat line 336 is never set. Thecatchblock never runs, so nothing is logged.Race the wait against a timeout.
🛡️ Proposed fix
+ 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 + );🤖 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 315 - 320, Bound the custom-element readiness wait in the preview initialization flow around customElementNames and Promise.all by racing each customElements.whenDefined(name) promise against a finite timeout, so unregistered preview tags cannot keep initialization pending forever. Preserve continuation to the subsequent seed, updateComplete, and previewReady steps when the timeout expires, while retaining the existing successful registration behavior.
🤖 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/_11ty/layouts/common.js`:
- Line 222: Add regression coverage for the navigation rendering logic around
the nve-tree-node entries: extend the existing tests to assert
/docs/code/codeblock/, /docs/markdown/, /docs/markdown/styles/, and each of the
five Monaco routes. Verify every route expands the Elements group where
applicable and marks the expected navigation entry as selected, while preserving
the existing /docs/elements/ and /docs/design-md/ assertions.
In `@projects/site/src/docs/elements/index.11ty.js`:
- Around line 463-468: Adjust the `.component-preview` sizing or the
`.preview-page` and `.preview-editor` mock dimensions so both remain fully
visible after `.component-preview-content` applies its 0.85 scale within the
180px-high, overflow-hidden container. Preserve the existing grid layout while
ensuring neither mock is vertically clipped in the single- and two-column spans.
---
Outside diff comments:
In `@projects/site/src/docs/elements/index.11ty.js`:
- Around line 260-280: Combine the two transformation stages into a single
`.map` callback that sets `preview` directly to
`getPreviewTemplate(element.name)` while constructing each catalog entry. Remove
the placeholder `preview: ''` and the subsequent `.map(component => ...)`,
preserving the existing fields and values.
---
Duplicate comments:
In `@projects/site/src/docs/elements/index.11ty.js`:
- Around line 322-324: Guard the nve-sparkline element lookup in the tag
initialization block by using optional chaining before assigning its data, so
missing preview markup does not throw and trigger the error path.
- Around line 315-320: Bound the custom-element readiness wait in the preview
initialization flow around customElementNames and Promise.all by racing each
customElements.whenDefined(name) promise against a finite timeout, so
unregistered preview tags cannot keep initialization pending forever. Preserve
continuation to the subsequent seed, updateComplete, and previewReady steps when
the timeout expires, while retaining the existing successful registration
behavior.
🪄 Autofix
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: 795ea63f-6008-49a4-96f6-29d8dbfdd1f9
📒 Files selected for processing (9)
projects/core/src/dialog/dialog.tsprojects/core/src/drawer/drawer.tsprojects/core/src/gauge/gauge.examples.tsprojects/core/src/notification/notification.tsprojects/core/src/toast/toast.tsprojects/core/src/tooltip/tooltip.tsprojects/markdown/src/markdown/markdown.examples.tsprojects/site/src/_11ty/layouts/common.jsprojects/site/src/docs/elements/index.11ty.js
| </nve-tree-node> | ||
|
|
||
| <nve-tree-node ${data.page.url.includes('/docs/elements/') ? 'expanded' : ''} ${data.page.url === '/docs/elements/' ? 'highlighted selected' : ''}> | ||
| <nve-tree-node ${data.page.url.includes('/docs/elements/') || data.page.url.includes('/docs/monaco/') || data.page.url.includes('/docs/markdown/') || data.page.url.includes('/docs/code/') ? 'expanded' : ''} ${data.page.url === '/docs/elements/' ? 'highlighted selected' : ''}> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Add regression tests for the relocated navigation entries.
The current tests cover /docs/elements/ and /docs/design-md/, but not the new route branches. Add assertions for /docs/code/codeblock/, /docs/markdown/, /docs/markdown/styles/, and all five Monaco routes. Verify the Elements group expands and the expected entry is selected.
Also applies to: 234-234, 289-297
🤖 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/_11ty/layouts/common.js` at line 222, Add regression
coverage for the navigation rendering logic around the nve-tree-node entries:
extend the existing tests to assert /docs/code/codeblock/, /docs/markdown/,
/docs/markdown/styles/, and each of the five Monaco routes. Verify every route
expands the Elements group where applicable and marks the expected navigation
entry as selected, while preserving the existing /docs/elements/ and
/docs/design-md/ assertions.
Comment on lines
+463
to
+468
| .preview-page { | ||
| width: 560px; | ||
| height: 260px; | ||
| overflow: hidden; | ||
| background: var(--nve-sys-layer-canvas-background); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
.preview-page and .preview-editor are clipped by the 180px preview container.
.component-preview sets height: 180px and overflow: hidden. .component-preview-content scales by 0.85. .preview-page is 560×260, so it still needs about 476×221. .preview-editor is 430×180, so it still needs about 366×153 and fits the height only exactly. Both mocks are cropped in the narrow single-column and two-column grid spans defined at line 599.
Scale these two mocks down, or raise the container height for them.
🎨 Proposed fix
.preview-page {
width: 560px;
height: 260px;
overflow: hidden;
background: var(--nve-sys-layer-canvas-background);
+ transform: scale(0.62);
}Also applies to: 539-546
🤖 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 463 - 468, Adjust
the `.component-preview` sizing or the `.preview-page` and `.preview-editor`
mock dimensions so both remain fully visible after `.component-preview-content`
applies its 0.85 scale within the 180px-high, overflow-hidden container.
Preserve the existing grid layout while ensuring neither mock is vertically
clipped in the single- and two-column spans.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Updates