Merged
Conversation
ref #1338 Established the primitive composition layer, added primitive stories, and grouped Storybook navigation to prioritize primitives before higher-level abstractions.
Contributor
WalkthroughThis PR introduces a primitives layer: new primitives Box, Container, Grid, Inline, Stack, and Text (components, prop types, and Storybook stories), plus a shared 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/shade/src/components/primitives/container.tsx (1)
1-3: Consider placing React import first.Minor style note: Conventionally, React imports are placed at the top of the import block before other imports.
Suggested reorder
-import {cn} from '@/lib/utils'; -import {PADDING_X_CLASSES, SpaceStep} from './types'; import React from 'react'; + +import {cn} from '@/lib/utils'; +import {PADDING_X_CLASSES, SpaceStep} from './types';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/shade/src/components/primitives/container.tsx` around lines 1 - 3, Move the React import to be the first import in the file: place "import React from 'react';" above the other imports (currently "import {cn} from '@/lib/utils';" and "import {PADDING_X_CLASSES, SpaceStep} from './types';") so the import block follows the conventional ordering; update the top of apps/shade/src/components/primitives/container.tsx to begin with the React import to satisfy the style convention and linters.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/shade/src/components/primitives/inline.stories.tsx`:
- Around line 20-46: Add a new Story export named "Aligned" that demonstrates
the Inline component's alignment/justification props (e.g., align: 'center' or
'start' and justify: 'space-between' or 'center') by passing those keys in args
alongside gap and sample children; also add/update the storybook default
export's docs.description.component to include a one-sentence overview of what
Inline does and when to use it so the stories meet the docs guideline. Reference
the existing Story objects Default and Wrapped and add export const Aligned:
Story = { args: { gap: 'sm', align: '<value>', justify: '<value>', children: (
... ) } } and ensure the children show visible spacing and alignment so the prop
behavior is clear.
---
Nitpick comments:
In `@apps/shade/src/components/primitives/container.tsx`:
- Around line 1-3: Move the React import to be the first import in the file:
place "import React from 'react';" above the other imports (currently "import
{cn} from '@/lib/utils';" and "import {PADDING_X_CLASSES, SpaceStep} from
'./types';") so the import block follows the conventional ordering; update the
top of apps/shade/src/components/primitives/container.tsx to begin with the
React import to satisfy the style convention and linters.
🪄 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: CHILL
Plan: Pro
Run ID: 471fbb46-8c80-49b0-a0fb-7b8387d97f20
📒 Files selected for processing (18)
apps/shade/.storybook/preview.tsxapps/shade/src/components/primitives/box.stories.tsxapps/shade/src/components/primitives/box.tsxapps/shade/src/components/primitives/container.stories.tsxapps/shade/src/components/primitives/container.tsxapps/shade/src/components/primitives/grid.stories.tsxapps/shade/src/components/primitives/grid.tsxapps/shade/src/components/primitives/index.tsapps/shade/src/components/primitives/inline.stories.tsxapps/shade/src/components/primitives/inline.tsxapps/shade/src/components/primitives/stack.stories.tsxapps/shade/src/components/primitives/stack.tsxapps/shade/src/components/primitives/text.stories.tsxapps/shade/src/components/primitives/text.tsxapps/shade/src/components/primitives/types.tsapps/shade/src/docs/architecture.mdxapps/shade/src/docs/contributing.mdxapps/shade/src/primitives.ts
ref #1338 Kept primitive exports compatible while migrating legacy layout internals to primitive composition and documenting the deprecation path for new work.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/shade/src/components/layout/view-header.tsx (2)
31-38: Minor:gap-5in className may conflict with Inline defaults.The
Inlinecomponent defaults togap='md'(which maps togap-3), butgap-5is hardcoded in theclassName. Since no explicitgapprop is passed, both classes will be applied, withgap-5(later in the class string) likely winning due to CSS specificity.Consider either:
- Removing
gap-5from className and passinggap='xl'(maps togap-6) as the closest equivalent, or- Keep
gap-5in className if the exact value is intentional (non-standard spacing).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/shade/src/components/layout/view-header.tsx` around lines 31 - 38, The Inline usage in view-header.tsx hardcodes gap-5 inside the className which conflicts with Inline's default gap prop (gap='md' => gap-3); fix by removing gap-5 from the className and passing an explicit gap prop on the Inline component (e.g., gap='xl' or the desired token) so spacing is controlled via the Inline API, or if the non-standard spacing is intentional, keep gap-5 but document that choice and ensure no gap prop is passed to avoid conflicting classes; update the Inline JSX (component name Inline and its className/gap usage) accordingly.
11-17: Remove redundant classes fromViewHeaderActions.The
className='flex items-center gap-2'duplicates what theInlinecomponent already provides:
Inlineappliesflex flex-rowby default.align='center'appliesitems-center.gap='sm'appliesgap-2.This appears to be leftover from the previous implementation.
♻️ Proposed fix
const ViewHeaderActions:React.FC<ViewHeaderActionsProps> = ({children}) => { return ( - <Inline align='center' className='flex items-center gap-2' gap='sm'> + <Inline align='center' gap='sm'> {children} </Inline> ); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/shade/src/components/layout/view-header.tsx` around lines 11 - 17, The ViewHeaderActions component is passing redundant classes to Inline; remove the hardcoded className='flex items-center gap-2' from the JSX so Inline's own align='center' and gap='sm' provide those styles; if you expect consumers to supply extra classes, instead accept and merge a className prop into Inline (e.g., combine props.className with Inline's classes) but do not keep the duplicate fixed classes—update the JSX in ViewHeaderActions to rely on Inline's align and gap props or to merge externally provided classes.apps/shade/src/components/primitives/container.tsx (1)
37-40: Classes work via Tailwind v4 CSS variables but naming is inconsistent.The custom
max-w-pageandmax-w-pageminsidebarclasses are functional through Tailwind v4's automatic CSS variable support (see--container-page: 148remand--container-pageminsidebar: 116remintailwind.theme.css). However, the naming is inconsistent: CSS variables use thecontainer-*prefix while the classes usemax-w-*. For clarity and maintainability, either rename the CSS variables to match the class pattern or explicitly configure amaxWidthextension in Tailwind config that maps to these variables.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/shade/src/components/primitives/container.tsx` around lines 37 - 40, The custom class names in the container mapping (prose: 'max-w-prose', page: 'max-w-page', 'page-with-sidebar': 'max-w-pageminsidebar') are inconsistent with the underlying CSS variables which use the container-* prefix (e.g. --container-page, --container-pageminsidebar); fix by either renaming the CSS variables to match the class pattern (e.g. --max-w-page, --max-w-pageminsidebar) or explicitly add a maxWidth extension in Tailwind config that maps 'page' and 'page-with-sidebar' to the correct CSS variables (referencing the variables like var(--container-page) / var(--container-pageminsidebar)) so the class names max-w-page and max-w-pageminsidebar resolve unambiguously.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/shade/src/components/primitives/box.tsx`:
- Around line 23-24: The file currently exports the React.forwardRef component
named Box (function Box and BoxProps) but is saved as box.tsx; rename the file
to Box.tsx to match the PascalCase component identifier and update all imports
that reference "box" to "Box" so module resolution and imports remain correct;
if this file is intentionally a ShadCN-generated kebab-case component, leave
as-is, otherwise ensure the forwardRef declaration (Box) and any exports/imports
use the new PascalCase filename consistently.
- Line 2: Replace the relative import in box.tsx with the project alias: change
the import of PADDING_CLASSES, PADDING_X_CLASSES, PADDING_Y_CLASSES, SpaceStep
from './types' to the @ alias pointing at the same module (e.g. import
{PADDING_CLASSES, PADDING_X_CLASSES, PADDING_Y_CLASSES, SpaceStep} from
'@/components/primitives/types'), so the file uses the mandated "@/..." internal
import style instead of a relative path.
---
Nitpick comments:
In `@apps/shade/src/components/layout/view-header.tsx`:
- Around line 31-38: The Inline usage in view-header.tsx hardcodes gap-5 inside
the className which conflicts with Inline's default gap prop (gap='md' =>
gap-3); fix by removing gap-5 from the className and passing an explicit gap
prop on the Inline component (e.g., gap='xl' or the desired token) so spacing is
controlled via the Inline API, or if the non-standard spacing is intentional,
keep gap-5 but document that choice and ensure no gap prop is passed to avoid
conflicting classes; update the Inline JSX (component name Inline and its
className/gap usage) accordingly.
- Around line 11-17: The ViewHeaderActions component is passing redundant
classes to Inline; remove the hardcoded className='flex items-center gap-2' from
the JSX so Inline's own align='center' and gap='sm' provide those styles; if you
expect consumers to supply extra classes, instead accept and merge a className
prop into Inline (e.g., combine props.className with Inline's classes) but do
not keep the duplicate fixed classes—update the JSX in ViewHeaderActions to rely
on Inline's align and gap props or to merge externally provided classes.
In `@apps/shade/src/components/primitives/container.tsx`:
- Around line 37-40: The custom class names in the container mapping (prose:
'max-w-prose', page: 'max-w-page', 'page-with-sidebar': 'max-w-pageminsidebar')
are inconsistent with the underlying CSS variables which use the container-*
prefix (e.g. --container-page, --container-pageminsidebar); fix by either
renaming the CSS variables to match the class pattern (e.g. --max-w-page,
--max-w-pageminsidebar) or explicitly add a maxWidth extension in Tailwind
config that maps 'page' and 'page-with-sidebar' to the correct CSS variables
(referencing the variables like var(--container-page) /
var(--container-pageminsidebar)) so the class names max-w-page and
max-w-pageminsidebar resolve unambiguously.
🪄 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: CHILL
Plan: Pro
Run ID: 080b2cb2-db78-4c2d-8aa8-96b5fe9f3639
📒 Files selected for processing (14)
apps/shade/src/components/layout/error-page.tsxapps/shade/src/components/layout/header.tsxapps/shade/src/components/layout/heading.tsxapps/shade/src/components/layout/list-header.tsxapps/shade/src/components/layout/page.tsxapps/shade/src/components/layout/view-header.tsxapps/shade/src/components/primitives/box.tsxapps/shade/src/components/primitives/container.tsxapps/shade/src/components/primitives/grid.tsxapps/shade/src/components/primitives/inline.tsxapps/shade/src/components/primitives/stack.tsxapps/shade/src/components/primitives/text.tsxapps/shade/src/docs/migration-root-imports.mdxapps/shade/src/primitives.ts
✅ Files skipped from review due to trivial changes (2)
- apps/shade/src/docs/migration-root-imports.mdx
- apps/shade/src/components/layout/error-page.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/shade/src/primitives.ts
- apps/shade/src/components/primitives/stack.tsx
- apps/shade/src/components/primitives/grid.tsx
- apps/shade/src/components/primitives/text.tsx
| @@ -0,0 +1,50 @@ | |||
| import {cn} from '@/lib/utils'; | |||
| import {PADDING_CLASSES, PADDING_X_CLASSES, PADDING_Y_CLASSES, SpaceStep} from './types'; | |||
Contributor
There was a problem hiding this comment.
Use the @ alias for internal primitive imports.
Line 2 should use the project alias instead of a relative internal import.
Suggested change
-import {PADDING_CLASSES, PADDING_X_CLASSES, PADDING_Y_CLASSES, SpaceStep} from './types';
+import {PADDING_CLASSES, PADDING_X_CLASSES, PADDING_Y_CLASSES, SpaceStep} from '@/components/primitives/types';As per coding guidelines: apps/shade/src/**/*.{ts,tsx,js}: Use the @ alias for internal imports (e.g. @/lib/utils).
📝 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
| import {PADDING_CLASSES, PADDING_X_CLASSES, PADDING_Y_CLASSES, SpaceStep} from './types'; | |
| import {PADDING_CLASSES, PADDING_X_CLASSES, PADDING_Y_CLASSES, SpaceStep} from '@/components/primitives/types'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/shade/src/components/primitives/box.tsx` at line 2, Replace the relative
import in box.tsx with the project alias: change the import of PADDING_CLASSES,
PADDING_X_CLASSES, PADDING_Y_CLASSES, SpaceStep from './types' to the @ alias
pointing at the same module (e.g. import {PADDING_CLASSES, PADDING_X_CLASSES,
PADDING_Y_CLASSES, SpaceStep} from '@/components/primitives/types'), so the file
uses the mandated "@/..." internal import style instead of a relative path.
ref DES-1338 Replaced milestone and promotion jargon with plain language, added explicit human/agent workflows, and aligned Storybook docs ordering.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (5)
apps/shade/src/docs/component-contracts.mdx (2)
79-84: Consider varying sentence structure for improved readability.Lines 80-84 begin four consecutive rules with "Do not," which can feel repetitive. While the rules are clear, varying the structure could improve flow.
✏️ Proposed variation for readability
Hard rules for agents: -- Do not add product-specific props in shared `components`. -- Do not skip required states. -- Do not remove existing public API without compatibility notes. -- If behavior is workflow-specific, move it to `patterns`. +- Never add product-specific props in shared `components`. +- Required states must not be skipped. +- Existing public API requires compatibility notes before removal. +- Workflow-specific behavior belongs in `patterns`, not base components.As per static analysis hints: "Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/shade/src/docs/component-contracts.mdx` around lines 79 - 84, The four list items starting with "Do not" are repetitive; reword them to vary sentence openings while preserving the prohibitions and meaning—update the bullet list in the component-contracts rules (the entries referencing shared `components`, required states, public API, and `patterns`) so that some items use alternate phrasing (e.g., "Avoid adding...", "Always include...", "Do not remove... (or: Preserve... with compatibility notes)", "When behavior is workflow-specific, move...") to improve flow and readability while keeping the same constraints and references to `components`, `patterns`, and public API.
52-55: Consider refining the out-of-scope description.The phrase "very large mixed-behavior surfaces" on line 55 could be more specific. Static analysis suggests this is a weak intensifier that could be replaced with concrete criteria.
✏️ Proposed refinement for clarity
Out of scope for Wave A: - workflow-level compositions (`filters`, app-specific orchestration) -- very large mixed-behavior surfaces (`sidebar`, `multi-select-combobox`, chart variants) +- complex mixed-behavior surfaces with 5+ interaction modes (`sidebar`, `multi-select-combobox`, chart variants)As per static analysis hints: "As an alternative to the over-used intensifier 'very', consider replacing this phrase."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/shade/src/docs/component-contracts.mdx` around lines 52 - 55, Replace the vague phrase "very large mixed-behavior surfaces" in the Out of scope list with a concrete, measurable description; update the bullet that currently reads "very large mixed-behavior surfaces (`sidebar`, `multi-select-combobox`, chart variants)" to specify clear criteria (for example: "components with more than N interaction types or M visual/behavioral variants — e.g., sidebars, multi-select comboboxes, complex chart variants") so reviewers can unambiguously determine scope; edit the text around that bullet in component-contracts.mdx to reflect the chosen numeric thresholds or concrete examples.apps/shade/src/docs/architecture.mdx (2)
22-26: Minor: Consider varying sentence structure.Lines 24-26 begin three consecutive rules with "If," which is slightly repetitive. This is a minor style point and doesn't impact clarity significantly.
✏️ Optional variation for flow
Fast decision rule: -- If it changes spacing/alignment/layout, use `primitives`. -- If it is a generic control, use `components`. -- If it encodes product workflow, use `patterns`. +- Spacing/alignment/layout changes → use `primitives`. +- Generic controls → use `components`. +- Product workflow encoding → use `patterns`.As per static analysis hints: "Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/shade/src/docs/architecture.mdx` around lines 22 - 26, The three consecutive bullet rules under the "Fast decision rule" heading start with the word "If", which is repetitive; revise the three lines so they vary sentence openings (for example, keep the first as "If it changes spacing/alignment/layout, use `primitives`" and change the others to "Use `components` for generic controls" and "Reserve `patterns` for product-workflow encodings" or similar), preserving the same mappings to `primitives`, `components`, and `patterns` while keeping tone and clarity.
88-92: Minor: Consider varying sentence structure.Lines 89-91 begin three consecutive guidance points with "Add to," which is slightly repetitive. The content is clear, but varying the structure could improve flow.
✏️ Optional variation for flow
When to add code to Shade: - Keep code local first, then add it to Shade when it is reused across surfaces. -- Add to `primitives` when the abstraction is structural. -- Add to `components` when the abstraction is a reusable control. -- Add to `patterns` when repeated product workflows emerge. +- Structural abstractions belong in `primitives`. +- Reusable controls belong in `components`. +- Repeated product workflows belong in `patterns`. - Do not place domain/product helpers in `utils`; route them through `app` during migration and then toward app-local/pattern ownership.As per static analysis hints: "Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/shade/src/docs/architecture.mdx` around lines 88 - 92, The three consecutive guidance bullets starting "Add to" are repetitive; rephrase them to vary sentence structure while preserving intent—e.g., use imperatives or clauses that begin differently and reference the same symbols: describe when to put abstractions into `primitives`, when controls belong in `components`, and when repeated workflows belong in `patterns` (keep the "Keep code local first..." and the "Do not place domain/product helpers in `utils`..." guidance intact), so rewrite the three lines to something like “Place structural abstractions in `primitives`,” “Use `components` for reusable UI controls,” and “Reserve `patterns` for recurring product workflows.”apps/shade/src/docs/contributing.mdx (1)
55-60: Minor: Consider varying sentence structure.Lines 55-59 begin five consecutive rules with "Put," which creates a repetitive rhythm. While the parallel structure aids scanning, varying the phrasing could improve readability.
✏️ Optional variation for flow
-- Put token names/helpers and CSS token definitions in `tokens`. -- Put layout/structure-only abstractions in `primitives`. -- Put reusable controls and visual assets in `components`. -- Put reusable feature-level compositions/rules in `patterns`. -- Put app shell/provider/context APIs in `app`. +- Token names/helpers and CSS token definitions belong in `tokens`. +- Layout/structure-only abstractions belong in `primitives`. +- Reusable controls and visual assets belong in `components`. +- Reusable feature-level compositions/rules belong in `patterns`. +- App shell/provider/context APIs belong in `app`. - Keep `utils` design-system-safe only (generic helpers/hooks + third-party namespaces). - Do not add new domain/product helpers to `utils`; place them in app-local code or transitional `app` exports.As per static analysis hints: "Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/shade/src/docs/contributing.mdx` around lines 55 - 60, The five consecutive rules starting with "Put" feel repetitive; rephrase some list items to vary sentence openings while preserving meaning and mapping to the same namespaces (`tokens`, `primitives`, `components`, `patterns`, `app`, `utils`); for example, change a couple to imperatives like "Use `primitives` for layout/structure-only abstractions" or descriptive forms like "`components` contains reusable controls and visual assets" so the wording alternates and improves flow without changing the listed destinations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/shade/src/docs/architecture.mdx`:
- Around line 22-26: The three consecutive bullet rules under the "Fast decision
rule" heading start with the word "If", which is repetitive; revise the three
lines so they vary sentence openings (for example, keep the first as "If it
changes spacing/alignment/layout, use `primitives`" and change the others to
"Use `components` for generic controls" and "Reserve `patterns` for
product-workflow encodings" or similar), preserving the same mappings to
`primitives`, `components`, and `patterns` while keeping tone and clarity.
- Around line 88-92: The three consecutive guidance bullets starting "Add to"
are repetitive; rephrase them to vary sentence structure while preserving
intent—e.g., use imperatives or clauses that begin differently and reference the
same symbols: describe when to put abstractions into `primitives`, when controls
belong in `components`, and when repeated workflows belong in `patterns` (keep
the "Keep code local first..." and the "Do not place domain/product helpers in
`utils`..." guidance intact), so rewrite the three lines to something like
“Place structural abstractions in `primitives`,” “Use `components` for reusable
UI controls,” and “Reserve `patterns` for recurring product workflows.”
In `@apps/shade/src/docs/component-contracts.mdx`:
- Around line 79-84: The four list items starting with "Do not" are repetitive;
reword them to vary sentence openings while preserving the prohibitions and
meaning—update the bullet list in the component-contracts rules (the entries
referencing shared `components`, required states, public API, and `patterns`) so
that some items use alternate phrasing (e.g., "Avoid adding...", "Always
include...", "Do not remove... (or: Preserve... with compatibility notes)",
"When behavior is workflow-specific, move...") to improve flow and readability
while keeping the same constraints and references to `components`, `patterns`,
and public API.
- Around line 52-55: Replace the vague phrase "very large mixed-behavior
surfaces" in the Out of scope list with a concrete, measurable description;
update the bullet that currently reads "very large mixed-behavior surfaces
(`sidebar`, `multi-select-combobox`, chart variants)" to specify clear criteria
(for example: "components with more than N interaction types or M
visual/behavioral variants — e.g., sidebars, multi-select comboboxes, complex
chart variants") so reviewers can unambiguously determine scope; edit the text
around that bullet in component-contracts.mdx to reflect the chosen numeric
thresholds or concrete examples.
In `@apps/shade/src/docs/contributing.mdx`:
- Around line 55-60: The five consecutive rules starting with "Put" feel
repetitive; rephrase some list items to vary sentence openings while preserving
meaning and mapping to the same namespaces (`tokens`, `primitives`,
`components`, `patterns`, `app`, `utils`); for example, change a couple to
imperatives like "Use `primitives` for layout/structure-only abstractions" or
descriptive forms like "`components` contains reusable controls and visual
assets" so the wording alternates and improves flow without changing the listed
destinations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9a01af1b-5fd6-4944-98f5-4badf79d04a2
📒 Files selected for processing (6)
apps/shade/.storybook/preview.tsxapps/shade/src/docs/architecture.mdxapps/shade/src/docs/component-contracts.mdxapps/shade/src/docs/contributing.mdxapps/shade/src/docs/migration-root-imports.mdxapps/shade/src/docs/tokens.mdx
✅ Files skipped from review due to trivial changes (3)
- apps/shade/src/docs/tokens.mdx
- apps/shade/.storybook/preview.tsx
- apps/shade/src/docs/migration-root-imports.mdx
ref DES-1338 Completed MS-3 remaining repo changes with primitive docs/examples, primitive prop-mapping coverage, and Storybook docs ordering update.
|
❌ The last analysis has failed. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/shade/test/unit/components/primitives/primitives.test.tsx (2)
11-12: Consolidate duplicate imports from the same module.SonarCloud flagged that
../../utils/test-utilsis imported twice. Merge into a single import statement.Suggested fix
-import {screen} from '../../utils/test-utils'; -import {render} from '../../utils/test-utils'; +import {render, screen} from '../../utils/test-utils';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/shade/test/unit/components/primitives/primitives.test.tsx` around lines 11 - 12, The two separate imports of screen and render from '../../utils/test-utils' should be consolidated into a single import statement; update the top of the test file so that both symbols (screen, render) are imported together from '../../utils/test-utils' (locate the duplicate import lines that reference screen and render and merge them into one import listing both identifiers).
1-1: Usenode:protocol prefix for Node.js built-in modules.Per SonarCloud, prefer the explicit
node:assert/strictimport specifier for Node.js built-in modules.Suggested fix
-import assert from 'assert/strict'; +import assert from 'node:assert/strict';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/shade/test/unit/components/primitives/primitives.test.tsx` at line 1, Update the import specifier for the Node.js built-in assert module to use the node: protocol; replace the existing import "import assert from 'assert/strict';" with the explicit "node:assert/strict" specifier so tools like SonarCloud recognize it as a core Node module, then run the unit tests to verify nothing else needs adjustment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/shade/test/unit/components/primitives/primitives.test.tsx`:
- Around line 11-12: The two separate imports of screen and render from
'../../utils/test-utils' should be consolidated into a single import statement;
update the top of the test file so that both symbols (screen, render) are
imported together from '../../utils/test-utils' (locate the duplicate import
lines that reference screen and render and merge them into one import listing
both identifiers).
- Line 1: Update the import specifier for the Node.js built-in assert module to
use the node: protocol; replace the existing import "import assert from
'assert/strict';" with the explicit "node:assert/strict" specifier so tools like
SonarCloud recognize it as a core Node module, then run the unit tests to verify
nothing else needs adjustment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0603ce61-d06f-4bf1-afae-b4e68816b808
📒 Files selected for processing (3)
apps/shade/.storybook/preview.tsxapps/shade/src/docs/primitives-guide.mdxapps/shade/test/unit/components/primitives/primitives.test.tsx
✅ Files skipped from review due to trivial changes (1)
- apps/shade/src/docs/primitives-guide.mdx
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
1 participant
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.
ref #1338