Skip to content

Topic tool cleanup - #198

Merged
coryrylan merged 4 commits into
mainfrom
topic-tool-cleanup
Jul 30, 2026
Merged

Topic tool cleanup#198
coryrylan merged 4 commits into
mainfrom
topic-tool-cleanup

Conversation

@coryrylan

@coryrylan coryrylan commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • New Features
    • Added a lint rule that restricts container="full" to direct children of the page, including grid-specific guidance to prefer container="flat".
  • Bug Fixes
    • Updated alert, card, grid, toolbar, and visual test templates/examples to consistently wrap content in nve-page and adjusted related container modes.
    • Added/standardized --gap support for page spacing.
  • Documentation
    • Documented the new lint rule and updated container guidance, migration guidance, and the grid “Full” example.
  • Tests
    • Marked relevant Storybook examples as test cases and added lint rule test coverage.

@coryrylan coryrylan self-assigned this Jul 30, 2026
Copilot AI review requested due to automatic review settings July 30, 2026 04:02

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The PR adds an ESLint rule restricting container="full" placement, updates affected examples and documentation to use nve-page, exposes configurable page spacing, adjusts related pattern layouts and styles, updates package metadata, and centralizes an MCP server dependency version.

Changes

Full-container placement and page layouts

Layer / File(s) Summary
Full-container lint rule and validation
projects/lint/src/eslint/rules/*, projects/lint/src/eslint/configs/html.ts, projects/lint/README.md, projects/site/src/docs/lint/index.md
Adds and registers no-restricted-container-full, including grid-specific suggestions and placement tests.
Component examples and documentation migration
projects/core/src/{alert,card,grid,toolbar,toggletip}/*, projects/core/eslint.config.js, projects/internals/tools/src/skills/migration.md, projects/site/src/docs/elements/data-grid/container.md
Moves full-container examples under nve-page, updates test-case metadata, and removes unsupported full-container usage.
Page spacing and pattern layouts
projects/core/src/page/*, projects/internals/patterns/src/*, projects/styles/src/layout.css
Adds configurable page --gap spacing and revises editor, template, browse, and layout structures.

MCP server catalog configuration

Layer / File(s) Summary
MCP server catalog wiring
pnpm-workspace.yaml, projects/cli/package.json
Pins @modelcontextprotocol/server in the workspace catalog and changes the CLI dependency to use catalog:.

Package metadata and site markup

Layer / File(s) Summary
Package keyword metadata
projects/cli/package.json, projects/lint/package.json, projects/styles/package.json
Replaces or expands package keyword arrays.
Site feature structure and layout styling
projects/site/src/index.md, projects/styles/src/layout.css
Changes hero feature blocks to section and heading markup and adds a full-width selector for spaced row layouts.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Developer
  participant ESLint
  participant noRestrictedContainerFull
  participant HTMLAST
  Developer->>ESLint: lint HTML template
  ESLint->>noRestrictedContainerFull: run rule
  noRestrictedContainerFull->>HTMLAST: inspect full-container ancestry
  HTMLAST-->>noRestrictedContainerFull: return element context
  noRestrictedContainerFull-->>ESLint: report error or grid suggestion
  ESLint-->>Developer: display lint result
Loading

Suggested reviewers: copilot

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title is very generic and doesn’t clearly describe the main changes in linting, docs, and page/container cleanup. Use a more specific title that reflects the primary change, such as adding the container-full lint rule and related page/layout updates.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch topic-tool-cleanup

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/lint/src/eslint/rules/no-restricted-container-full.ts`:
- Around line 36-47: Update the no-restricted-container-full rule description to
state that container="full" is permitted for template-root elements and direct
children of nve-page. Update the metadata assertion in
projects/lint/src/eslint/rules/no-restricted-container-full.test.ts (lines
42-61), the rule documentation in projects/lint/README.md (line 89), and the
site documentation in projects/site/src/docs/lint/index.md (lines 211-216) to
describe both permitted placements.
🪄 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: 8b59082c-25b3-49a8-b85b-6bf1f6090974

📥 Commits

Reviewing files that changed from the base of the PR and between 1521595 and cc511bc.

📒 Files selected for processing (22)
  • projects/core/eslint.config.js
  • projects/core/src/alert/alert.examples.ts
  • projects/core/src/card/card.examples.ts
  • projects/core/src/grid/grid.examples.ts
  • projects/core/src/grid/grid.test.visual.ts
  • projects/core/src/page/page.css
  • projects/core/src/page/page.ts
  • projects/core/src/toggletip/toggletip.examples.ts
  • projects/core/src/toolbar/toolbar.examples.ts
  • projects/internals/patterns/src/browse.examples.ts
  • projects/internals/patterns/src/editor.examples.ts
  • projects/internals/patterns/src/templates.examples.ts
  • projects/internals/tools/src/skills/migration.md
  • projects/lint/README.md
  • projects/lint/src/eslint/configs/html.ts
  • projects/lint/src/eslint/rules/no-nested-container-types.test.ts
  • projects/lint/src/eslint/rules/no-nested-container-types.ts
  • projects/lint/src/eslint/rules/no-restricted-container-full.test.ts
  • projects/lint/src/eslint/rules/no-restricted-container-full.ts
  • projects/site/src/docs/elements/data-grid/container.md
  • projects/site/src/docs/lint/index.md
  • projects/styles/src/layout.css

Comment thread projects/lint/src/eslint/rules/no-restricted-container-full.ts
Copilot AI review requested due to automatic review settings July 30, 2026 19:29
@coryrylan
coryrylan force-pushed the topic-tool-cleanup branch from cc511bc to 61da880 Compare July 30, 2026 19:29

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/internals/patterns/src/templates.examples.ts`:
- Around line 835-839: Set --gap: 0 on the nve-page containing the toolbar and
nve-monaco-input in this editor example, matching the FileBrowser configuration,
so the unnamed-slot siblings remain contiguous after removing the main wrapper.
🪄 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: f001af3c-a29e-4481-93b7-3246176da6dd

📥 Commits

Reviewing files that changed from the base of the PR and between cc511bc and 61da880.

📒 Files selected for processing (22)
  • projects/core/eslint.config.js
  • projects/core/src/alert/alert.examples.ts
  • projects/core/src/card/card.examples.ts
  • projects/core/src/grid/grid.examples.ts
  • projects/core/src/grid/grid.test.visual.ts
  • projects/core/src/page/page.css
  • projects/core/src/page/page.ts
  • projects/core/src/toggletip/toggletip.examples.ts
  • projects/core/src/toolbar/toolbar.examples.ts
  • projects/internals/patterns/src/browse.examples.ts
  • projects/internals/patterns/src/editor.examples.ts
  • projects/internals/patterns/src/templates.examples.ts
  • projects/internals/tools/src/skills/migration.md
  • projects/lint/README.md
  • projects/lint/src/eslint/configs/html.ts
  • projects/lint/src/eslint/rules/no-nested-container-types.test.ts
  • projects/lint/src/eslint/rules/no-nested-container-types.ts
  • projects/lint/src/eslint/rules/no-restricted-container-full.test.ts
  • projects/lint/src/eslint/rules/no-restricted-container-full.ts
  • projects/site/src/docs/elements/data-grid/container.md
  • projects/site/src/docs/lint/index.md
  • projects/styles/src/layout.css

Comment thread projects/internals/patterns/src/templates.examples.ts
Copilot AI review requested due to automatic review settings July 30, 2026 19:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions github-actions Bot added scope(ci) scope(cli) dependencies Pull requests that update a dependency file labels Jul 30, 2026
@coryrylan
coryrylan force-pushed the topic-tool-cleanup branch from 1e6bb5e to c873349 Compare July 30, 2026 19:53
Copilot AI review requested due to automatic review settings July 30, 2026 19:53

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coryrylan
coryrylan force-pushed the topic-tool-cleanup branch from c873349 to e75395f Compare July 30, 2026 20:58
Copilot AI review requested due to automatic review settings July 30, 2026 20:58

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Signed-off-by: Cory Rylan <crylan@nvidia.com>
- Introduced a new ESLint rule to restrict the use of container="full" to direct children of nve-page elements.
- Updated relevant documentation and examples to reflect this change.
- Adjusted existing components and tests to comply with the new linting rule.

Signed-off-by: Cory Rylan <crylan@nvidia.com>
- set the width to 100% for elements with the row layout and space-between alignment.
- This enhancement improves agent assumptions and layout consistency across components.

Signed-off-by: Cory Rylan <crylan@nvidia.com>
Signed-off-by: Cory Rylan <crylan@nvidia.com>
@coryrylan
coryrylan force-pushed the topic-tool-cleanup branch from e75395f to b6a2577 Compare July 30, 2026 21:09
Copilot AI review requested due to automatic review settings July 30, 2026 21:09

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coryrylan
coryrylan merged commit 5ecbd96 into main Jul 30, 2026
15 checks passed
@coryrylan
coryrylan deleted the topic-tool-cleanup branch July 30, 2026 21:31
@coryrylan

Copy link
Copy Markdown
Collaborator Author

🎉 This issue has been resolved in version 2.1.2 🎉

Changelog

@coryrylan

Copy link
Copy Markdown
Collaborator Author

🎉 This issue has been resolved in version 2.3.0 🎉

Changelog

@coryrylan

Copy link
Copy Markdown
Collaborator Author

🎉 This issue has been resolved in version 2.1.9 🎉

Changelog

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants