feat(design-system): full UI overhaul — type tokens, Storybook, screen-by-screen alignment#855
feat(design-system): full UI overhaul — type tokens, Storybook, screen-by-screen alignment#855
Conversation
Add font-size and letter-spacing tokens to @theme inline in globals.css: - text-label (10px): section headers, badge text, micro labels — replaces text-[10px] across 28 files - text-compact (13px): field values, rail text, tab labels — replaces text-[13px] across 7 files - text-stat (19px): stat/KPI numbers — replaces text-[19px] - text-page-title (22px) / text-page-title-md (26px): page headings - tracking-label (0.12em), tracking-stat-label (0.08em), tracking-heading (-0.02em) Update SectionHeader atom to use text-label tracking-label instead of arbitrary values. Add type-scale.stories.tsx with font sizes, letter spacing, and composed pattern docs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…kens Replace arbitrary text-[Npx] and tracking-[Nem] values with formalized tokens: - text-[10px] → text-label (52 occurrences across 28 files) - text-[13px] → text-compact (19 occurrences across 7 files) - text-[19px] → text-stat - text-[22px] → text-page-title - text-[26px] → text-page-title-md - tracking-[0.12em] → tracking-label - tracking-[0.08em] → tracking-stat-label - tracking-[-0.02em] → tracking-heading Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… indicator - Extract FieldRow from CustomerDetailRail into shared atom (src/shared/ui/atoms/FieldRow.tsx) - Add Storybook stories for FieldRow (Default, Colored Value, Financial Group, Long Values) - Add Storybook stories for SectionHeader (Default, Common Labels, Custom Class) - Tab sliding indicator now uses GPU-composited transform (translateX/scaleX) instead of left/width animation — eliminates reflow, guaranteed 60fps Closes PR #854 follow-up #4 (GPU tab animation). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ay functions - Storybook a11y: change from 'todo' to 'error' — violations now fail stories - Playwright visual regression: tests/e2e/visual-regression/design-system.spec.ts with dashboard + customer list at desktop (1280x720) and mobile (375x812) - Add test:visual script to package.json - Button story: ClickInteraction with fn() mock + userEvent.click - Badge stories: play functions verifying text label rendering - Dialog stories: open/close interaction tests with assertions - Select stories: play functions for placeholder, selected, disabled states plus SelectOption story that opens menu + selects + verifies Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Stories for: BottomTabBar, MobileHeader, NijiActiveIndicator. Mobile-viewport focused with entity color variants. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Stories for: StatusBadge, MoneyAmount, GarmentImage, ColorSwatchStrip, GarmentMiniCard, ServiceTypeBadge, FavoriteStar. Each with variants, states, and autodocs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Stories for: Card, Tabs, Sheet, Checkbox, Switch, Textarea, Label, Separator, Tooltip, DropdownMenu. Each includes variants, states, and autodocs for design system reference. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Page heading: text-2xl → text-page-title/text-page-title-md with tracking-heading - Stat numbers: text-2xl font-bold → text-stat font-semibold tracking-heading tabular-nums - Add empty state for "In Progress" section (icon + message + CTA) - Consistent with customer detail reference implementation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…agic numbers Add text-micro (0.6875rem / 11px) to type scale — timestamps, role labels, helper text, secondary metadata. 13 occurrences across 10 files. Also migrate text-[12px] → text-xs (Tailwind default) in EditNoteDialog. Remaining arbitrary sizes (4 occurrences) are intentional edge cases: text-[8px] for color swatch labels and text-[9px] for micro badges. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Login heading: text-2xl → text-page-title tracking-heading - Add text-micro to type-scale Storybook story documentation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace typography magic numbers with semantic design tokens: - Page headings: text-lg -> text-page-title md:text-page-title-md tracking-heading - Lane labels (BoardLane, CapacitySummaryBar): text-xs tracking-wider -> text-label tracking-label - Section labels (BoardSection): text-xs tracking-wider -> text-label tracking-label - Tooltip section labels (JobBoardCard, QuoteBoardCard): text-xs tracking-wider -> text-label tracking-label Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Stat card labels: text-sm → text-label tracking-stat-label uppercase
(matches CustomerQuickStats pattern)
- Priority badge on blocked jobs: inline color override → statusBadge('warning')
(uses design system recipe API instead of violating two-pool encoding rules)
- Import statusBadge from @shared/lib/design-system
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds semantic type-scale tokens and swaps many ad-hoc Tailwind text classes to token-based classes across the app; introduces a shared FieldRow atom and many Storybook stories/play tests; tightens Storybook a11y to error, updates Storybook/Vite config, adds Playwright visual-regression tests, and CI/Chromatic automation. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 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 unit tests (beta)
📝 Coding Plan
|
| <span className="text-label text-muted-foreground"> | ||
| {contact.email} | ||
| </span> |
There was a problem hiding this comment.
🟡 Customer email downsized from 12px to 10px via incorrect token mapping (text-xs → text-label)
In the customer data table, the contact email was text-xs (12px) and was changed to text-label (10px) — a 17% size reduction. The text-label token is defined in src/app/globals.css:104 as "10px — section headers, badge text, micro labels", which is semantically wrong for data content like email addresses. The correct token should be text-micro (11px, for "helper text, metadata") or remain text-xs (12px), since no custom token was created for that size. At 10px, email addresses in the table become harder to read.
| <span className="text-label text-muted-foreground"> | |
| {contact.email} | |
| </span> | |
| <span className="text-micro text-muted-foreground"> | |
| {contact.email} | |
| </span> |
Was this helpful? React with 👍 or 👎 to provide feedback.
| </span> | ||
| </div> | ||
| <p className="mt-1.5 text-[22px] font-semibold tracking-[-0.02em] text-foreground tabular-nums"> | ||
| <p className="mt-1.5 text-stat font-semibold tracking-heading text-foreground tabular-nums"> |
There was a problem hiding this comment.
🟡 CustomerListStatsBar stat numbers shrunk from 22px to 19px via wrong token choice
The stat numbers in CustomerListStatsBar were text-[22px] (22px) and were mapped to text-stat (19px) — a 14% size reduction. The PR defines --font-size-page-title: 1.375rem /* 22px */ in src/app/globals.css:108, which exactly matches the original value, but text-stat (19px) was used instead. The text-stat token was calibrated from CustomerQuickStats.tsx which used text-[19px], but this component's stat numbers were intentionally larger at 22px. This breaks the visual hierarchy of the customer list stats bar, making the KPI numbers noticeably smaller.
| <p className="mt-1.5 text-stat font-semibold tracking-heading text-foreground tabular-nums"> | |
| <p className="mt-1.5 text-page-title font-semibold tracking-heading text-foreground tabular-nums"> |
Was this helpful? React with 👍 or 👎 to provide feedback.
| <p className="text-stat font-semibold tracking-heading tabular-nums"> | ||
| {blockedJobs.length} | ||
| </p> |
There was a problem hiding this comment.
🟡 Dashboard summary card numbers shrunk from 24px to 19px via text-2xl → text-stat mapping
The four dashboard summary card numbers (Blocked, In Progress, Completed, Total Jobs) were text-2xl font-bold (24px) and were all mapped to text-stat font-semibold (19px) — a 21% size reduction plus a weight downgrade from bold to semibold. No design token was created for 24px, yet it was the most prominent stat display on the main dashboard. These are the primary KPI numbers a user sees on the landing page, and the combined reduction in size and weight significantly diminishes their visual prominence.
Was this helpful? React with 👍 or 👎 to provide feedback.
| {indicatorTransform.ready && ( | ||
| <div | ||
| className="absolute top-1 bottom-1 rounded-md bg-action/[0.08] border-[1.5px] border-action pointer-events-none" | ||
| className="absolute top-1 bottom-1 rounded-md bg-action/[0.08] border-[1.5px] border-action pointer-events-none origin-left will-change-transform" | ||
| style={{ | ||
| left: `${indicatorStyle.left}px`, | ||
| width: `${indicatorStyle.width}px`, | ||
| width: `${indicatorTransform.baseWidth}px`, | ||
| transform: `translateX(${indicatorTransform.translateX}px) scaleX(${indicatorTransform.scaleX})`, | ||
| boxShadow: '1.5px 1.5px 0 var(--entity-shadow-action)', | ||
| transition: | ||
| 'left 0.22s cubic-bezier(0.34, 1.56, 0.64, 1), width 0.22s cubic-bezier(0.34, 1.56, 0.64, 1)', | ||
| transition: 'transform 0.22s cubic-bezier(0.34, 1.56, 0.64, 1)', | ||
| }} | ||
| aria-hidden="true" | ||
| /> |
There was a problem hiding this comment.
🟡 scaleX-based tab indicator distorts border width, border-radius, and box-shadow
The sliding tab indicator in CustomerTabs.tsx was changed from animating left/width to translateX/scaleX for GPU compositing. However, scaleX transforms the entire rendered output including the border-[1.5px] (which becomes horizontally stretched — e.g. 2.25px for a tab 1.5× wider than the base tab), the rounded-md border-radius (which becomes elliptical instead of circular), and the boxShadow: '1.5px 1.5px 0 ...' offset (which scales horizontally). For tabs significantly wider or narrower than the first "Overview" tab, the indicator pill will look visually inconsistent compared to the old implementation.
Was this helpful? React with 👍 or 👎 to provide feedback.
Add use-sync-external-store and its /shim subpath to Vite's optimizeDeps.include so it gets pre-bundled with proper CJS→ESM interop. Without this, Storybook fails with: "does not provide an export named 'useSyncExternalStore'" Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/(dashboard)/customers/[id]/_components/CustomerTabs.tsx (1)
115-133:⚠️ Potential issue | 🟡 MinorDon’t mark the transform indicator ready from zero-width desktop measurements.
Line 123 can be
0when this effect runs under the mobile breakpoint because the desktop tablist is still mounted butdisplay: none, and Line 126 then computes aNaNscale. Since Lines 124-129 still storeready: true, resizing from mobile to desktop leaves the indicator invisible untilactiveTabchanges again.Suggested fix
useEffect(() => { - const rafId = requestAnimationFrame(() => { + const updateIndicator = () => { const container = desktopContainerRef.current const activeBtn = tabRefs.current.get(activeTab) const firstBtn = tabRefs.current.get(DESKTOP_PRIMARY_TABS[0]) if (container && activeBtn && firstBtn) { const cRect = container.getBoundingClientRect() const bRect = activeBtn.getBoundingClientRect() const baseWidth = firstBtn.getBoundingClientRect().width + if (baseWidth === 0 || bRect.width === 0) { + setIndicatorTransform((prev) => ({ ...prev, ready: false })) + return + } setIndicatorTransform({ translateX: bRect.left - cRect.left, scaleX: bRect.width / baseWidth, baseWidth, ready: true, }) } - }) - return () => cancelAnimationFrame(rafId) + } + const rafId = requestAnimationFrame(updateIndicator) + window.addEventListener('resize', updateIndicator) + return () => { + cancelAnimationFrame(rafId) + window.removeEventListener('resize', updateIndicator) + } }, [activeTab])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(dashboard)/customers/[id]/_components/CustomerTabs.tsx around lines 115 - 133, The effect in useEffect that measures desktop tabs (using desktopContainerRef, tabRefs, DESKTOP_PRIMARY_TABS and activeTab) can compute zero widths when the desktop tablist is display:none, producing a NaN scale and prematurely setting ready: true; modify the measurement logic in the effect to bail out if any measured width (container or firstBtn width or activeBtn width) is zero or invalid and only call setIndicatorTransform with ready: true when baseWidth and bRect.width are > 0 (otherwise set ready: false or skip updating), ensuring indicator isn't marked ready from zero-width mobile measurements.
🧹 Nitpick comments (13)
src/shared/ui/organisms/ServiceTypeBadge.stories.tsx (2)
15-16:AllServiceTypesdefinesargsthat are never used.This story is effectively static because
renderignoresargs. Consider removingargshere, or consume them inrenderso controls behave as expected.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/organisms/ServiceTypeBadge.stories.tsx` around lines 15 - 16, The AllServiceTypes story declares args: { serviceType: 'screen-print' } but its render function ignores args, making controls ineffective; update the story so render accepts and uses story args (e.g., function signature using args and pass args.serviceType into the ServiceTypeBadge/props) or simply remove the unused args declaration from AllServiceTypes; specifically locate the AllServiceTypes story and either wire its render to accept (args) and consume args.serviceType, or delete the args property to keep the story static.
19-19: Consider semantic typography tokens for story labels.To stay aligned with the PR’s tokenization direction, replace
text-xslabels with the corresponding semantic text token (for exampletext-label/text-compact, whichever matches spec).Also applies to: 27-27, 35-35
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/organisms/ServiceTypeBadge.stories.tsx` at line 19, The story labels in ServiceTypeBadge.stories.tsx use the presentational utility class "text-xs text-muted-foreground"; update each label span (the ones currently using className="text-xs text-muted-foreground" at the three occurrences) to use the project's semantic typography token (e.g., replace "text-xs" with the appropriate token such as "text-label" or "text-compact" per design spec) while keeping the "text-muted-foreground" color class intact so the labels retain muted styling.src/shared/ui/layouts/NijiActiveIndicator.stories.tsx (3)
13-32: Consider using Tailwind arbitrary values instead of inline styles.For consistency with the Tailwind-first approach used elsewhere, the inline
stylecould be replaced with a Tailwind class.🧹 Proposed fix
(Story) => ( <div - className="relative w-56 bg-sidebar rounded-md border border-sidebar-border" - style={{ height: 300 }} + className="relative w-56 h-[300px] bg-sidebar rounded-md border border-sidebar-border" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/layouts/NijiActiveIndicator.stories.tsx` around lines 13 - 32, The decorator in NijiActiveIndicator.stories.tsx uses an inline style height: 300 on the wrapper div; replace that inline style with a Tailwind arbitrary value class (e.g., h-[300px]) on the same div in the decorators arrow function to keep Tailwind-first consistency and remove the style prop; keep the existing className values and the Story component placement unchanged.
1-4: Consider removing the'use client'directive.Storybook files are bundled separately and always run in a browser context, so this directive has no functional effect. Removing it keeps the file cleaner and aligns with the guideline to add it only when required.
🧹 Proposed fix
-'use client' - import type { Meta, StoryObj } from '@storybook/nextjs-vite'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/layouts/NijiActiveIndicator.stories.tsx` around lines 1 - 4, The story file includes an unnecessary 'use client' directive at the top of NijiActiveIndicator.stories.tsx; remove the `'use client'` line so the file becomes a normal Storybook module (it already runs in the browser and doesn't need client directive), leaving the imports and the NijiActiveIndicator story definitions unchanged.
97-112: Same suggestion: prefer Tailwind arbitrary values for consistency.🧹 Proposed fix
decorators: [ (Story) => ( <div - className="relative bg-sidebar rounded-md border border-sidebar-border" - style={{ width: 72, height: 200 }} + className="relative w-[72px] h-[200px] bg-sidebar rounded-md border border-sidebar-border" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/layouts/NijiActiveIndicator.stories.tsx` around lines 97 - 112, The decorator uses an inline style for sizing the wrapper (style={{ width: 72, height: 200 }}) which breaks Tailwind consistency; replace the inline style by adding Tailwind arbitrary width/height classes to the wrapper div’s className (e.g., add w-[72px] and h-[200px]) inside the decorators entry that renders (Story) so sizing is handled by Tailwind instead of the style prop.src/shared/ui/organisms/StatusBadge.stories.tsx (2)
27-38: Consider adding individual stories for all invoice statuses.The catalog shows five invoice statuses (
draft,sent,partial,paid,void), but only two have dedicated individual stories below. Adding individual stories for the missing statuses would improve Storybook documentation completeness and allow each status to be tested/viewed in isolation.📚 Suggested additions
Add after line 58 (or in a similar location):
export const InvoiceDraft: Story = { args: { status: 'draft', variant: 'invoice' }, } export const InvoiceSent: Story = { args: { status: 'sent', variant: 'invoice' }, } export const InvoicePartial: Story = { args: { status: 'partial', variant: 'invoice' }, }Also consider removing the unused
argsfrom this catalog story:export const AllInvoiceStatuses: Story = { - args: { status: 'draft', variant: 'invoice' }, render: () => (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/organisms/StatusBadge.stories.tsx` around lines 27 - 38, The Storybook catalog story AllInvoiceStatuses currently lists all invoice statuses but individual stories for 'draft', 'sent', and 'partial' are missing; add separate stories (e.g., InvoiceDraft, InvoiceSent, InvoicePartial) that set args { status: '<status>', variant: 'invoice' } so each status can be viewed/tested in isolation, and remove the unused args from AllInvoiceStatuses (it only needs the render block) to avoid duplication; update references to StatusBadge only via these new stories.
14-25: Consider removing unusedargsfrom catalog story.The
argsobject is defined but unused sincerenderis overridden. This can be confusing for readers who might expect the args to control the rendered output.♻️ Suggested cleanup
export const AllQuoteStatuses: Story = { - args: { status: 'draft', variant: 'quote' }, render: () => ( <div className="flex flex-col gap-3 p-4">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/organisms/StatusBadge.stories.tsx` around lines 14 - 25, The AllQuoteStatuses story defines an unused args object which is misleading because the render override supplies all props; remove the args property from the AllQuoteStatuses export (or alternatively change the export to use args in the render/template) so that the story either relies on args or omits them—update the AllQuoteStatuses declaration accordingly and keep the render function that renders multiple StatusBadge instances (StatusBadge, AllQuoteStatuses, args, render).src/shared/ui/primitives/separator.stories.tsx (1)
39-45: Strengthen play assertions to validate the separator itself.Right now the play tests only validate surrounding text, so they would still pass if
<Separator />were removed. Add separator-count assertions so the interaction test covers component rendering too.Suggested diff
export const Horizontal: Story = { render: function Render() { @@ play: async ({ canvasElement }) => { const canvas = within(canvasElement) await expect(canvas.getByText('Order details')).toBeInTheDocument() await expect(canvas.getByText('Production status')).toBeInTheDocument() await expect(canvas.getByText('Shipping')).toBeInTheDocument() + await expect(canvas.getAllByRole('separator')).toHaveLength(2) }, } @@ export const Vertical: Story = { @@ play: async ({ canvasElement }) => { const canvas = within(canvasElement) await expect(canvas.getByText('Orders')).toBeInTheDocument() await expect(canvas.getByText('Customers')).toBeInTheDocument() await expect(canvas.getByText('Garments')).toBeInTheDocument() + await expect(canvas.getAllByRole('separator')).toHaveLength(3) }, }Also applies to: 62-68
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/primitives/separator.stories.tsx` around lines 39 - 45, The play tests currently only assert surrounding text and should also verify the Separator component is rendered; update the play async functions in the stories to include assertions that count Separator instances (e.g., use canvas.getAllByRole('separator') or a test-id used by <Separator />) and assert the expected length for each story (add matching separator-count assertions for the play blocks around the existing text checks at the two places corresponding to the current ranges). Ensure you reference the play async functions and the Separator component when adding these additional assertions so the interaction test fails if Separator is removed.src/shared/ui/primitives/label.stories.tsx (1)
67-76: Consider adding aplayfunction for consistency.The Default and Required stories include
playfunctions with assertions, but DisabledState does not. Adding a basic assertion (e.g., verifying the input is disabled) would improve consistency and test coverage.✨ Suggested play function
export const DisabledState: Story = { render: function Render() { return ( <div className="grid w-64 gap-2" data-disabled="true"> <Label htmlFor="locked">Locked field</Label> <Input id="locked" disabled defaultValue="Cannot edit" /> </div> ) }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement) + + await expect(canvas.getByText('Locked field')).toBeInTheDocument() + await expect(canvas.getByRole('textbox')).toBeDisabled() + }, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/primitives/label.stories.tsx` around lines 67 - 76, Add a play function to the DisabledState story to assert the input is disabled: locate the DisabledState export (its Render function that returns <Label htmlFor="locked"> and <Input id="locked" disabled ... />) and add an async play that uses the testing-library selectors (within canvasElement) to query the input by role or id and expect it to be disabled; attach the play to DisabledState so it mirrors the Default/Required stories' structure and basic assertion.src/shared/ui/organisms/GarmentImage.stories.tsx (1)
14-32: Consider using semantic design tokens for labels.The labels currently use
text-xs, which is fine, but given this PR's focus on migrating to semantic design tokens (text-label, text-micro, etc.), you might consider using one of those tokens for consistency across the codebase.♻️ Optional: Use semantic token
- <span className="text-xs text-muted-foreground">sm (40px)</span> + <span className="text-label text-muted-foreground">sm (40px)</span> </div> <div className="flex flex-col items-center gap-2"> <GarmentImage brand="Bella+Canvas" sku="3001" name="Unisex Jersey Tee" size="md" /> - <span className="text-xs text-muted-foreground">md (80px)</span> + <span className="text-label text-muted-foreground">md (80px)</span> </div> <div className="flex flex-col items-center gap-2"> <GarmentImage brand="Bella+Canvas" sku="3001" name="Unisex Jersey Tee" size="lg" /> - <span className="text-xs text-muted-foreground">lg (160px)</span> + <span className="text-label text-muted-foreground">lg (160px)</span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/organisms/GarmentImage.stories.tsx` around lines 14 - 32, The story AllSizes uses hardcoded Tailwind utility "text-xs" for the size labels; replace that with the semantic design token class (e.g., "text-label" or "text-micro" per our design system) to stay consistent—update the three <span> elements in the AllSizes story in src/shared/ui/organisms/GarmentImage.stories.tsx to use the chosen semantic class instead of "text-xs" (leave the rest of the markup and GarmentImage props unchanged).src/shared/ui/organisms/GarmentMiniCard.stories.tsx (1)
67-67: Prefer a utility class over inline width in the story container.For consistency with the design-system/Tailwind-first approach, consider replacing
style={{ width: 320 }}with a class (for examplew-80).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/organisms/GarmentMiniCard.stories.tsx` at line 67, The story container div in GarmentMiniCard.stories.tsx uses an inline style width (style={{ width: 320 }}) which should be replaced with a Tailwind utility; remove the inline style and add the corresponding utility (e.g., include "w-80") to the existing className on the div with "flex flex-col gap-4 p-4" so the container width is defined by a utility class instead of inline styles.tests/e2e/visual-regression/design-system.spec.ts (1)
19-19: ReplacewaitForLoadState('networkidle')with deterministic UI readiness markers in visual tests.
networkidleis unreliable for visual regression tests because pages with ongoing background requests (analytics, polling) may timeout or resolve inconsistently. Instead, wait for specific page elements to be visible (e.g.,await expect(page.locator('.user-list')).toBeVisible()) or usepage.goto()withwaitUntil: 'load'before taking the screenshot.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/visual-regression/design-system.spec.ts` at line 19, Replace the fragile pw.waitForLoadState('networkidle') call in the test with deterministic UI readiness checks: remove the pw.waitForLoadState('networkidle') usage and instead wait for specific visible elements using page.locator(...) combined with expect(...).toBeVisible(), or ensure navigation uses page.goto(..., waitUntil: 'load') before asserting or taking screenshots; look for the pw.waitForLoadState call and update it to use page.locator and expect or page.goto with waitUntil to guarantee a stable UI state for visual regression.stories/foundations/type-scale.stories.tsx (1)
193-194: Use semantic heading tokens in the foundation story itself.This page is the canonical token reference; replacing
text-2xl tracking-tightwith your semantic heading pattern keeps the docs fully self-consistent.♻️ Suggested patch
- <h1 className="text-2xl font-semibold tracking-tight mb-2">Font Size Tokens</h1> + <h1 className="text-page-title md:text-page-title-md font-bold tracking-heading mb-2">Font Size Tokens</h1> - <h1 className="text-2xl font-semibold tracking-tight mb-2">Letter Spacing Tokens</h1> + <h1 className="text-page-title md:text-page-title-md font-bold tracking-heading mb-2">Letter Spacing Tokens</h1> - <h1 className="text-2xl font-semibold tracking-tight mb-2">Composed Type Patterns</h1> + <h1 className="text-page-title md:text-page-title-md font-bold tracking-heading mb-2">Composed Type Patterns</h1>Also applies to: 234-235, 269-270
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stories/foundations/type-scale.stories.tsx` around lines 193 - 194, Replace the hardcoded utility classes on the canonical token headings with the project's semantic heading token: find the h1 elements that currently have className="text-2xl font-semibold tracking-tight mb-2" (and the other two occurrences referenced) and swap out the sizing/tracking utilities (text-2xl and tracking-tight) for the semantic heading token used across the foundations (e.g., your "text-heading-*" or "heading-lg" token pattern) while preserving font weight and margin classes; update all three occurrences in this file to use the semantic token pattern so the story remains self-consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/`(dashboard)/page.tsx:
- Around line 189-190: The UI currently renders raw job.dueDate which can be
inconsistent; replace the raw value with the same date formatter used elsewhere
on this page (e.g., formatDate/formatFriendlyDate) so the in-progress list shows
consistently formatted dates. Find where other dates are rendered on this page,
reuse that formatter (import it if needed), and update the JSX to use
formatDate(job.dueDate) (keeping job.jobNumber and
getCustomerName(job.customerId) unchanged).
In `@src/shared/ui/primitives/label.stories.tsx`:
- Around line 58-64: The play function's assertions use canvas.getByText with
exact strings but the Label components include child spans (e.g., asterisk), so
getByText('Customer name') will fail; inside the play async ({ canvasElement })
=> { const canvas = within(canvasElement) ... } replace the exact getByText
calls with either partial/regex matches (e.g., canvas.getByText(/Customer
name/)) or switch to accessibility queries like canvas.getByLabelText('Customer
name') for each label (same for 'Email' and 'Phone') and keep the await
expect(...).toBeInTheDocument() assertions.
In `@src/shared/ui/primitives/textarea.stories.tsx`:
- Around line 49-54: The play function currently queries the canvas for a
textarea but silently skips assertions if it's null; update the play handler
(the async play function) to explicitly fail when
canvasElement.querySelector('textarea') returns null by asserting existence or
throwing a clear error before checking disabled state, then continue to assert
await expect(textarea).toBeDisabled(); this ensures missing textarea elements
surface as test failures rather than being ignored.
---
Outside diff comments:
In `@src/app/`(dashboard)/customers/[id]/_components/CustomerTabs.tsx:
- Around line 115-133: The effect in useEffect that measures desktop tabs (using
desktopContainerRef, tabRefs, DESKTOP_PRIMARY_TABS and activeTab) can compute
zero widths when the desktop tablist is display:none, producing a NaN scale and
prematurely setting ready: true; modify the measurement logic in the effect to
bail out if any measured width (container or firstBtn width or activeBtn width)
is zero or invalid and only call setIndicatorTransform with ready: true when
baseWidth and bRect.width are > 0 (otherwise set ready: false or skip updating),
ensuring indicator isn't marked ready from zero-width mobile measurements.
---
Nitpick comments:
In `@src/shared/ui/layouts/NijiActiveIndicator.stories.tsx`:
- Around line 13-32: The decorator in NijiActiveIndicator.stories.tsx uses an
inline style height: 300 on the wrapper div; replace that inline style with a
Tailwind arbitrary value class (e.g., h-[300px]) on the same div in the
decorators arrow function to keep Tailwind-first consistency and remove the
style prop; keep the existing className values and the Story component placement
unchanged.
- Around line 1-4: The story file includes an unnecessary 'use client' directive
at the top of NijiActiveIndicator.stories.tsx; remove the `'use client'` line so
the file becomes a normal Storybook module (it already runs in the browser and
doesn't need client directive), leaving the imports and the NijiActiveIndicator
story definitions unchanged.
- Around line 97-112: The decorator uses an inline style for sizing the wrapper
(style={{ width: 72, height: 200 }}) which breaks Tailwind consistency; replace
the inline style by adding Tailwind arbitrary width/height classes to the
wrapper div’s className (e.g., add w-[72px] and h-[200px]) inside the decorators
entry that renders (Story) so sizing is handled by Tailwind instead of the style
prop.
In `@src/shared/ui/organisms/GarmentImage.stories.tsx`:
- Around line 14-32: The story AllSizes uses hardcoded Tailwind utility
"text-xs" for the size labels; replace that with the semantic design token class
(e.g., "text-label" or "text-micro" per our design system) to stay
consistent—update the three <span> elements in the AllSizes story in
src/shared/ui/organisms/GarmentImage.stories.tsx to use the chosen semantic
class instead of "text-xs" (leave the rest of the markup and GarmentImage props
unchanged).
In `@src/shared/ui/organisms/GarmentMiniCard.stories.tsx`:
- Line 67: The story container div in GarmentMiniCard.stories.tsx uses an inline
style width (style={{ width: 320 }}) which should be replaced with a Tailwind
utility; remove the inline style and add the corresponding utility (e.g.,
include "w-80") to the existing className on the div with "flex flex-col gap-4
p-4" so the container width is defined by a utility class instead of inline
styles.
In `@src/shared/ui/organisms/ServiceTypeBadge.stories.tsx`:
- Around line 15-16: The AllServiceTypes story declares args: { serviceType:
'screen-print' } but its render function ignores args, making controls
ineffective; update the story so render accepts and uses story args (e.g.,
function signature using args and pass args.serviceType into the
ServiceTypeBadge/props) or simply remove the unused args declaration from
AllServiceTypes; specifically locate the AllServiceTypes story and either wire
its render to accept (args) and consume args.serviceType, or delete the args
property to keep the story static.
- Line 19: The story labels in ServiceTypeBadge.stories.tsx use the
presentational utility class "text-xs text-muted-foreground"; update each label
span (the ones currently using className="text-xs text-muted-foreground" at the
three occurrences) to use the project's semantic typography token (e.g., replace
"text-xs" with the appropriate token such as "text-label" or "text-compact" per
design spec) while keeping the "text-muted-foreground" color class intact so the
labels retain muted styling.
In `@src/shared/ui/organisms/StatusBadge.stories.tsx`:
- Around line 27-38: The Storybook catalog story AllInvoiceStatuses currently
lists all invoice statuses but individual stories for 'draft', 'sent', and
'partial' are missing; add separate stories (e.g., InvoiceDraft, InvoiceSent,
InvoicePartial) that set args { status: '<status>', variant: 'invoice' } so each
status can be viewed/tested in isolation, and remove the unused args from
AllInvoiceStatuses (it only needs the render block) to avoid duplication; update
references to StatusBadge only via these new stories.
- Around line 14-25: The AllQuoteStatuses story defines an unused args object
which is misleading because the render override supplies all props; remove the
args property from the AllQuoteStatuses export (or alternatively change the
export to use args in the render/template) so that the story either relies on
args or omits them—update the AllQuoteStatuses declaration accordingly and keep
the render function that renders multiple StatusBadge instances (StatusBadge,
AllQuoteStatuses, args, render).
In `@src/shared/ui/primitives/label.stories.tsx`:
- Around line 67-76: Add a play function to the DisabledState story to assert
the input is disabled: locate the DisabledState export (its Render function that
returns <Label htmlFor="locked"> and <Input id="locked" disabled ... />) and add
an async play that uses the testing-library selectors (within canvasElement) to
query the input by role or id and expect it to be disabled; attach the play to
DisabledState so it mirrors the Default/Required stories' structure and basic
assertion.
In `@src/shared/ui/primitives/separator.stories.tsx`:
- Around line 39-45: The play tests currently only assert surrounding text and
should also verify the Separator component is rendered; update the play async
functions in the stories to include assertions that count Separator instances
(e.g., use canvas.getAllByRole('separator') or a test-id used by <Separator />)
and assert the expected length for each story (add matching separator-count
assertions for the play blocks around the existing text checks at the two places
corresponding to the current ranges). Ensure you reference the play async
functions and the Separator component when adding these additional assertions so
the interaction test fails if Separator is removed.
In `@stories/foundations/type-scale.stories.tsx`:
- Around line 193-194: Replace the hardcoded utility classes on the canonical
token headings with the project's semantic heading token: find the h1 elements
that currently have className="text-2xl font-semibold tracking-tight mb-2" (and
the other two occurrences referenced) and swap out the sizing/tracking utilities
(text-2xl and tracking-tight) for the semantic heading token used across the
foundations (e.g., your "text-heading-*" or "heading-lg" token pattern) while
preserving font weight and margin classes; update all three occurrences in this
file to use the semantic token pattern so the story remains self-consistent.
In `@tests/e2e/visual-regression/design-system.spec.ts`:
- Line 19: Replace the fragile pw.waitForLoadState('networkidle') call in the
test with deterministic UI readiness checks: remove the
pw.waitForLoadState('networkidle') usage and instead wait for specific visible
elements using page.locator(...) combined with expect(...).toBeVisible(), or
ensure navigation uses page.goto(..., waitUntil: 'load') before asserting or
taking screenshots; look for the pw.waitForLoadState call and update it to use
page.locator and expect or page.goto with waitUntil to guarantee a stable UI
state for visual regression.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4afedf3f-b2b7-4a81-8215-24b2eaf7d600
📒 Files selected for processing (85)
.storybook/preview.tspackage.jsonsrc/app/(auth)/login/page.tsxsrc/app/(dashboard)/customers/[id]/_components/ActivityTimeline.tsxsrc/app/(dashboard)/customers/[id]/_components/ContactHierarchy.tsxsrc/app/(dashboard)/customers/[id]/_components/CustomerDetailHeader.tsxsrc/app/(dashboard)/customers/[id]/_components/CustomerDetailRail.tsxsrc/app/(dashboard)/customers/[id]/_components/CustomerDetailsPanel.tsxsrc/app/(dashboard)/customers/[id]/_components/CustomerOverviewTab.tsxsrc/app/(dashboard)/customers/[id]/_components/CustomerTabs.tsxsrc/app/(dashboard)/customers/_components/CustomerListStatsBar.tsxsrc/app/(dashboard)/customers/_components/CustomersDataTable.tsxsrc/app/(dashboard)/customers/_components/SmartViewTabs.tsxsrc/app/(dashboard)/customers/page.tsxsrc/app/(dashboard)/garments/favorites/GarmentFavoritesClient.tsxsrc/app/(dashboard)/garments/favorites/_components/StyleDetailModal.tsxsrc/app/(dashboard)/jobs/_components/BoardFilterBar.tsxsrc/app/(dashboard)/jobs/_components/BoardLane.tsxsrc/app/(dashboard)/jobs/_components/BoardSection.tsxsrc/app/(dashboard)/jobs/_components/CapacitySummaryBar.tsxsrc/app/(dashboard)/jobs/_components/JobBoardCard.tsxsrc/app/(dashboard)/jobs/_components/JobsList.tsxsrc/app/(dashboard)/jobs/_components/QuoteBoardCard.tsxsrc/app/(dashboard)/jobs/board/_components/ProductionBoard.tsxsrc/app/(dashboard)/page.tsxsrc/app/(dashboard)/quotes/[id]/edit/page.tsxsrc/app/(dashboard)/quotes/[id]/page.tsxsrc/app/(dashboard)/quotes/_components/ArtworkPreview.tsxsrc/app/(dashboard)/quotes/_components/PricingSummary.tsxsrc/app/(dashboard)/quotes/_components/QuoteDetailView.tsxsrc/app/(dashboard)/quotes/_components/QuoteForm.tsxsrc/app/(dashboard)/quotes/_components/QuotesDataTable.tsxsrc/app/(dashboard)/quotes/_components/SheetCalculationPanel.tsxsrc/app/(dashboard)/quotes/error.tsxsrc/app/(dashboard)/quotes/new/page.tsxsrc/app/(dashboard)/settings/pricing/_components/GarmentTypePricingEditor.tsxsrc/app/(dashboard)/settings/pricing/_components/LocationUpchargeEditor.tsxsrc/app/(dashboard)/settings/pricing/_components/MatrixPreviewSelector.tsxsrc/app/(dashboard)/settings/pricing/_components/MobileToolsSheet.tsxsrc/app/(dashboard)/settings/pricing/_components/QuantityTierEditor.tsxsrc/app/(dashboard)/settings/pricing/_components/SetupFeeEditor.tsxsrc/app/(dashboard)/settings/pricing/page.tsxsrc/app/(dashboard)/settings/pricing/screen-print/[id]/editor.tsxsrc/app/globals.csssrc/features/artwork/components/ArtworkLibraryClient.tsxsrc/features/customers/components/ActivityEntry.tsxsrc/features/customers/components/CustomerQuickStats.tsxsrc/features/customers/components/EditNoteDialog.tsxsrc/features/customers/components/QuickNoteRail.tsxsrc/features/pricing/components/PricingTemplateCard.tsxsrc/features/quotes/components/ArtworkGallery.tsxsrc/features/quotes/components/InheritanceDetail.tsxsrc/features/quotes/components/NotesPanel.tsxsrc/shared/ui/atoms/FieldRow.stories.tsxsrc/shared/ui/atoms/FieldRow.tsxsrc/shared/ui/atoms/SectionHeader.stories.tsxsrc/shared/ui/atoms/SectionHeader.tsxsrc/shared/ui/layouts/BottomTabBar.stories.tsxsrc/shared/ui/layouts/MobileHeader.stories.tsxsrc/shared/ui/layouts/NijiActiveIndicator.stories.tsxsrc/shared/ui/layouts/bottom-tab-bar.tsxsrc/shared/ui/organisms/ColorSwatchStrip.stories.tsxsrc/shared/ui/organisms/ColorSwatchStrip.tsxsrc/shared/ui/organisms/FavoriteStar.stories.tsxsrc/shared/ui/organisms/GarmentImage.stories.tsxsrc/shared/ui/organisms/GarmentMiniCard.stories.tsxsrc/shared/ui/organisms/MoneyAmount.stories.tsxsrc/shared/ui/organisms/ServiceTypeBadge.stories.tsxsrc/shared/ui/organisms/StatusBadge.stories.tsxsrc/shared/ui/primitives/badge.stories.tsxsrc/shared/ui/primitives/button.stories.tsxsrc/shared/ui/primitives/card.stories.tsxsrc/shared/ui/primitives/checkbox.stories.tsxsrc/shared/ui/primitives/dialog.stories.tsxsrc/shared/ui/primitives/dropdown-menu.stories.tsxsrc/shared/ui/primitives/label.stories.tsxsrc/shared/ui/primitives/select.stories.tsxsrc/shared/ui/primitives/separator.stories.tsxsrc/shared/ui/primitives/sheet.stories.tsxsrc/shared/ui/primitives/switch.stories.tsxsrc/shared/ui/primitives/tabs.stories.tsxsrc/shared/ui/primitives/textarea.stories.tsxsrc/shared/ui/primitives/tooltip.stories.tsxstories/foundations/type-scale.stories.tsxtests/e2e/visual-regression/design-system.spec.ts
| {job.jobNumber} · {getCustomerName(job.customerId)} · Due{' '} | ||
| {job.dueDate} |
There was a problem hiding this comment.
Format due dates before rendering in the in-progress list.
This currently renders raw job.dueDate, which can surface inconsistent/UTC-formatted strings. Format it the same way as other date UI in this page.
💡 Suggested patch
- {job.jobNumber} · {getCustomerName(job.customerId)} · Due{' '}
- {job.dueDate}
+ {job.jobNumber} · {getCustomerName(job.customerId)} · Due{' '}
+ {new Date(job.dueDate).toLocaleDateString()}📝 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.
| {job.jobNumber} · {getCustomerName(job.customerId)} · Due{' '} | |
| {job.dueDate} | |
| {job.jobNumber} · {getCustomerName(job.customerId)} · Due{' '} | |
| {new Date(job.dueDate).toLocaleDateString()} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`(dashboard)/page.tsx around lines 189 - 190, The UI currently
renders raw job.dueDate which can be inconsistent; replace the raw value with
the same date formatter used elsewhere on this page (e.g.,
formatDate/formatFriendlyDate) so the in-progress list shows consistently
formatted dates. Find where other dates are rendered on this page, reuse that
formatter (import it if needed), and update the JSX to use
formatDate(job.dueDate) (keeping job.jobNumber and
getCustomerName(job.customerId) unchanged).
| play: async ({ canvasElement }) => { | ||
| const canvas = within(canvasElement) | ||
|
|
||
| await expect(canvas.getByText('Customer name')).toBeInTheDocument() | ||
| await expect(canvas.getByText('Email')).toBeInTheDocument() | ||
| await expect(canvas.getByText('Phone')).toBeInTheDocument() | ||
| }, |
There was a problem hiding this comment.
getByText assertions may fail due to child element text content.
The labels contain child <span> elements with asterisks, so the full textContent is "Customer name *", not "Customer name". Since getByText uses exact matching by default, these assertions will likely fail.
Consider using regex for partial matching or getByLabelText for a more robust approach:
🔧 Proposed fix using regex matching
play: async ({ canvasElement }) => {
const canvas = within(canvasElement)
- await expect(canvas.getByText('Customer name')).toBeInTheDocument()
- await expect(canvas.getByText('Email')).toBeInTheDocument()
+ await expect(canvas.getByText(/Customer name/)).toBeInTheDocument()
+ await expect(canvas.getByText(/Email/)).toBeInTheDocument()
await expect(canvas.getByText('Phone')).toBeInTheDocument()
},📝 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.
| play: async ({ canvasElement }) => { | |
| const canvas = within(canvasElement) | |
| await expect(canvas.getByText('Customer name')).toBeInTheDocument() | |
| await expect(canvas.getByText('Email')).toBeInTheDocument() | |
| await expect(canvas.getByText('Phone')).toBeInTheDocument() | |
| }, | |
| play: async ({ canvasElement }) => { | |
| const canvas = within(canvasElement) | |
| await expect(canvas.getByText(/Customer name/)).toBeInTheDocument() | |
| await expect(canvas.getByText(/Email/)).toBeInTheDocument() | |
| await expect(canvas.getByText('Phone')).toBeInTheDocument() | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/ui/primitives/label.stories.tsx` around lines 58 - 64, The play
function's assertions use canvas.getByText with exact strings but the Label
components include child spans (e.g., asterisk), so getByText('Customer name')
will fail; inside the play async ({ canvasElement }) => { const canvas =
within(canvasElement) ... } replace the exact getByText calls with either
partial/regex matches (e.g., canvas.getByText(/Customer name/)) or switch to
accessibility queries like canvas.getByLabelText('Customer name') for each label
(same for 'Email' and 'Phone') and keep the await
expect(...).toBeInTheDocument() assertions.
| play: async ({ canvasElement }) => { | ||
| const textarea = canvasElement.querySelector('textarea') | ||
| if (textarea) { | ||
| await expect(textarea).toBeDisabled() | ||
| } | ||
| }, |
There was a problem hiding this comment.
Silent null check may mask test failures.
If canvasElement.querySelector('textarea') returns null, the test passes silently without asserting anything. This could hide rendering issues.
🛡️ Proposed fix to fail explicitly when textarea is missing
play: async ({ canvasElement }) => {
const textarea = canvasElement.querySelector('textarea')
- if (textarea) {
- await expect(textarea).toBeDisabled()
- }
+ await expect(textarea).not.toBeNull()
+ await expect(textarea).toBeDisabled()
},📝 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.
| play: async ({ canvasElement }) => { | |
| const textarea = canvasElement.querySelector('textarea') | |
| if (textarea) { | |
| await expect(textarea).toBeDisabled() | |
| } | |
| }, | |
| play: async ({ canvasElement }) => { | |
| const textarea = canvasElement.querySelector('textarea') | |
| await expect(textarea).not.toBeNull() | |
| await expect(textarea).toBeDisabled() | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/ui/primitives/textarea.stories.tsx` around lines 49 - 54, The play
function currently queries the canvas for a textarea but silently skips
assertions if it's null; update the play handler (the async play function) to
explicitly fail when canvasElement.querySelector('textarea') returns null by
asserting existence or throwing a clear error before checking disabled state,
then continue to assert await expect(textarea).toBeDisabled(); this ensures
missing textarea elements surface as test failures rather than being ignored.
| setIndicatorTransform({ | ||
| translateX: bRect.left - cRect.left, | ||
| scaleX: bRect.width / baseWidth, | ||
| baseWidth, | ||
| ready: true, | ||
| }) |
There was a problem hiding this comment.
🟡 Tab indicator sets ready=true with NaN scaleX on mobile due to division by zero
On mobile viewports, the desktop tab container has hidden md:block (display: none), so getBoundingClientRect().width returns 0 for all child elements. At CustomerTabs.tsx:126, scaleX: bRect.width / baseWidth computes 0 / 0 = NaN, yet ready: true is set. The old code guarded with indicatorStyle.width > 0 which prevented rendering when dimensions were zero. The new indicatorTransform.ready boolean provides no such value validation. If the user resizes their viewport from mobile to desktop width, the indicator div is visible (because ready is true) but has width: 0px and an invalid scaleX(NaN) transform, so it appears broken/missing until activeTab changes and triggers re-computation.
| setIndicatorTransform({ | |
| translateX: bRect.left - cRect.left, | |
| scaleX: bRect.width / baseWidth, | |
| baseWidth, | |
| ready: true, | |
| }) | |
| setIndicatorTransform({ | |
| translateX: bRect.left - cRect.left, | |
| scaleX: baseWidth > 0 ? bRect.width / baseWidth : 1, | |
| baseWidth, | |
| ready: baseWidth > 0, | |
| }) |
Was this helpful? React with 👍 or 👎 to provide feedback.
Configure 5 Mokumo-specific viewports in preview.ts: - Mobile Small (375×812), Mobile Large (428×926) - Tablet (768×1024) — matches md: breakpoint - Desktop (1280×720), Desktop Wide (1536×864) Viewport addon is built into storybook core — only parameters config needed, no addon registration required. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- chromatic.config.json with TurboSnap (onlyChanged) and globals.css as external dependency for change detection - CI job runs on PRs and main pushes, after check job passes - exitZeroOnChanges: true (visual diffs don't block merge — review in Chromatic dashboard) - exitOnceUploaded: true (don't wait for full processing in CI) - npm run chromatic script for local testing Requires CHROMATIC_PROJECT_TOKEN secret in GitHub repo settings. Replace projectId in chromatic.config.json with your Chromatic project ID. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Project ID: 69bc6ebd92497c2210ae60d6 First build: 46 components, 193 stories, 193 snapshots captured. CHROMATIC_PROJECT_TOKEN set as GitHub Actions secret. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
package.json (1)
18-18:STORYBOOK_TEST=1environment variable is not consumed.Based on
vitest.config.ts(lines 1-20) and.storybook/vitest.setup.ts(lines 1-7), neither file readsprocess.env.STORYBOOK_TEST. This makes the environment variable injection inert — it has no effect on test behavior or configuration.Either remove the unused variable or wire it into the Vitest config if it's intended for conditional behavior.
🔧 Option: Remove unused variable
- "test:storybook": "STORYBOOK_TEST=1 vitest run --project=storybook", + "test:storybook": "vitest run --project=storybook",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 18, The package.json script "test:storybook" sets STORYBOOK_TEST=1 but that env var isn't consumed; either remove it from the script or wire it into the Vitest setup by reading process.env.STORYBOOK_TEST in vitest.config.ts (and/or .storybook/vitest.setup.ts) and using it to toggle the intended behavior (e.g., enable the storybook project, conditional setupFiles, or different test options). Update the "test:storybook" script or update vitest.config.ts to check process.env.STORYBOOK_TEST and apply the conditional configuration using the existing project names or setup hooks referenced in vitest.config.ts and .storybook/vitest.setup.ts..github/workflows/ci.yml (2)
112-112:onlyChangedis duplicated in both the workflow andchromatic.config.json.The
chromatic.config.jsonfile already specifies"onlyChanged": trueat line 4. Specifying it again here is redundant. Consider consolidating all Chromatic configuration in one place (the config file) to avoid drift and confusion if someone updates only one location.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml at line 112, Remove the redundant onlyChanged: true entry from the Chromatic step in the workflow and rely on the existing chromatic.config.json (which already has "onlyChanged": true) to avoid configuration drift; locate the Chromatic invocation in .github/workflows/ci.yml (the Chromatic workflow step) and delete the onlyChanged key there or change the step to reference the config file explicitly so all Chromatic options live in chromatic.config.json.
109-109: Consider pinning the Chromatic action to a major version for consistency, though@latestis officially supported.While
chromaui/action@latestis officially recommended by Chromatic and will receive updates automatically, pinning to a major version (e.g.,chromaui/action@v15) provides explicit control over when updates are applied. If pinning, usev15as the current major version rather than older versions.🔧 Optional: Pin to major version
- name: Run Chromatic - uses: chromaui/action@latest + uses: chromaui/action@v15 with:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml at line 109, The workflow currently uses the floating reference "chromaui/action@latest"; change it to a pinned major version to avoid unintended updates — replace "chromaui/action@latest" with a major-pinned tag like "chromaui/action@v15" in the CI workflow so updates are applied intentionally while keeping the action stable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Around line 92-95: The Storybook dependency versions currently set to ^10.3.1
are invalid on npm; update the version specifiers for the Storybook packages
(e.g., "@storybook/addon-a11y", "@storybook/addon-docs",
"@storybook/addon-vitest", "@storybook/nextjs-vite" and the other Storybook
entries referenced later) to a valid published release such as ^10.2.19 or to a
published pre-release tag (for example the "next" 10.3.0-beta.3) so all
Storybook package entries use the same valid version specifier.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Line 112: Remove the redundant onlyChanged: true entry from the Chromatic step
in the workflow and rely on the existing chromatic.config.json (which already
has "onlyChanged": true) to avoid configuration drift; locate the Chromatic
invocation in .github/workflows/ci.yml (the Chromatic workflow step) and delete
the onlyChanged key there or change the step to reference the config file
explicitly so all Chromatic options live in chromatic.config.json.
- Line 109: The workflow currently uses the floating reference
"chromaui/action@latest"; change it to a pinned major version to avoid
unintended updates — replace "chromaui/action@latest" with a major-pinned tag
like "chromaui/action@v15" in the CI workflow so updates are applied
intentionally while keeping the action stable.
In `@package.json`:
- Line 18: The package.json script "test:storybook" sets STORYBOOK_TEST=1 but
that env var isn't consumed; either remove it from the script or wire it into
the Vitest setup by reading process.env.STORYBOOK_TEST in vitest.config.ts
(and/or .storybook/vitest.setup.ts) and using it to toggle the intended behavior
(e.g., enable the storybook project, conditional setupFiles, or different test
options). Update the "test:storybook" script or update vitest.config.ts to check
process.env.STORYBOOK_TEST and apply the conditional configuration using the
existing project names or setup hooks referenced in vitest.config.ts and
.storybook/vitest.setup.ts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 91689c33-83a8-4d96-93e3-d101a33d50f9
📒 Files selected for processing (3)
.github/workflows/ci.ymlchromatic.config.jsonpackage.json
✅ Files skipped from review due to trivial changes (1)
- chromatic.config.json
| "@storybook/addon-a11y": "^10.3.1", | ||
| "@storybook/addon-docs": "^10.3.1", | ||
| "@storybook/addon-vitest": "^10.3.1", | ||
| "@storybook/nextjs-vite": "^10.3.1", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Storybook 10.3.1 release npm
💡 Result:
There is no Storybook 10.3.1 release available on npm as of 2026-03-19. The latest stable version is 10.2.19. Storybook 10.3.0 was released on GitHub on 2026-03-18 with improvements including Vite 8 support, Next.js 16.2 support, ESLint 10 support, and Storybook MCP preview for React, but it has not yet been published to npm (next tag shows 10.3.0-beta.3). To install the latest stable: npm install storybook@latest (10.2.19). For pre-releases: npm install storybook@next.
Citations:
- 1: https://github.com/storybookjs/storybook/releases
- 2: https://github.com/storybookjs/storybook/releases/tag/v10.3.0
- 3: https://npmjs.com/package/storybook?activeTab=versions
- 4: https://www.npmjs.com/package/storybook
- 5: https://www.npmjs.com/package/storybook?activeTab=versions
Storybook 10.3.1 does not exist on npm; use 10.2.19 or a pre-release version instead.
The package.json specifies Storybook packages at version ^10.3.1, but this version does not exist on npm as of March 2026. The latest stable version available is 10.2.19. Version 10.3.0 was released on GitHub but has not been published to npm (only pre-releases are available under the "next" tag as 10.3.0-beta.3). Update the version specifiers at lines 92–95 and 115, 123 to either ^10.2.19 or a valid pre-release tag.
Also applies to: 115-115, 123-123
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 92 - 95, The Storybook dependency versions
currently set to ^10.3.1 are invalid on npm; update the version specifiers for
the Storybook packages (e.g., "@storybook/addon-a11y", "@storybook/addon-docs",
"@storybook/addon-vitest", "@storybook/nextjs-vite" and the other Storybook
entries referenced later) to a valid published release such as ^10.2.19 or to a
published pre-release tag (for example the "next" 10.3.0-beta.3) so all
Storybook package entries use the same valid version specifier.
The play function queried getByRole('button', { name: 'Close' }) which
matched both the X close button AND the footer Close button — causing
ambiguity in Chromatic's headless test runner.
Fix: click the specific "Approve proof" action button (wrapped in
DialogClose, so it closes the dialog). Added step() grouping to both
Dialog stories for better Interactions panel debugging.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Watermarks (visual indicators in coverage reports): - statements/lines/functions: [70, 90] — red below floor, green at domain-quality - branches: [65, 85] — slightly lower, branches are harder to cover fully New threshold for UI components: - src/features/*/components/**: 60% lines/functions (initial floor) - Will raise as Storybook play function coverage grows Existing thresholds unchanged: - money.ts, pricing.service.ts: 100% (financial-critical) - domain/rules/**: 90% (business logic) - infrastructure/repositories/**: 80% (data access) - Overall floor: 70% Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Run npm run test:storybook (Vitest + Playwright browser) in CI after unit tests and coverage. This executes all play functions and a11y checks headlessly — catching interaction regressions on every PR. Playwright install is now unconditional (needed for both storybook and E2E tests). Storybook tests are blocking (not continue-on-error) since a11y is set to 'error' mode — violations should fail the PR. CI pipeline order: lint → types → architecture → unit → acceptance → build → coverage → playwright install → storybook tests → E2E (non-blocking) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Enable MDX file discovery in Storybook story patterns alongside .stories.tsx. This allows rich documentation pages using Doc Blocks (Meta, Canvas, Story, Controls) alongside component stories. Add badge-encoding.mdx — the canonical reference for the two-pool color system (status vs categorical), encoding channels, correct/incorrect examples, and live story embeds. This is the most common design mistake in the codebase and now has a dedicated Storybook docs page. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…rols Add component descriptions, argType descriptions, control types, and default values to all 15 primitive story files. Autodocs pages now show useful documentation instead of bare prop tables. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add component descriptions referencing design system rules, argType descriptions with proper controls, and default values to all 12 organism and atom story files. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Three new MDX foundation docs: - Color Rules: two-pool system, encoding channels, golden rule - Spacing & Rhythm: spacing scale, layout patterns, mobile tokens - Motion Guide: duration scale, easing, GPU rules, spring presets Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Documents the standard anatomy of a detail page using Customer Detail as the canonical reference: Topbar → Header → Tabs + Rail layout. Covers heading patterns, tab system, rail sections, field rows, stats patterns, empty states, and responsive behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Documents the standard anatomy of a list page using Customers List as the canonical reference: stats bar, smart view tabs, data table, URL state management, responsive card layout, and empty states. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Each MDX documentation page now includes a blockquote referencing the org-level standard it implements (from ops/standards/design-system/). Makes the ops/ → Storybook relationship explicit: ops/ is the authority, Storybook is the product-level projection of those standards. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove the close+verify steps that fail in Chromatic's headless env (dialog close animation timing differs). Keep open+verify — sufficient to test that the trigger works and content renders. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ents New skill: storybook-authoring - Complete reference for writing production-grade Storybook stories - Controls reference (argTypes, select/radio/range, conditional, mapping) - Play function patterns by component type - Autodocs enrichment checklist - Chromatic configuration, a11y override rules - Design token quick reference, MDX documentation patterns Updated: quality-gate — Category 11: Storybook Coverage Updated: AGENTS.md — frontend-builder now preloads storybook-authoring Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.claude/skills/quality-gate/SKILL.md (1)
27-41:⚠️ Potential issue | 🟡 MinorUpdate category count to avoid checklist drift.
Line 27 says “Audit categories (10 total)”, but Line 41 adds Category 11. Please update the total to 11 to keep the gate instructions accurate.
Suggested doc fix
-Audit categories (10 total): +Audit categories (11 total):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/quality-gate/SKILL.md around lines 27 - 41, Update the header text that currently reads "Audit categories (10 total)" to "Audit categories (11 total)" so the count matches the listed categories (there are 11 entries, including "Storybook Coverage"); locate the string "Audit categories (10 total)" in SKILL.md and change the numeral only, leaving the rest of the line and the category table intact.src/shared/ui/organisms/LifecycleBadge.stories.tsx (1)
51-63:⚠️ Potential issue | 🟡 MinorMissing
contractstage inAllStagesstory.The
argTypes.stage.optionscorrectly includes all 7 lifecycle stages, but this story only renders 6 —contractis missing. This leaves a gap in visual coverage.Proposed fix to include the missing stage
render: () => ( <div className="flex flex-col gap-3 p-4"> <LifecycleBadge stage="prospect" /> <LifecycleBadge stage="new" /> <LifecycleBadge stage="repeat" /> <LifecycleBadge stage="vip" /> <LifecycleBadge stage="at-risk" /> <LifecycleBadge stage="archived" /> + <LifecycleBadge stage="contract" /> </div> ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/organisms/LifecycleBadge.stories.tsx` around lines 51 - 63, The AllStages Story (export const AllStages) is missing the "contract" lifecycle stage, so update the render for AllStages to include a <LifecycleBadge stage="contract" /> alongside the other badges; ensure args (args: { stage: 'prospect' }) can remain or be adjusted but the primary fix is to add the missing LifecycleBadge("contract") to the render list so all seven stages from argTypes.stage.options are visually covered.
🧹 Nitpick comments (4)
src/shared/ui/organisms/TypeTagBadges.stories.tsx (1)
45-57:argsis unused inAllTagsstory.The
renderfunction ignores theargsparameter and hardcodes its owntagsvalues. This makes the Storybook controls panel misleading — users can modifytagsbut the output won't change.♻️ Proposed fix: remove unused args
export const AllTags: Story = { - args: { tags: ['retail'] }, render: () => ( <div className="flex flex-col gap-4 p-4"> <TypeTagBadges tags={['retail']} /> <TypeTagBadges tags={['sports-school']} /> <TypeTagBadges tags={['corporate']} /> <TypeTagBadges tags={['wholesale']} /> <TypeTagBadges tags={['nonprofit']} /> <TypeTagBadges tags={['sports-school', 'nonprofit']} /> </div> ), }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/organisms/TypeTagBadges.stories.tsx` around lines 45 - 57, The AllTags story currently defines args but the render ignores them; remove the unused args property from the AllTags export (delete the line "args: { tags: ['retail'] }") so Storybook controls are not misleading, or alternatively change render to accept and forward args (e.g., render: (args) => <TypeTagBadges {...args} />) — update the AllTags export and the render usage of TypeTagBadges accordingly.src/shared/ui/organisms/ServiceTypeBadge.stories.tsx (1)
44-74: Consider clarifying that Controls don't affect this showcase story.The
AllServiceTypesstory providesargsbut the customrenderfunction doesn't use them, meaning the Controls panel won't affect the displayed output. This is a valid pattern for showcase stories, but users might be confused when controls don't change anything.You could either add a note in the story parameters or use a decorator to clarify this is a static showcase:
export const AllServiceTypes: Story = { args: { serviceType: 'screen-print' }, + parameters: { + controls: { disable: true }, + }, render: () => (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/organisms/ServiceTypeBadge.stories.tsx` around lines 44 - 74, The story AllServiceTypes currently defines args but uses a custom render that ignores them, which makes the Controls panel non-functional; update the story to either remove the unused args or make the intent explicit by adding story parameters (e.g., docs or parameters: { controls: { disable: true }, docs: { description: { story: "Static showcase — Controls do not affect this story" } } }) or change render to consume args (pass serviceType/variant from args into ServiceTypeBadge) so Controls work; locate the AllServiceTypes export and adjust its args/parameters or render function accordingly to reflect the chosen approach.src/shared/ui/organisms/StatusBadge.stories.tsx (1)
90-96: Consider adding individual stories for remaining invoice statuses.All 5 quote statuses have dedicated stories (
QuoteDraftthroughQuoteRevised), but only 2 invoice statuses have individual stories (InvoicePaid,InvoiceVoid). For consistency and easier Storybook navigation, consider addingInvoiceDraft,InvoiceSent, andInvoicePartial.Proposed stories for remaining invoice statuses
export const InvoiceVoid: Story = { args: { status: 'void', variant: 'invoice' }, } + +export const InvoiceDraft: Story = { + args: { status: 'draft', variant: 'invoice' }, +} + +export const InvoiceSent: Story = { + args: { status: 'sent', variant: 'invoice' }, +} + +export const InvoicePartial: Story = { + args: { status: 'partial', variant: 'invoice' }, +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/organisms/StatusBadge.stories.tsx` around lines 90 - 96, Add individual Story exports for the missing invoice statuses to mirror the quote stories: create exported consts InvoiceDraft, InvoiceSent, and InvoicePartial in StatusBadge.stories.tsx that follow the same pattern as InvoicePaid and InvoiceVoid (each should be typed as Story and set args to { status: '<draft|sent|partial>', variant: 'invoice' } respectively); place them alongside InvoicePaid and InvoiceVoid so Storybook shows all five invoice states consistently.src/shared/ui/primitives/tooltip.stories.tsx (1)
46-52: AddTooltipProvideras a global decorator in.storybook/preview.tsto eliminate repetition across story files.Currently,
TooltipProvideris added individually in 6+ story files (tooltip.stories.tsx, sidebar.stories.tsx, LifecycleBadge.stories.tsx, HealthBadge.stories.tsx, ColorSwatchStrip.stories.tsx). Centralizing it in the Storybook global decorators config would prevent this duplication and ensure consistent tooltip behavior across all stories.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/primitives/tooltip.stories.tsx` around lines 46 - 52, Move the inline TooltipProvider wrapper out of individual stories and register it as a global Storybook decorator: add a decorator that returns <TooltipProvider><Story/></TooltipProvider> to the Storybook global decorators array (the preview config) so TooltipProvider is applied to all stories and you can remove the per-story decorators (references: TooltipProvider, Story, decorators in tooltip.stories.tsx and other story files).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@stories/foundations/motion-guide.mdx`:
- Around line 25-26: Replace the nonstandard range punctuation and spacing in
the duration text "150 -- 300ms" with a consistent style like "150–300 ms";
locate the string in the motion guide content (the sentence starting "If a user
can perceive a delay...") and update the dash to an en dash and add a space
before the unit so it reads "150–300 ms".
- Around line 61-77: Update the "GPU-only" animation rule in motion-guide.mdx to
avoid an absolute prohibition: change the wording around the allowed/never lists
to state that GPU-composited properties (transform, opacity) are preferred for
60fps, but document explicit, justified exceptions (e.g., decorative effects) in
a new "Exceptions" subsection; specifically reference the existing `lane-glow`
keyframes and either (a) call out `box-shadow` there as a documented,
performance-accepted exception with guidance (pre-render stages, limit
spread/blur, low frequency, fallback/disable on low-power) or (b) move the
`lane-glow` example into that Exceptions section and add a clear label
explaining why `box-shadow` is allowed in that case.
In `@stories/patterns/detail-page-anatomy.mdx`:
- Around line 368-373: Rephrase the “Import boundary rules” section to present
these as the intended/target architecture rather than absolute current-state
rules: change the header to something like “Intended import boundary rules” and
update bullets so “page.tsx imports…” becomes “Intended: page.tsx should import
from…”, change “Never import _components/ from outside the route segment” to
“Avoid importing _components/ from outside the route segment”, and change
“Shared components live in…” to “Shared components are intended to live in
features/*/components/ and shared/ui/”; also append a short link to the
enforcement/source-of-truth (the repo’s enforcement policy) so readers can find
current rules and known exceptions.
---
Outside diff comments:
In @.claude/skills/quality-gate/SKILL.md:
- Around line 27-41: Update the header text that currently reads "Audit
categories (10 total)" to "Audit categories (11 total)" so the count matches the
listed categories (there are 11 entries, including "Storybook Coverage"); locate
the string "Audit categories (10 total)" in SKILL.md and change the numeral
only, leaving the rest of the line and the category table intact.
In `@src/shared/ui/organisms/LifecycleBadge.stories.tsx`:
- Around line 51-63: The AllStages Story (export const AllStages) is missing the
"contract" lifecycle stage, so update the render for AllStages to include a
<LifecycleBadge stage="contract" /> alongside the other badges; ensure args
(args: { stage: 'prospect' }) can remain or be adjusted but the primary fix is
to add the missing LifecycleBadge("contract") to the render list so all seven
stages from argTypes.stage.options are visually covered.
---
Nitpick comments:
In `@src/shared/ui/organisms/ServiceTypeBadge.stories.tsx`:
- Around line 44-74: The story AllServiceTypes currently defines args but uses a
custom render that ignores them, which makes the Controls panel non-functional;
update the story to either remove the unused args or make the intent explicit by
adding story parameters (e.g., docs or parameters: { controls: { disable: true
}, docs: { description: { story: "Static showcase — Controls do not affect this
story" } } }) or change render to consume args (pass serviceType/variant from
args into ServiceTypeBadge) so Controls work; locate the AllServiceTypes export
and adjust its args/parameters or render function accordingly to reflect the
chosen approach.
In `@src/shared/ui/organisms/StatusBadge.stories.tsx`:
- Around line 90-96: Add individual Story exports for the missing invoice
statuses to mirror the quote stories: create exported consts InvoiceDraft,
InvoiceSent, and InvoicePartial in StatusBadge.stories.tsx that follow the same
pattern as InvoicePaid and InvoiceVoid (each should be typed as Story and set
args to { status: '<draft|sent|partial>', variant: 'invoice' } respectively);
place them alongside InvoicePaid and InvoiceVoid so Storybook shows all five
invoice states consistently.
In `@src/shared/ui/organisms/TypeTagBadges.stories.tsx`:
- Around line 45-57: The AllTags story currently defines args but the render
ignores them; remove the unused args property from the AllTags export (delete
the line "args: { tags: ['retail'] }") so Storybook controls are not misleading,
or alternatively change render to accept and forward args (e.g., render: (args)
=> <TypeTagBadges {...args} />) — update the AllTags export and the render usage
of TypeTagBadges accordingly.
In `@src/shared/ui/primitives/tooltip.stories.tsx`:
- Around line 46-52: Move the inline TooltipProvider wrapper out of individual
stories and register it as a global Storybook decorator: add a decorator that
returns <TooltipProvider><Story/></TooltipProvider> to the Storybook global
decorators array (the preview config) so TooltipProvider is applied to all
stories and you can remove the per-story decorators (references:
TooltipProvider, Story, decorators in tooltip.stories.tsx and other story
files).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5ff41347-9e92-42ee-bb19-3ea9873f0383
📒 Files selected for processing (41)
.claude/agents/AGENTS.md.claude/skills/quality-gate/SKILL.md.claude/skills/storybook-authoring/SKILL.md.claude/skills/storybook-authoring/reference/controls-reference.md.github/workflows/ci.yml.storybook/main.tschromatic.config.jsonsrc/shared/ui/atoms/FieldRow.stories.tsxsrc/shared/ui/atoms/SectionHeader.stories.tsxsrc/shared/ui/organisms/ColorSwatchStrip.stories.tsxsrc/shared/ui/organisms/FavoriteStar.stories.tsxsrc/shared/ui/organisms/GarmentImage.stories.tsxsrc/shared/ui/organisms/GarmentMiniCard.stories.tsxsrc/shared/ui/organisms/HealthBadge.stories.tsxsrc/shared/ui/organisms/LifecycleBadge.stories.tsxsrc/shared/ui/organisms/MoneyAmount.stories.tsxsrc/shared/ui/organisms/ServiceTypeBadge.stories.tsxsrc/shared/ui/organisms/StatusBadge.stories.tsxsrc/shared/ui/organisms/TypeTagBadges.stories.tsxsrc/shared/ui/primitives/badge.stories.tsxsrc/shared/ui/primitives/button.stories.tsxsrc/shared/ui/primitives/card.stories.tsxsrc/shared/ui/primitives/checkbox.stories.tsxsrc/shared/ui/primitives/dialog.stories.tsxsrc/shared/ui/primitives/dropdown-menu.stories.tsxsrc/shared/ui/primitives/input.stories.tsxsrc/shared/ui/primitives/label.stories.tsxsrc/shared/ui/primitives/select.stories.tsxsrc/shared/ui/primitives/separator.stories.tsxsrc/shared/ui/primitives/sheet.stories.tsxsrc/shared/ui/primitives/switch.stories.tsxsrc/shared/ui/primitives/tabs.stories.tsxsrc/shared/ui/primitives/textarea.stories.tsxsrc/shared/ui/primitives/tooltip.stories.tsxstories/foundations/color-rules.mdxstories/foundations/motion-guide.mdxstories/foundations/spacing-rhythm.mdxstories/patterns/badge-encoding.mdxstories/patterns/detail-page-anatomy.mdxstories/patterns/list-page-anatomy.mdxvitest.config.ts
✅ Files skipped from review due to trivial changes (14)
- chromatic.config.json
- src/shared/ui/primitives/input.stories.tsx
- src/shared/ui/organisms/HealthBadge.stories.tsx
- stories/foundations/spacing-rhythm.mdx
- stories/patterns/badge-encoding.mdx
- stories/patterns/list-page-anatomy.mdx
- src/shared/ui/atoms/SectionHeader.stories.tsx
- src/shared/ui/primitives/textarea.stories.tsx
- src/shared/ui/primitives/switch.stories.tsx
- src/shared/ui/atoms/FieldRow.stories.tsx
- src/shared/ui/organisms/FavoriteStar.stories.tsx
- src/shared/ui/organisms/GarmentImage.stories.tsx
- stories/foundations/color-rules.mdx
- src/shared/ui/organisms/GarmentMiniCard.stories.tsx
🚧 Files skipped from review as they are similar to previous changes (12)
- .github/workflows/ci.yml
- src/shared/ui/primitives/dialog.stories.tsx
- src/shared/ui/organisms/ColorSwatchStrip.stories.tsx
- src/shared/ui/primitives/sheet.stories.tsx
- src/shared/ui/primitives/checkbox.stories.tsx
- src/shared/ui/primitives/dropdown-menu.stories.tsx
- src/shared/ui/organisms/MoneyAmount.stories.tsx
- src/shared/ui/primitives/card.stories.tsx
- src/shared/ui/primitives/label.stories.tsx
- .storybook/main.ts
- src/shared/ui/primitives/tabs.stories.tsx
- src/shared/ui/primitives/select.stories.tsx
| **Rule**: If a user can perceive a delay, the animation is too slow. Most UI animations | ||
| should be 150 -- 300ms. |
There was a problem hiding this comment.
Use standard range punctuation/spacing for duration text.
“150 -- 300ms” should be formatted consistently (e.g., “150–300 ms”) for readability and style consistency.
🧰 Tools
🪛 LanguageTool
[grammar] ~26-~26: Ensure spelling is correct
Context: ...ow. Most UI animations should be 150 -- 300ms. --- ## Easing | Easing | C...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stories/foundations/motion-guide.mdx` around lines 25 - 26, Replace the
nonstandard range punctuation and spacing in the duration text "150 -- 300ms"
with a consistent style like "150–300 ms"; locate the string in the motion guide
content (the sentence starting "If a user can perceive a delay...") and update
the dash to an en dash and add a space before the unit so it reads "150–300 ms".
| Animations **must** use only GPU-composited CSS properties for 60fps performance: | ||
|
|
||
| | Allowed Property | Use For | | ||
| | -------------------------------------- | -------------------------------- | | ||
| | `transform` (translate, scale, rotate) | Movement, size changes, rotation | | ||
| | `opacity` | Fade in/out, show/hide | | ||
|
|
||
| ### Never Animate | ||
|
|
||
| | Property | Why Not | Alternative | | ||
| | -------------------------------- | ----------------------------- | -------------------------------------- | | ||
| | `width`, `height` | Triggers layout recalculation | `transform: scale()` | | ||
| | `top`, `left`, `right`, `bottom` | Triggers layout recalculation | `transform: translate()` | | ||
| | `margin`, `padding` | Triggers layout recalculation | `transform: translate()` or `gap` | | ||
| | `border-width` | Triggers layout + paint | `box-shadow` or `outline` | | ||
| | `box-shadow` | Triggers paint | Pre-render both states, fade `opacity` | | ||
|
|
There was a problem hiding this comment.
Clarify the “GPU-only” rule to account for documented exceptions.
This section states an absolute rule (“must use only GPU-composited properties” / never animate box-shadow), but later the lane-glow keyframe example animates box-shadow (Line 147, Line 150). Please either (a) make the rule non-absolute with explicit exceptions, or (b) move non-performant effects into a clearly labeled exception section.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stories/foundations/motion-guide.mdx` around lines 61 - 77, Update the
"GPU-only" animation rule in motion-guide.mdx to avoid an absolute prohibition:
change the wording around the allowed/never lists to state that GPU-composited
properties (transform, opacity) are preferred for 60fps, but document explicit,
justified exceptions (e.g., decorative effects) in a new "Exceptions"
subsection; specifically reference the existing `lane-glow` keyframes and either
(a) call out `box-shadow` there as a documented, performance-accepted exception
with guidance (pre-render stages, limit spread/blur, low frequency,
fallback/disable on low-power) or (b) move the `lane-glow` example into that
Exceptions section and add a clear label explaining why `box-shadow` is allowed
in that case.
| ### Import boundary rules | ||
|
|
||
| - `page.tsx` imports from `@infra/repositories/`, `@domain/`, `@shared/`, and local `_components/` | ||
| - `_components/` import from `@features/`, `@shared/`, `@domain/`, and sibling `_components/` | ||
| - Never import `_components/` from outside the route segment | ||
| - Shared components live in `features/*/components/` and `shared/ui/` |
There was a problem hiding this comment.
Clarify these boundary rules as target-state, not universally enforced current-state.
This section reads as absolute policy, but current architecture config/known-violations indicate exceptions still exist (e.g., feature → app action imports). Please reword to “intended pattern” and link to the enforcement source of truth to avoid misleading readers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stories/patterns/detail-page-anatomy.mdx` around lines 368 - 373, Rephrase
the “Import boundary rules” section to present these as the intended/target
architecture rather than absolute current-state rules: change the header to
something like “Intended import boundary rules” and update bullets so “page.tsx
imports…” becomes “Intended: page.tsx should import from…”, change “Never import
_components/ from outside the route segment” to “Avoid importing _components/
from outside the route segment”, and change “Shared components live in…” to
“Shared components are intended to live in features/*/components/ and
shared/ui/”; also append a short link to the enforcement/source-of-truth (the
repo’s enforcement policy) so readers can find current rules and known
exceptions.
npm ci was failing because package-lock.json still referenced storybook 10.2.19 while package.json required 10.3.1 (from the chromatic setup that triggered an auto-upgrade). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Full design system UI overhaul + Storybook production-grade setup. 32 commits, 103 files changed, +5,604 lines.
Wave 0 — Foundation
@theme inline(text-label, text-micro, text-compact, text-stat, text-page-title, text-page-title-md, tracking-label, tracking-stat-label, tracking-heading)Wave 1 — Storybook Completeness (15 → 38 stories)
Wave 2 — Screen Application (all 31 screens)
Storybook Infrastructure
Documentation (6 MDX pages)
Skills & Agents
storybook-authoringskill with controls referencequality-gateupdated with Category 11: Storybook Coveragefrontend-builderagent preloads storybook-authoringTest plan
npx tsc --noEmit— 0 errorsnpm run lint— 0 errorsnpm run build— clean production build🤖 Generated with Claude Code