refactor(web): replace Sidebar tablet overlay with shared Drawer component#928
refactor(web): replace Sidebar tablet overlay with shared Drawer component#928
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🧰 Additional context used📓 Path-based instructions (3)web/src/**/*.{tsx,ts}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
web/src/**/*.tsx📄 CodeRabbit inference engine (CLAUDE.md)
Files:
web/src/components/ui/**/*.{tsx,ts}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (9)📓 Common learnings📚 Learning: 2026-03-30T10:40:15.387ZApplied to files:
📚 Learning: 2026-03-30T10:40:15.387ZApplied to files:
📚 Learning: 2026-03-30T10:40:15.387ZApplied to files:
📚 Learning: 2026-03-30T10:40:15.387ZApplied to files:
📚 Learning: 2026-03-30T10:40:15.387ZApplied to files:
📚 Learning: 2026-03-30T10:40:15.387ZApplied to files:
📚 Learning: 2026-03-30T10:40:15.387ZApplied to files:
📚 Learning: 2026-03-30T10:40:15.387ZApplied to files:
🔇 Additional comments (7)
WalkthroughThe Drawer component API was changed: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Code Review
This pull request enhances the Drawer component by adding support for a side prop (allowing left or right positioning), an optional header (headerless mode), and a contentClassName prop for style overrides. It also refactors the Sidebar component to utilize this shared Drawer for its tablet overlay mode, improving code reuse and consistency. Documentation, stories, and unit tests have been updated to reflect these changes. I have no feedback to provide.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches. Re-running this action after a short time may resolve the issue. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/components/layout/Sidebar.tsx`:
- Around line 34-35: The component currently defines NOOP and allows an invalid
prop pairing where overlayOpen can be true while onOverlayClose is absent,
making dismiss actions inert; change the SidebarProps union to explicitly pair
overlayOpen with onOverlayClose the same way DrawerProps pairs title/ariaLabel
so callers cannot pass overlayOpen:true without a real onOverlayClose. Update
the SidebarProps type (and any related prop usage at the component signature and
type imports) to a discriminated union such as one branch for overlayOpen?:
false with optional onOverlayClose, and another branch for overlayOpen: true (or
boolean) that requires onOverlayClose, and remove reliance on NOOP as a
fallback.
- Around line 120-149: The Drawer overrides currently still yield a 320px panel
because Drawer enforces min-w-80 and the wrapper is a block; update the Drawer
props used in Sidebar.tsx so the outer panel and inner wrapper enforce 240px and
column flex: set the Drawer className to fix width/min/max to 240px (e.g.,
include w-60 plus min-w-[240px] and max-w-[240px] or equivalent) and change
contentClassName from "p-0" to "p-0 flex flex-col" so SidebarNav
(collapsed={false}) can keep its flex-1 behavior and push SidebarFooter to the
bottom; keep onOverlayClose, overlayOpen, SidebarNav and SidebarFooter props
unchanged.
- Around line 130-136: The tablet drawer close button (the button with
onClick={onOverlayClose} and the <X /> icon) has only a 28×28 hit area; increase
its touch target to at least 32×32 by matching the shared Drawer button
treatment—replace the current className ("rounded-md p-1 text-muted-foreground
hover:bg-card-hover hover:text-foreground") with the same classes used by the
app's Drawer close buttons (use p-2 or the shared Drawer button utility
classes), keep the X icon (size-5) and aria-label="Close navigation menu", and
ensure the button also includes the same focus outline/focus-visible styles so
focus and touch behavior stay consistent with other drawers.
In `@web/src/components/ui/drawer.tsx`:
- Around line 34-40: The getPanelVariants helper currently hardcodes
spring/tween values; change it to use the shared motion layer exports so Drawer
picks up centralized tuning and reduced-motion handling—import the appropriate
configs/variants from the motion module (the shared export used by other
components, e.g., the module's spring/tween/variant exports) and replace the
inline transition objects in getPanelVariants with those shared values (so
hidden/visible/exit use the motion layer's transition/variant constants),
ensuring getPanelVariants and the Drawer component now rely on the shared motion
variants rather than hardcoded numbers.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cdd64a72-0b22-4e61-9096-696249cc86d6
📒 Files selected for processing (8)
CLAUDE.mddocs/design/brand-and-ux.mddocs/design/ux-guidelines.mdweb/src/__tests__/components/layout/Sidebar.test.tsxweb/src/__tests__/components/ui/drawer.test.tsxweb/src/components/layout/Sidebar.tsxweb/src/components/ui/drawer.stories.tsxweb/src/components/ui/drawer.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Deploy Preview
- GitHub Check: Dashboard Test
- GitHub Check: Build Sandbox
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: TypeScript: Use camelCase for variable and function names
TypeScript: Always use async/await for promises instead of .then() chains
Files:
web/src/__tests__/components/ui/drawer.test.tsxweb/src/components/ui/drawer.stories.tsxweb/src/components/layout/Sidebar.tsxweb/src/__tests__/components/layout/Sidebar.test.tsxweb/src/components/ui/drawer.tsx
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
web/src/**/*.{ts,tsx}: Always reuse existing components from web/src/components/ui/ (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, DeptHealthBar, ProgressGauge, StatPill, Avatar, Button, Toast/ToastContainer, Skeleton variants, EmptyState, ErrorBoundary, ConfirmDialog, CommandPalette, InlineEdit, AnimatedPresence, StaggerGroup/StaggerItem, Drawer, form fields, TaskStatusIndicator, PriorityBadge, ProviderHealthBadge, TokenUsageBar, CodeMirrorEditor, SegmentedControl, ThemeToggle, LiveRegion, MobileUnsupportedOverlay, LazyCodeMirrorEditor) before creating new components
Web dashboard design tokens: use Tailwind semantic classes (text-foreground, bg-card, text-accent, text-success, bg-danger, etc.) or CSS variables (var(--so-accent)); never hardcode hex values in .tsx/.ts files
Web dashboard typography: use font-sans or font-mono (maps to Geist tokens); never set fontFamily directly in .tsx/.ts files
Web dashboard spacing: use density-aware tokens (p-card, gap-section-gap, gap-grid-gap) or standard Tailwind spacing; never hardcode pixel values for layout spacing
Web dashboard shadows/borders: use token variables (var(--so-shadow-card-hover), border-border, border-bright); never hardcode shadow or border values
Do NOT recreate status dots inline—use StatusBadge component
Do NOT build card-with-header layouts from scratch—use SectionCard component
Do NOT create metric displays with text-metric font-bold—use MetricCard component
Do NOT render initials circles manually—use Avatar component
Do NOT create complex (>8 line) JSX inside .map()—extract to a shared component
Do NOT use rgba() with hardcoded values—use design token variables
Web dashboard ESLint: Zero warnings enforced; use npm run lint to check and npm run lint --fix for auto-fix
Files:
web/src/__tests__/components/ui/drawer.test.tsxweb/src/components/ui/drawer.stories.tsxweb/src/components/layout/Sidebar.tsxweb/src/__tests__/components/layout/Sidebar.test.tsxweb/src/components/ui/drawer.tsx
web/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Web dashboard: Use React Hypothesis (fast-check) for property-based testing with fc.assert + fc.property
Files:
web/src/__tests__/components/ui/drawer.test.tsxweb/src/__tests__/components/layout/Sidebar.test.tsx
web/src/components/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
When creating new shared web components, place in web/src/components/ui/ with kebab-case filename, create .stories.tsx alongside with all states (default, hover, loading, error, empty), export props as TypeScript interface, use design tokens exclusively with no hardcoded colors/fonts/spacing, and import cn from
@/lib/utilsfor conditional class merging
Files:
web/src/components/ui/drawer.stories.tsxweb/src/components/ui/drawer.tsx
web/**/*.stories.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
web/**/*.stories.{ts,tsx}: Storybook 10: Use storybook/test (not@storybook/test) and storybook/actions (not@storybook/addon-actions) import paths
Storybook 10: Use parameters.backgrounds.options (object keyed by name) + initialGlobals.backgrounds.value for background options (replaces old default + values array)
Storybook 10: Use parameters.a11y.test: 'error' | 'todo' | 'off' for a11y testing (replaces old .element and .manual); set globally in preview.tsx to enforce WCAG compliance on all stories
Files:
web/src/components/ui/drawer.stories.tsx
🧠 Learnings (11)
📚 Learning: 2026-03-18T08:23:08.912Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T08:23:08.912Z
Learning: When approved deviations occur, update the relevant `docs/design/` page to reflect the new reality.
Applied to files:
docs/design/ux-guidelines.md
📚 Learning: 2026-03-30T10:09:41.160Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:09:41.160Z
Learning: Applies to web/src/**/*.{ts,tsx} : Always reuse existing components from web/src/components/ui/ (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, DeptHealthBar, ProgressGauge, StatPill, Avatar, Button, Toast/ToastContainer, Skeleton variants, EmptyState, ErrorBoundary, ConfirmDialog, CommandPalette, InlineEdit, AnimatedPresence, StaggerGroup/StaggerItem, Drawer, form fields, TaskStatusIndicator, PriorityBadge, ProviderHealthBadge, TokenUsageBar, CodeMirrorEditor, SegmentedControl, ThemeToggle, LiveRegion, MobileUnsupportedOverlay, LazyCodeMirrorEditor) before creating new components
Applied to files:
CLAUDE.mddocs/design/brand-and-ux.mdweb/src/components/ui/drawer.stories.tsxweb/src/components/layout/Sidebar.tsxweb/src/components/ui/drawer.tsx
📚 Learning: 2026-03-15T21:20:09.993Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:20:09.993Z
Learning: Applies to web/src/components/** : Vue components organized by feature (agents/, approvals/, budget/, common/, dashboard/, layout/, messages/, org-chart/, tasks/).
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-30T10:09:41.160Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:09:41.160Z
Learning: Applies to web/**/*.test.{ts,tsx} : Web dashboard: Use React Hypothesis (fast-check) for property-based testing with fc.assert + fc.property
Applied to files:
web/src/__tests__/components/ui/drawer.test.tsxweb/src/__tests__/components/layout/Sidebar.test.tsx
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to web/src/__tests__/**/*.{ts,js} : Dashboard testing: Vitest unit tests organized by feature under `web/src/__tests__/`. Use fast-check for property-based testing (`fc.assert` + `fc.property`).
Applied to files:
web/src/__tests__/components/ui/drawer.test.tsxweb/src/__tests__/components/layout/Sidebar.test.tsx
📚 Learning: 2026-03-30T10:09:41.160Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:09:41.160Z
Learning: Applies to web/src/**/*.{ts,tsx} : Do NOT create complex (>8 line) JSX inside .map()—extract to a shared component
Applied to files:
docs/design/brand-and-ux.md
📚 Learning: 2026-03-30T10:09:41.160Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:09:41.160Z
Learning: Applies to web/src/components/ui/**/*.{ts,tsx} : When creating new shared web components, place in web/src/components/ui/ with kebab-case filename, create .stories.tsx alongside with all states (default, hover, loading, error, empty), export props as TypeScript interface, use design tokens exclusively with no hardcoded colors/fonts/spacing, and import cn from `@/lib/utils` for conditional class merging
Applied to files:
docs/design/brand-and-ux.mdweb/src/components/ui/drawer.stories.tsxweb/src/components/ui/drawer.tsx
📚 Learning: 2026-03-30T10:09:41.160Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:09:41.160Z
Learning: Applies to web/**/*.stories.{ts,tsx} : Storybook 10: Use storybook/test (not storybook/test) and storybook/actions (not storybook/addon-actions) import paths
Applied to files:
web/src/components/ui/drawer.stories.tsxweb/src/__tests__/components/layout/Sidebar.test.tsx
📚 Learning: 2026-03-30T10:09:41.160Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:09:41.160Z
Learning: Applies to web/**/*.stories.{ts,tsx} : Storybook 10: Use parameters.backgrounds.options (object keyed by name) + initialGlobals.backgrounds.value for background options (replaces old default + values array)
Applied to files:
web/src/components/ui/drawer.stories.tsx
📚 Learning: 2026-03-30T10:09:41.160Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:09:41.160Z
Learning: Applies to web/**/*.stories.{ts,tsx} : Storybook 10: Use parameters.a11y.test: 'error' | 'todo' | 'off' for a11y testing (replaces old .element and .manual); set globally in preview.tsx to enforce WCAG compliance on all stories
Applied to files:
web/src/components/ui/drawer.stories.tsxweb/src/__tests__/components/layout/Sidebar.test.tsx
📚 Learning: 2026-03-30T10:09:41.160Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:09:41.160Z
Learning: Applies to web/src/**/*.{ts,tsx} : Web dashboard shadows/borders: use token variables (var(--so-shadow-card-hover), border-border, border-bright); never hardcode shadow or border values
Applied to files:
web/src/components/layout/Sidebar.tsxweb/src/components/ui/drawer.tsx
🔇 Additional comments (1)
web/src/components/ui/drawer.tsx (1)
18-27: Nice type-level accessibility guard.The discriminated union makes it impossible to render a headerless drawer without an accessible name, which is exactly the right constraint for this public props surface.
| const NOOP = () => {} | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Make the overlay-close pair explicit in SidebarProps.
NOOP hides a broken prop combination: a caller can render the tablet overlay open without a real onOverlayClose, and every dismiss path becomes inert. Encode that pairing in the prop type the same way DrawerProps does for title/ariaLabel.
♻️ Example shape
type SidebarProps =
| { overlayOpen?: false; onOverlayClose?: () => void }
| { overlayOpen: boolean; onOverlayClose: () => void }+ const handleOverlayClose = onOverlayClose ?? NOOP
...
- onClose={onOverlayClose ?? NOOP}
+ onClose={handleOverlayClose}
...
- onClick={onOverlayClose}
+ onClick={handleOverlayClose}Also applies to: 120-136
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/layout/Sidebar.tsx` around lines 34 - 35, The component
currently defines NOOP and allows an invalid prop pairing where overlayOpen can
be true while onOverlayClose is absent, making dismiss actions inert; change the
SidebarProps union to explicitly pair overlayOpen with onOverlayClose the same
way DrawerProps pairs title/ariaLabel so callers cannot pass overlayOpen:true
without a real onOverlayClose. Update the SidebarProps type (and any related
prop usage at the component signature and type imports) to a discriminated union
such as one branch for overlayOpen?: false with optional onOverlayClose, and
another branch for overlayOpen: true (or boolean) that requires onOverlayClose,
and remove reliance on NOOP as a fallback.
a8e57fb to
f778c32
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
web/src/components/layout/Sidebar.tsx (1)
57-61:⚠️ Potential issue | 🟠 MajorMake
overlayOpenandonOverlayClosea compile-time pair (runtime warning is not enough).Right now Line 127 allows
open={true}with a noop close handler, and Line 137 can still be inert ifonOverlayCloseis missing. This keeps a broken prop combination representable.♻️ Proposed fix
-interface SidebarProps { - /** Whether the overlay sidebar is visible (used at tablet breakpoints). */ - overlayOpen?: boolean - /** Called when the overlay requests close. Required when overlayOpen is used. */ - onOverlayClose?: () => void -} +type SidebarProps = + | { + /** Overlay disabled/closed path */ + overlayOpen?: false + onOverlayClose?: () => void + } + | { + /** Overlay-enabled path */ + overlayOpen: boolean + onOverlayClose: () => void + } export function Sidebar({ overlayOpen = false, onOverlayClose }: SidebarProps) { @@ - <Drawer + <Drawer open={overlayOpen} - onClose={onOverlayClose ?? (() => {})} + onClose={onOverlayClose} @@ - onClick={onOverlayClose} + onClick={onOverlayClose}Also applies to: 125-127, 137-137
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/layout/Sidebar.tsx` around lines 57 - 61, Make the overlayOpen/onOverlayClose pair a compile-time-checked discriminated union instead of two independent optional props: replace the SidebarProps interface with a union such as one variant where overlayOpen is true and onOverlayClose: () => void is required, and the other variant where overlayOpen is false or undefined and onOverlayClose is omitted/never; update the Sidebar component signature to use this SidebarProps union so TypeScript enforces the pairing (and then fix call sites that passed open={true} without a real handler to provide one or change open to false).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/design/brand-and-ux.md`:
- Line 225: The docs entry for Drawer is ambiguous about accessible-name
requirements: update the description for the Drawer component (drawer.tsx) to
state that either the title prop or ariaLabel prop must be provided (i.e., at
least one is required) so the component always has an accessible name; reference
the prop names `title?` and `ariaLabel?` and mention the constraint explicitly
in the same table cell or a short note so it matches the Drawer prop contract
and accessibility behavior (focus trap/keyboard interactions).
In `@web/src/__tests__/components/layout/Sidebar.test.tsx`:
- Around line 227-336: Add a fast-check property test that asserts the tablet
overlay's close callback fires exactly once for any route navigation while open:
create a new it(...) test that uses fc.assert(fc.asyncProperty(...)) (import fc)
and inside the property call setupTablet(true, onOverlayClose) (referencing the
setupTablet helper), clear the mock, perform navigation via
router.navigate(target) inside act, and assert onOverlayClose was called once;
ensure the property uses fc.constantFrom with several routes (e.g.
'/settings','/agents','/providers','/tasks') and the test matches the style of
existing async tests in this file.
In `@web/src/components/ui/drawer.tsx`:
- Around line 77-80: The focus-trap early return causes Tab to escape when the
header/close button is omitted; modify the focusable collection logic in the
drawer focus-trap (the code that builds the focusable NodeList and checks
focusable.length) to treat the panel itself as a fallback tabbable element when
no tabbable descendants exist—i.e., if focusable.length === 0, replace or
fallback to an array containing the panel (or otherwise add a tabindex=0
sentinel on the panel) so the trap can cycle focus inside; make the same change
in the other focus-trap block referenced around lines 138-155 to ensure
consistent behavior.
- Around line 49-53: The current logic treats empty strings as valid names
because it uses nullish coalescing (ariaLabel ?? title) and the dev-only warning
only checks for nullish values; update both the runtime accessible-name
resolution and the useEffect check to treat empty or whitespace-only strings as
missing by using truthy/trim checks instead of ?? (e.g., compute const
accessibleName = ariaLabel?.trim() ? ariaLabel : title?.trim() ? title :
undefined) and change the warning condition in useEffect to if
(process.env.NODE_ENV !== 'production' && !(ariaLabel?.trim() || title?.trim()))
so empty strings no longer win and the dialog never ends up unnamed; make
changes around the useEffect, ariaLabel/title usage, and wherever accessibleName
is derived (references: useEffect, ariaLabel, title).
- Line 132: The drawer currently uses an arbitrary viewport width token
'w-[40vw]' inside the class list ('fixed inset-y-0 z-50 flex w-[40vw] min-w-80
max-w-xl flex-col'); replace that hardcoded vw value with a semantic
design-token or standard Tailwind width utility so the component follows the
design system (for example, use a named token like 'w-card' or a standard width
class such as 'w-80'/'w-96' or the established project token for card/drawer
width) and keep the existing min/max constraints ('min-w-80' and 'max-w-xl') to
preserve responsiveness. Ensure you update the class string in the Drawer
component (the element with the 'fixed inset-y-0 z-50 flex ...' classes) and run
the UI token mapping so it aligns with the design tokens used across the app.
---
Duplicate comments:
In `@web/src/components/layout/Sidebar.tsx`:
- Around line 57-61: Make the overlayOpen/onOverlayClose pair a
compile-time-checked discriminated union instead of two independent optional
props: replace the SidebarProps interface with a union such as one variant where
overlayOpen is true and onOverlayClose: () => void is required, and the other
variant where overlayOpen is false or undefined and onOverlayClose is
omitted/never; update the Sidebar component signature to use this SidebarProps
union so TypeScript enforces the pairing (and then fix call sites that passed
open={true} without a real handler to provide one or change open to false).
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: bf345582-89a4-4cab-b432-d4cb04a31d33
📒 Files selected for processing (9)
CLAUDE.mddocs/design/brand-and-ux.mddocs/design/page-structure.mddocs/design/ux-guidelines.mdweb/src/__tests__/components/layout/Sidebar.test.tsxweb/src/__tests__/components/ui/drawer.test.tsxweb/src/components/layout/Sidebar.tsxweb/src/components/ui/drawer.stories.tsxweb/src/components/ui/drawer.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Dashboard Test
- GitHub Check: Build Web
- GitHub Check: Build Sandbox
- GitHub Check: Build Backend
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (5)
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
web/src/**/*.{ts,tsx}: Enforce zero ESLint warnings in web dashboard vianpm --prefix web run lint
ALWAYS reuse existing components fromweb/src/components/ui/before creating new ones; refer to design system inventory (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, etc.)
Use Tailwind semantic classes (text-foreground,bg-card,text-accent,text-success,bg-danger, etc.) or CSS variables (var(--so-accent)); NEVER hardcode hex values in.tsx/.tsfiles
Use Tailwind utility fonts (font-sansorfont-monomapping to Geist tokens); NEVER setfontFamilydirectly
Use density-aware spacing tokens (p-card,gap-section-gap,gap-grid-gap) or standard Tailwind spacing; NEVER hardcode pixel values for layout spacing
Use token variables for shadows/borders (var(--so-shadow-card-hover),border-border,border-bright); no hardcoded values
Do NOT recreate status dots inline; use<StatusBadge>
Do NOT build card-with-header layouts from scratch; use<SectionCard>
Do NOT create metric displays with inline styles; use<MetricCard>
Do NOT render initials circles manually; use<Avatar>
Do NOT create complex (>8 line) JSX inside.map(); extract to a shared component
Do NOT usergba()with hardcoded values; use design token variables
MountToastContaineronce in AppLayout; all toast notifications use sharedToastcomponent fromweb/src/components/ui/toast
MountCommandPaletteonce in AppLayout; register commands viauseCommandPalettehook
Use@/components/ui/animated-presence(Framer Motion AnimatePresence keyed by route) for page transitions
Use Zustand stores for state management in the web dashboard; each domain has its own store module (auth, WebSocket, toast, analytics, setup, company, agents, budget, tasks, settings, providers, theme, per-domain stores)
Files:
web/src/components/ui/drawer.stories.tsxweb/src/components/layout/Sidebar.tsxweb/src/__tests__/components/ui/drawer.test.tsxweb/src/__tests__/components/layout/Sidebar.test.tsxweb/src/components/ui/drawer.tsx
web/src/components/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Create new shared components in
web/src/components/ui/with.stories.tsxStorybook file covering all states (default, hover, loading, error, empty)
Files:
web/src/components/ui/drawer.stories.tsxweb/src/components/ui/drawer.tsx
web/src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
web/src/components/**/*.{ts,tsx}: Export props as TypeScript interfaces for all new web components
Use design tokens exclusively in web components; no hardcoded colors, fonts, or spacing
Importcnfrom@/lib/utilsfor conditional class merging in web components
Files:
web/src/components/ui/drawer.stories.tsxweb/src/components/layout/Sidebar.tsxweb/src/components/ui/drawer.tsx
web/src/**/*.stories.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Storybook 10: import from
storybook/test(not@storybook/test),storybook/actions(not@storybook/addon-actions)
Files:
web/src/components/ui/drawer.stories.tsx
web/src/__tests__/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use property-based testing with fast-check in React tests (
fc.assert+fc.property)
Files:
web/src/__tests__/components/ui/drawer.test.tsxweb/src/__tests__/components/layout/Sidebar.test.tsx
🧠 Learnings (13)
📚 Learning: 2026-03-30T10:18:08.053Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:18:08.053Z
Learning: Applies to web/src/components/ui/**/*.{ts,tsx} : Create new shared components in `web/src/components/ui/` with `.stories.tsx` Storybook file covering all states (default, hover, loading, error, empty)
Applied to files:
web/src/components/ui/drawer.stories.tsxweb/src/__tests__/components/layout/Sidebar.test.tsx
📚 Learning: 2026-03-30T10:18:08.053Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:18:08.053Z
Learning: Applies to web/src/**/*.stories.tsx : Storybook 10: import from `storybook/test` (not `storybook/test`), `storybook/actions` (not `storybook/addon-actions`)
Applied to files:
web/src/components/ui/drawer.stories.tsxweb/src/__tests__/components/ui/drawer.test.tsxweb/src/__tests__/components/layout/Sidebar.test.tsx
📚 Learning: 2026-03-30T10:18:08.053Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:18:08.053Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use `@/components/ui/animated-presence` (Framer Motion AnimatePresence keyed by route) for page transitions
Applied to files:
CLAUDE.mddocs/design/brand-and-ux.mdweb/src/__tests__/components/ui/drawer.test.tsxweb/src/__tests__/components/layout/Sidebar.test.tsx
📚 Learning: 2026-03-15T21:20:09.993Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:20:09.993Z
Learning: Applies to web/src/components/** : Vue components organized by feature (agents/, approvals/, budget/, common/, dashboard/, layout/, messages/, org-chart/, tasks/).
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-30T10:18:08.053Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:18:08.053Z
Learning: Update the relevant `docs/design/` page to reflect new reality when approved deviations from spec occur
Applied to files:
docs/design/ux-guidelines.mddocs/design/page-structure.md
📚 Learning: 2026-03-18T08:23:08.912Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T08:23:08.912Z
Learning: When approved deviations occur, update the relevant `docs/design/` page to reflect the new reality.
Applied to files:
docs/design/ux-guidelines.md
📚 Learning: 2026-03-30T10:18:08.053Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:18:08.053Z
Learning: Applies to web/src/**/*.{ts,tsx} : Do NOT build card-with-header layouts from scratch; use `<SectionCard>`
Applied to files:
docs/design/brand-and-ux.md
📚 Learning: 2026-03-30T10:18:08.053Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:18:08.053Z
Learning: Applies to web/src/**/*.{ts,tsx} : ALWAYS reuse existing components from `web/src/components/ui/` before creating new ones; refer to design system inventory (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, etc.)
Applied to files:
docs/design/brand-and-ux.md
📚 Learning: 2026-03-30T10:18:08.053Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:18:08.053Z
Learning: Applies to web/src/**/*.{ts,tsx} : Do NOT create complex (>8 line) JSX inside `.map()`; extract to a shared component
Applied to files:
docs/design/brand-and-ux.md
📚 Learning: 2026-03-30T10:18:08.053Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:18:08.053Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use token variables for shadows/borders (`var(--so-shadow-card-hover)`, `border-border`, `border-bright`); no hardcoded values
Applied to files:
web/src/components/layout/Sidebar.tsxweb/src/components/ui/drawer.tsx
📚 Learning: 2026-03-30T10:18:08.053Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:18:08.053Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use Zustand stores for state management in the web dashboard; each domain has its own store module (auth, WebSocket, toast, analytics, setup, company, agents, budget, tasks, settings, providers, theme, per-domain stores)
Applied to files:
web/src/components/layout/Sidebar.tsx
📚 Learning: 2026-03-30T10:18:08.053Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:18:08.053Z
Learning: Applies to web/src/__tests__/**/*.test.{ts,tsx} : Use property-based testing with fast-check in React tests (`fc.assert` + `fc.property`)
Applied to files:
web/src/__tests__/components/layout/Sidebar.test.tsx
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to web/src/__tests__/**/*.{ts,js} : Dashboard testing: Vitest unit tests organized by feature under `web/src/__tests__/`. Use fast-check for property-based testing (`fc.assert` + `fc.property`).
Applied to files:
web/src/__tests__/components/layout/Sidebar.test.tsx
🔇 Additional comments (5)
docs/design/page-structure.md (1)
311-311: Spec sync is clear and accurate.Line 311 correctly records the new shared
Drawerbehavior and accessibility semantics for tablet overlay mode.Based on learnings: When approved deviations occur, update the relevant
docs/design/page to reflect the new reality.docs/design/ux-guidelines.md (1)
573-573: Tablet interaction guidance now matches implementation.Line 573 documents the Drawer-based overlay behavior and dismissal rules clearly.
Based on learnings: When approved deviations occur, update the relevant
docs/design/page to reflect the new reality.CLAUDE.md (1)
264-264: Drawer inventory update is aligned with the refactor.The new prop/behavior description is precise and useful for downstream contributors.
web/src/components/ui/drawer.stories.tsx (1)
61-99: Good Storybook coverage for the expanded Drawer API.
LeftSideandHeaderlessstories validate the two new behavioral surfaces and improve regression visibility.web/src/__tests__/components/ui/drawer.test.tsx (1)
16-22: Coverage additions are strong and focused.The updated mock plus new assertions for
side, headerless mode, accessible naming, and content-class merging materially improve confidence in Drawer behavior.Also applies to: 86-130
| className={cn( | ||
| 'fixed inset-y-0 right-0 z-50 flex w-[40vw] min-w-80 max-w-xl flex-col', | ||
| 'border-l border-border bg-card shadow-[var(--so-shadow-card-hover)]', | ||
| 'fixed inset-y-0 z-50 flex w-[40vw] min-w-80 max-w-xl flex-col', |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Move the drawer width off the arbitrary viewport value.
w-[40vw] hardcodes sizing inside a shared UI component. Please replace it with a semantic class/token or another standard utility so Drawer width stays aligned with the design system.
As per coding guidelines, "Use design tokens exclusively in web components; no hardcoded colors, fonts, or spacing" and "Use density-aware spacing tokens (p-card, gap-section-gap, gap-grid-gap) or standard Tailwind spacing; NEVER hardcode pixel values for layout spacing".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/ui/drawer.tsx` at line 132, The drawer currently uses an
arbitrary viewport width token 'w-[40vw]' inside the class list ('fixed
inset-y-0 z-50 flex w-[40vw] min-w-80 max-w-xl flex-col'); replace that
hardcoded vw value with a semantic design-token or standard Tailwind width
utility so the component follows the design system (for example, use a named
token like 'w-card' or a standard width class such as 'w-80'/'w-96' or the
established project token for card/drawer width) and keep the existing min/max
constraints ('min-w-80' and 'max-w-xl') to preserve responsiveness. Ensure you
update the class string in the Drawer component (the element with the 'fixed
inset-y-0 z-50 flex ...' classes) and run the UI token mapping so it aligns with
the design tokens used across the app.
…ntentClassName props Add four backward-compatible props to the shared Drawer component to support both left-side and right-side panels, headerless mode, explicit aria-label override, and custom content padding. No breaking changes for existing consumers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…onent Replace ~40 lines of bespoke overlay code (manual backdrop, FocusScope, Escape handler) with the shared Drawer component using side="left". Gains spring animation, focus restore, aria-modal, and consistent backdrop styling. Add tablet overlay test coverage (6 tests). Closes #918 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use discriminated union type for DrawerProps to enforce accessible name (at least one of title or ariaLabel must be provided, WCAG 4.1.2) - Lift NOOP fallback to module scope to avoid unstable function identity - Add tablet overlay tests: Escape key, backdrop click, close-on-navigate - Add ariaLabel precedence test when both title and ariaLabel provided - Tighten close button assertion to toHaveBeenCalledOnce() - Update Drawer description in CLAUDE.md, brand-and-ux.md, ux-guidelines.md Pre-reviewed by 5 agents, 10 findings addressed Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fix 15 review items from local agents (docs-consistency, frontend-reviewer, test-quality, type-design, comment-analyzer, security) and CodeRabbit: - Fix width bug: override Drawer min-w-80 with min-w-60 max-w-60 for 240px sidebar - Add flex flex-col to content wrapper for proper footer push-to-bottom - Import springDefault from shared motion layer instead of hardcoding values - Increase close button touch target to 32x32 with focus-visible styles - Add dev-mode warnings for missing onOverlayClose and empty aria-label - Fix WCAG reference (4.1.2 -> WAI-ARIA dialog pattern) and JSDoc wording - Add [contenteditable] to focus trap selector for rich-text children - Update page-structure.md tablet sidebar description to reference Drawer - Update CLAUDE.md Drawer entry to document ariaLabel prop - Add data-testid on overlay/content for resilient test queries - Improve motion.div mock to strip framer-motion props via Object.entries - Add mobile breakpoint and desktop-sm forced collapse tests - Replace fragile previousElementSibling/parentElement test assertions Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Round 2 review fixes: - Use || instead of ?? for aria-label resolution so empty strings fall through - Extract accessibleName variable for clarity - Fix focus trap to prevent Tab escape when no focusable children exist - Document accessible-name constraint in brand-and-ux.md Drawer entry - Add fast-check property test for close-on-navigate invariant Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
f778c32 to
a3f17da
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/src/components/layout/Sidebar.tsx (1)
1-1:⚠️ Potential issue | 🟠 MajorDon’t fire
onOverlayCloseon the first render.This effect also runs on mount. If
Sidebaris rendered withoverlayOpen={true}, it closes immediately before any navigation occurs, which is why the new tablet tests have to clear the callback right aftersetupTablet(true). Compare against the previous location before closing.🐛 Suggested fix
-import { useEffect, useState } from 'react' +import { useEffect, useRef, useState } from 'react' @@ + const previousPathnameRef = useRef(location.pathname) + // Close overlay on navigation useEffect(() => { - if (overlayOpen && onOverlayClose) { + if (overlayOpen && onOverlayClose && previousPathnameRef.current !== location.pathname) { onOverlayClose() } + previousPathnameRef.current = location.pathname // Only trigger on route changes, not on prop changes // eslint-disable-next-line `@eslint-react/exhaustive-deps` }, [location.pathname])Also applies to: 83-90
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/layout/Sidebar.tsx` at line 1, The effect in Sidebar that calls onOverlayClose runs on mount and closes an overlay immediately when overlayOpen starts true; update the useEffect that watches overlayOpen/location to skip the first render and only call onOverlayClose when the location actually changes (i.e., navigation occurred). Specifically, in the Sidebar component's useEffect (the one referencing overlayOpen and onOverlayClose, also around lines 83-90), add a mounted/ref guard (or track previousLocation via useRef with useLocation) so you compare the previous location to the current one and only invoke onOverlayClose when overlayOpen is true and the location differs from the previous value, not on initial mount.web/src/__tests__/components/ui/drawer.test.tsx (1)
10-25:⚠️ Potential issue | 🟠 MajorWrap the
motion.divmock inforwardRefto preserve ref forwarding.The Drawer component uses
panelReffor focus management (lines 72–111 of drawer.tsx): it sets initial focus, traps Tab cycling, and restores focus on close. The current mock is a plain function component, so the ref passed tomotion.divwill not reach the backing<div>, breaking these focus behaviors in tests. Wrap the function inforwardRef:Example fix
motion: { div: forwardRef(({ children, ...allProps }, ref) => { const domProps = Object.fromEntries( Object.entries(allProps).filter(([key]) => !['variants', 'initial', 'animate', 'exit', 'transition'].includes(key)), ) return <div ref={ref} {...domProps}>{children}</div> }), }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/__tests__/components/ui/drawer.test.tsx` around lines 10 - 25, The motion.div mock used in the vi.mock for 'framer-motion' doesn't forward refs, breaking Drawer focus behavior that relies on panelRef; wrap the mock component in React.forwardRef so the ref passed to motion.div is attached to the underlying <div>. Update the mocked export under motion.div to accept (props, ref), filter out framer-motion-specific props as before, and pass ref to the returned div; keep MockAnimatePresence unchanged. This preserves ref forwarding for Drawer (panelRef) and restores focus management in tests.
♻️ Duplicate comments (3)
web/src/components/layout/Sidebar.tsx (1)
57-61: 🛠️ Refactor suggestion | 🟠 MajorMake the tablet overlay contract enforceable.
The dev-only warning still allows
overlayOpen={true}without a real close handler, and the() => {}fallback makes Escape/backdrop dismissal silently inert. Encode that pairing inSidebarPropsso this cannot compile.♻️ Example shape
type SidebarProps = | { overlayOpen?: false; onOverlayClose?: () => void } | { overlayOpen: boolean; onOverlayClose: () => void }Also applies to: 77-81, 125-137
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/layout/Sidebar.tsx` around lines 57 - 61, The SidebarProps type must encode the overlayOpen/onOverlayClose pairing so you can't pass overlayOpen={true} without a real handler: change the interface SidebarProps into a discriminated union (e.g., one branch where overlayOpen is false/optional and onOverlayClose is optional, and another branch where overlayOpen is true and onOverlayClose is required) and remove the internal noop fallback used where onOverlayClose was defaulted; update any places using the Sidebar component to satisfy the new type (or pass overlayOpen={false}) and adjust the component logic to assume onOverlayClose exists when overlayOpen is true.web/src/components/ui/drawer.tsx (2)
49-56: 🧹 Nitpick | 🔵 TrivialWhitespace-only strings can still produce an unnamed dialog.
The
||operator handles empty strings but not whitespace-only values like" ". Adding.trim()would close this edge case.- const accessibleName = ariaLabel || title || undefined + const accessibleName = ariaLabel?.trim() || title?.trim() || undefined🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/ui/drawer.tsx` around lines 49 - 56, accessibleName currently uses ariaLabel || title || undefined which treats whitespace-only strings as truthy; update the logic to trim both ariaLabel and title before checking so whitespace-only strings are treated as empty. Specifically, compute accessibleName from ariaLabel?.trim() and title?.trim() (e.g., const accessibleName = ariaLabel?.trim() || title?.trim() || undefined) and use that trimmed value in the useEffect warning so the console warns for whitespace-only values too; update references to accessibleName, ariaLabel, title, and the useEffect dependency accordingly.
140-145: 🛠️ Refactor suggestion | 🟠 MajorArbitrary viewport width violates design system guidelines.
w-[40vw]hardcodes sizing inside a shared UI component. Themin-w-80andmax-w-xlconstraints are appropriate, but the base width should use a semantic token or standard Tailwind utility.Consider using a standard width like
w-96or introducing a design token if40vwis intentional behavior:- 'fixed inset-y-0 z-50 flex w-[40vw] min-w-80 max-w-xl flex-col', + 'fixed inset-y-0 z-50 flex w-96 min-w-80 max-w-xl flex-col',As per coding guidelines, "Use design tokens exclusively -- no hardcoded colors, fonts, or spacing" and spacing tokens should use density-aware or standard Tailwind values.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/ui/drawer.tsx` around lines 140 - 145, The drawer component's hardcoded responsive width token w-[40vw] inside the cn(...) class list violates the design system; replace it with a semantic/standard token (e.g. w-96) or wire it to a design-token/CSS variable (e.g. --drawer-width) and fall back to the existing min-w-80 / max-w-xl constraints, and optionally expose a width prop on the Drawer component to control it; update the className expression that currently contains 'w-[40vw]' (the cn(...) call in web/src/components/ui/drawer.tsx) to use the chosen semantic utility or CSS variable and ensure any docs/tests reference the new prop/token if added.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@web/src/__tests__/components/ui/drawer.test.tsx`:
- Around line 10-25: The motion.div mock used in the vi.mock for 'framer-motion'
doesn't forward refs, breaking Drawer focus behavior that relies on panelRef;
wrap the mock component in React.forwardRef so the ref passed to motion.div is
attached to the underlying <div>. Update the mocked export under motion.div to
accept (props, ref), filter out framer-motion-specific props as before, and pass
ref to the returned div; keep MockAnimatePresence unchanged. This preserves ref
forwarding for Drawer (panelRef) and restores focus management in tests.
In `@web/src/components/layout/Sidebar.tsx`:
- Line 1: The effect in Sidebar that calls onOverlayClose runs on mount and
closes an overlay immediately when overlayOpen starts true; update the useEffect
that watches overlayOpen/location to skip the first render and only call
onOverlayClose when the location actually changes (i.e., navigation occurred).
Specifically, in the Sidebar component's useEffect (the one referencing
overlayOpen and onOverlayClose, also around lines 83-90), add a mounted/ref
guard (or track previousLocation via useRef with useLocation) so you compare the
previous location to the current one and only invoke onOverlayClose when
overlayOpen is true and the location differs from the previous value, not on
initial mount.
---
Duplicate comments:
In `@web/src/components/layout/Sidebar.tsx`:
- Around line 57-61: The SidebarProps type must encode the
overlayOpen/onOverlayClose pairing so you can't pass overlayOpen={true} without
a real handler: change the interface SidebarProps into a discriminated union
(e.g., one branch where overlayOpen is false/optional and onOverlayClose is
optional, and another branch where overlayOpen is true and onOverlayClose is
required) and remove the internal noop fallback used where onOverlayClose was
defaulted; update any places using the Sidebar component to satisfy the new type
(or pass overlayOpen={false}) and adjust the component logic to assume
onOverlayClose exists when overlayOpen is true.
In `@web/src/components/ui/drawer.tsx`:
- Around line 49-56: accessibleName currently uses ariaLabel || title ||
undefined which treats whitespace-only strings as truthy; update the logic to
trim both ariaLabel and title before checking so whitespace-only strings are
treated as empty. Specifically, compute accessibleName from ariaLabel?.trim()
and title?.trim() (e.g., const accessibleName = ariaLabel?.trim() ||
title?.trim() || undefined) and use that trimmed value in the useEffect warning
so the console warns for whitespace-only values too; update references to
accessibleName, ariaLabel, title, and the useEffect dependency accordingly.
- Around line 140-145: The drawer component's hardcoded responsive width token
w-[40vw] inside the cn(...) class list violates the design system; replace it
with a semantic/standard token (e.g. w-96) or wire it to a design-token/CSS
variable (e.g. --drawer-width) and fall back to the existing min-w-80 / max-w-xl
constraints, and optionally expose a width prop on the Drawer component to
control it; update the className expression that currently contains 'w-[40vw]'
(the cn(...) call in web/src/components/ui/drawer.tsx) to use the chosen
semantic utility or CSS variable and ensure any docs/tests reference the new
prop/token if added.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d44ff1c6-0ac4-49d3-a28c-956fb428bfda
📒 Files selected for processing (9)
CLAUDE.mddocs/design/brand-and-ux.mddocs/design/page-structure.mddocs/design/ux-guidelines.mdweb/src/__tests__/components/layout/Sidebar.test.tsxweb/src/__tests__/components/ui/drawer.test.tsxweb/src/components/layout/Sidebar.tsxweb/src/components/ui/drawer.stories.tsxweb/src/components/ui/drawer.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Dashboard Test
- GitHub Check: Build Sandbox
- GitHub Check: Build Backend
- GitHub Check: Build Web
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (7)
docs/design/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
When approved deviations occur, update the relevant
docs/design/page to reflect the new reality.
Files:
docs/design/page-structure.mddocs/design/ux-guidelines.mddocs/design/brand-and-ux.md
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Docs built with Zensical (config:
mkdocs.yml). Design spec indocs/design/(11 pages), Architecture indocs/architecture/, Roadmap indocs/roadmap/, Security indocs/security.md, Licensing indocs/licensing.md, API reference indocs/rest-api.mdand auto-generateddocs/_generated/api-reference.html.
Files:
docs/design/page-structure.mddocs/design/ux-guidelines.mddocs/design/brand-and-ux.md
web/src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
web/src/**/*.{tsx,ts}: ALWAYS reuse existing components fromweb/src/components/ui/before creating new ones. These are the shared building blocks that every page composes from.
Design tokens: use Tailwind semantic classes (text-foreground,bg-card,text-accent,text-success,bg-danger) or CSS variables (var(--so-accent)). NEVER hardcode hex values in.tsx/.tsfiles.
Typography tokens: usefont-sansorfont-mono(maps to Geist tokens). NEVER setfontFamilydirectly in.tsx/.ts.
Spacing tokens: use density-aware tokens (p-card,gap-section-gap,gap-grid-gap) or standard Tailwind spacing. NEVER hardcode pixel values for layout spacing.
Shadows and borders: use token variables (var(--so-shadow-card-hover),border-border,border-bright). NEVER hardcode shadow or border values.
ESLint enforces zero warnings on web dashboard code. Usenpm --prefix web run lintto check and fix linting issues.
A PostToolUse hook (scripts/check_web_design_system.py) runs automatically on every Edit/Write toweb/src/files. It catches hardcoded colors/rgba, hardcoded font-family, missing Storybook stories, duplicate patterns, and complex.map()blocks. Fix all violations before proceeding.
Files:
web/src/__tests__/components/ui/drawer.test.tsxweb/src/components/ui/drawer.stories.tsxweb/src/components/layout/Sidebar.tsxweb/src/__tests__/components/layout/Sidebar.test.tsxweb/src/components/ui/drawer.tsx
web/src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
web/src/**/*.tsx: Do NOT recreate status dots inline -- use<StatusBadge>. Do NOT build card-with-header layouts from scratch -- use<SectionCard>. Do NOT create metric displays with inline text -- use<MetricCard>. Do NOT render initials circles manually -- use<Avatar>. Do NOT create complex (>8 line) JSX inside.map()-- extract to a shared component.
Web dashboard: importcnfrom@/lib/utilsfor conditional class merging. Never use bare string concatenation for Tailwind classes.
Files:
web/src/__tests__/components/ui/drawer.test.tsxweb/src/components/ui/drawer.stories.tsxweb/src/components/layout/Sidebar.tsxweb/src/__tests__/components/layout/Sidebar.test.tsxweb/src/components/ui/drawer.tsx
web/src/components/ui/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
When creating new shared components: place in
web/src/components/ui/with kebab-case filename. Create a.stories.tsxfile alongside with all states (default, hover, loading, error, empty). Export props as a TypeScript interface. Use design tokens exclusively -- no hardcoded colors, fonts, or spacing. Importcnfrom@/lib/utilsfor conditional class merging.
Files:
web/src/components/ui/drawer.stories.tsxweb/src/components/ui/drawer.tsx
web/src/components/ui/**/*.stories.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
React component Storybook stories required for all new shared components in
web/src/components/ui/. Stories must demonstrate all component states (default, hover, loading, error, empty).
Files:
web/src/components/ui/drawer.stories.tsx
web/src/**/*.stories.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
web/src/**/*.stories.{tsx,ts}: Storybook 10 is ESM-only -- all CJS support removed. Use ESM import/export syntax in all Storybook files.
Storybook 10: import paths changed -- usestorybook/test(not@storybook/test),storybook/actions(not@storybook/addon-actions).
Files:
web/src/components/ui/drawer.stories.tsx
🧠 Learnings (15)
📚 Learning: 2026-03-30T10:40:15.387Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:40:15.387Z
Learning: Applies to docs/design/**/*.md : When approved deviations occur, update the relevant `docs/design/` page to reflect the new reality.
Applied to files:
docs/design/page-structure.mddocs/design/ux-guidelines.mddocs/design/brand-and-ux.md
📚 Learning: 2026-03-18T08:23:08.912Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T08:23:08.912Z
Learning: When approved deviations occur, update the relevant `docs/design/` page to reflect the new reality.
Applied to files:
docs/design/ux-guidelines.md
📚 Learning: 2026-03-30T10:40:15.387Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:40:15.387Z
Learning: Applies to web/src/**/*.tsx : Do NOT recreate status dots inline -- use `<StatusBadge>`. Do NOT build card-with-header layouts from scratch -- use `<SectionCard>`. Do NOT create metric displays with inline text -- use `<MetricCard>`. Do NOT render initials circles manually -- use `<Avatar>`. Do NOT create complex (>8 line) JSX inside `.map()` -- extract to a shared component.
Applied to files:
CLAUDE.mddocs/design/brand-and-ux.mdweb/src/components/layout/Sidebar.tsxweb/src/components/ui/drawer.tsx
📚 Learning: 2026-03-15T21:20:09.993Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:20:09.993Z
Learning: Applies to web/src/components/** : Vue components organized by feature (agents/, approvals/, budget/, common/, dashboard/, layout/, messages/, org-chart/, tasks/).
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-30T10:40:15.387Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:40:15.387Z
Learning: Applies to web/src/**/*.stories.{tsx,ts} : Storybook 10: import paths changed -- use `storybook/test` (not `storybook/test`), `storybook/actions` (not `storybook/addon-actions`).
Applied to files:
web/src/__tests__/components/ui/drawer.test.tsxweb/src/components/ui/drawer.stories.tsxweb/src/__tests__/components/layout/Sidebar.test.tsx
📚 Learning: 2026-03-30T10:40:15.387Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:40:15.387Z
Learning: Applies to web/src/**/*.{tsx,ts} : ALWAYS reuse existing components from `web/src/components/ui/` before creating new ones. These are the shared building blocks that every page composes from.
Applied to files:
docs/design/brand-and-ux.mdweb/src/components/ui/drawer.tsx
📚 Learning: 2026-03-30T10:40:15.387Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:40:15.387Z
Learning: Applies to web/src/components/ui/**/*.{tsx,ts} : When creating new shared components: place in `web/src/components/ui/` with kebab-case filename. Create a `.stories.tsx` file alongside with all states (default, hover, loading, error, empty). Export props as a TypeScript interface. Use design tokens exclusively -- no hardcoded colors, fonts, or spacing. Import `cn` from `@/lib/utils` for conditional class merging.
Applied to files:
docs/design/brand-and-ux.mdweb/src/components/ui/drawer.stories.tsxweb/src/__tests__/components/layout/Sidebar.test.tsxweb/src/components/ui/drawer.tsx
📚 Learning: 2026-03-30T10:40:15.387Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:40:15.387Z
Learning: Applies to web/src/components/ui/**/*.stories.tsx : React component Storybook stories required for all new shared components in `web/src/components/ui/`. Stories must demonstrate all component states (default, hover, loading, error, empty).
Applied to files:
web/src/components/ui/drawer.stories.tsx
📚 Learning: 2026-03-30T10:40:15.387Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:40:15.387Z
Learning: Applies to web/src/**/*.stories.{tsx,ts} : Storybook 10 is ESM-only -- all CJS support removed. Use ESM import/export syntax in all Storybook files.
Applied to files:
web/src/components/ui/drawer.stories.tsx
📚 Learning: 2026-03-30T10:40:15.387Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:40:15.387Z
Learning: Applies to web/.storybook/{main,preview}.ts : Storybook 10: use `defineMain` from `storybook/react-vite/node` and `definePreview` from `storybook/react-vite`. Must include explicit `framework` field.
Applied to files:
web/src/components/ui/drawer.stories.tsx
📚 Learning: 2026-03-30T10:40:15.387Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:40:15.387Z
Learning: Applies to web/src/**/*.{tsx,ts} : Shadows and borders: use token variables (`var(--so-shadow-card-hover)`, `border-border`, `border-bright`). NEVER hardcode shadow or border values.
Applied to files:
web/src/components/layout/Sidebar.tsxweb/src/components/ui/drawer.tsx
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to web/src/__tests__/**/*.{ts,js} : Dashboard testing: Vitest unit tests organized by feature under `web/src/__tests__/`. Use fast-check for property-based testing (`fc.assert` + `fc.property`).
Applied to files:
web/src/__tests__/components/layout/Sidebar.test.tsx
📚 Learning: 2026-03-30T10:40:15.387Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:40:15.387Z
Learning: Applies to web/src/**/*.{tsx,ts} : Spacing tokens: use density-aware tokens (`p-card`, `gap-section-gap`, `gap-grid-gap`) or standard Tailwind spacing. NEVER hardcode pixel values for layout spacing.
Applied to files:
web/src/components/ui/drawer.tsx
📚 Learning: 2026-03-30T10:40:15.387Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:40:15.387Z
Learning: Applies to web/src/**/*.{tsx,ts} : Design tokens: use Tailwind semantic classes (`text-foreground`, `bg-card`, `text-accent`, `text-success`, `bg-danger`) or CSS variables (`var(--so-accent)`). NEVER hardcode hex values in `.tsx`/`.ts` files.
Applied to files:
web/src/components/ui/drawer.tsx
📚 Learning: 2026-03-30T10:40:15.387Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:40:15.387Z
Learning: Applies to web/src/**/*.tsx : Web dashboard: import `cn` from `@/lib/utils` for conditional class merging. Never use bare string concatenation for Tailwind classes.
Applied to files:
web/src/components/ui/drawer.tsx
🔇 Additional comments (5)
web/src/components/ui/drawer.tsx (5)
1-6: LGTM on imports.Good use of
useMemofor memoizing panel variants and importingspringDefaultfrom the shared motion module aligns with the motion layer consolidation.
19-28: Clean discriminated union for accessibility enforcement.The union type correctly enforces that either
titleorariaLabelmust be provided at compile time, satisfying WAI-ARIA dialog naming requirements. The JSDoc clearly documents the contract.
35-42: Good adoption of shared motion config.Using
springDefaultfrom the motion module ensures the drawer follows centralized animation tuning.
80-89: Focus trap correctly handles headerless drawers.The logic now prevents Tab from escaping when no focusable children exist by refocusing the panel itself. The expanded selector including
[contenteditable]is a good addition for rich text scenarios.
147-168: Conditional header and content styling look good.The header correctly renders only when
titleis provided, and the close button has proper accessibility attributes. ThecontentClassNamemerge pattern usingcn()allows callers like Sidebar to override default padding (p-0) cleanly.
…r/Sidebar Round 3 CodeRabbit fixes: - Forward ref in motion.div test mock (React 19 ref-as-prop pattern) so Drawer's panelRef works correctly in tests for focus management - Fix close-on-navigate useEffect firing on mount: track previous pathname via useRef and only call onOverlayClose when location actually changes - Remove mockClear() workarounds from tests now that mount-fire is fixed - Add explicit test: overlay does not call onOverlayClose on mount - Trim whitespace in accessibleName resolution so whitespace-only strings are treated as empty for aria-label Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
web/src/components/layout/Sidebar.tsx (1)
57-61:⚠️ Potential issue | 🟠 MajorRequire
onOverlayClosewhenever overlay state is controlled.Lines 57-61 still allow
overlayOpen={true}withoutonOverlayClose. In that configuration, Line 130 turns Escape/backdrop dismiss into a noop and Line 140 can render a visible close button with no handler, so the tablet drawer becomes non-dismissible in production.♻️ Suggested shape
-interface SidebarProps { - /** Whether the overlay sidebar is visible (used at tablet breakpoints). */ - overlayOpen?: boolean - /** Called when the overlay requests close. Required when overlayOpen is used. */ - onOverlayClose?: () => void -} +type SidebarProps = + | { + /** Whether the overlay sidebar is visible (used at tablet breakpoints). */ + overlayOpen?: false + onOverlayClose?: () => void + } + | { + /** Whether the overlay sidebar is visible (used at tablet breakpoints). */ + overlayOpen: boolean + onOverlayClose: () => void + } export function Sidebar({ overlayOpen = false, onOverlayClose }: SidebarProps) { + const handleOverlayClose = onOverlayClose ?? (() => {}) // ... <Drawer open={overlayOpen} - onClose={onOverlayClose ?? (() => {})} + onClose={handleOverlayClose} side="left" ariaLabel="Navigation menu" > <div className="flex h-14 shrink-0 items-center justify-between border-b border-border px-3"> <button type="button" - onClick={onOverlayClose} + onClick={handleOverlayClose}Also applies to: 128-140
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/layout/Sidebar.tsx` around lines 57 - 61, Make onOverlayClose required whenever overlayOpen is used by changing the SidebarProps shape to a discriminated/unioned type so controlled-overlay consumers must supply the handler; e.g. replace the flat props with a union like { overlayOpen?: false; onOverlayClose?: never } | { overlayOpen: true; onOverlayClose: () => void } (or equivalent) and update the Sidebar component signature (SidebarProps) so usages of overlayOpen handlers (the Escape/backdrop dismiss logic and the close button rendering that reference onOverlayClose) can safely call onOverlayClose without a noop or missing handler.web/src/components/ui/drawer.tsx (1)
140-144: 🛠️ Refactor suggestion | 🟠 MajorDrop the arbitrary default width from the shared drawer.
Line 141 hardcodes
w-[40vw]in a reusable UI primitive. That forces consumers likeSidebarto fight the base class with overrides and bypasses the design-system constraint on shared components. Prefer a token or standard Tailwind width here and leave one-off sizing toclassName.As per coding guidelines, "Use design tokens exclusively -- no hardcoded colors, fonts, or spacing" and "Spacing tokens: use density-aware tokens (
p-card,gap-section-gap,gap-grid-gap) or standard Tailwind spacing."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/ui/drawer.tsx` around lines 140 - 144, The shared Drawer component currently hardcodes a width via the utility w-[40vw] inside the className construction (see the cn(...) call in the Drawer component), which forces consumers to override sizing; remove that arbitrary w-[40vw] and replace it with a design-token or neutral utility (e.g., a density-aware token such as a component width token or a standard Tailwind utility like w-full or omit width altogether) so sizing is left to the consumer via the className prop; update the className inside the cn(...) call used by the Drawer component (and keep existing max-w-xl/min-w-80) so one-off widths are applied by the Sidebar or other consumers rather than baked into the shared Drawer.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/__tests__/components/ui/drawer.test.tsx`:
- Around line 86-130: Add a new test in
web/src/__tests__/components/ui/drawer.test.tsx that verifies focus is restored
to the opener when the Drawer closes: render a button (e.g.,
data-testid="opener") that opens <Drawer open={true} ... /> via userEvent.click,
ensure initial focus moves into the Drawer (e.g., screen.getByRole('dialog') or
panelRef target), then trigger close (call onClose via clicking the Close button
or simulating Escape) and assert document.activeElement is the opener element;
use testing-library methods like screen.getByTestId('opener'), userEvent.click,
and expect(document.activeElement).toBe(opener) to lock the focus-restore
regression coverage for the Drawer component.
In `@web/src/components/ui/drawer.tsx`:
- Around line 49-56: Create a single trimmed-title variable (e.g., const
trimmedTitle = title?.trim()) and reuse it in the accessible-name computation
(accessibleName = ariaLabel?.trim() || trimmedTitle || undefined) and in the
header render conditional so the header is not rendered for whitespace-only
titles; update any checks that currently use title?.trim() or title directly to
use trimmedTitle and ensure the same logic applies in production and dev
branches (references: accessibleName, title, ariaLabel, and the header rendering
JSX).
---
Duplicate comments:
In `@web/src/components/layout/Sidebar.tsx`:
- Around line 57-61: Make onOverlayClose required whenever overlayOpen is used
by changing the SidebarProps shape to a discriminated/unioned type so
controlled-overlay consumers must supply the handler; e.g. replace the flat
props with a union like { overlayOpen?: false; onOverlayClose?: never } | {
overlayOpen: true; onOverlayClose: () => void } (or equivalent) and update the
Sidebar component signature (SidebarProps) so usages of overlayOpen handlers
(the Escape/backdrop dismiss logic and the close button rendering that reference
onOverlayClose) can safely call onOverlayClose without a noop or missing
handler.
In `@web/src/components/ui/drawer.tsx`:
- Around line 140-144: The shared Drawer component currently hardcodes a width
via the utility w-[40vw] inside the className construction (see the cn(...) call
in the Drawer component), which forces consumers to override sizing; remove that
arbitrary w-[40vw] and replace it with a design-token or neutral utility (e.g.,
a density-aware token such as a component width token or a standard Tailwind
utility like w-full or omit width altogether) so sizing is left to the consumer
via the className prop; update the className inside the cn(...) call used by the
Drawer component (and keep existing max-w-xl/min-w-80) so one-off widths are
applied by the Sidebar or other consumers rather than baked into the shared
Drawer.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6128ca82-781d-42d1-864f-4f952ad9148a
📒 Files selected for processing (4)
web/src/__tests__/components/layout/Sidebar.test.tsxweb/src/__tests__/components/ui/drawer.test.tsxweb/src/components/layout/Sidebar.tsxweb/src/components/ui/drawer.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (go)
🧰 Additional context used
📓 Path-based instructions (3)
web/src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
web/src/**/*.{tsx,ts}: ALWAYS reuse existing components fromweb/src/components/ui/before creating new ones. These are the shared building blocks that every page composes from.
Design tokens: use Tailwind semantic classes (text-foreground,bg-card,text-accent,text-success,bg-danger) or CSS variables (var(--so-accent)). NEVER hardcode hex values in.tsx/.tsfiles.
Typography tokens: usefont-sansorfont-mono(maps to Geist tokens). NEVER setfontFamilydirectly in.tsx/.ts.
Spacing tokens: use density-aware tokens (p-card,gap-section-gap,gap-grid-gap) or standard Tailwind spacing. NEVER hardcode pixel values for layout spacing.
Shadows and borders: use token variables (var(--so-shadow-card-hover),border-border,border-bright). NEVER hardcode shadow or border values.
ESLint enforces zero warnings on web dashboard code. Usenpm --prefix web run lintto check and fix linting issues.
A PostToolUse hook (scripts/check_web_design_system.py) runs automatically on every Edit/Write toweb/src/files. It catches hardcoded colors/rgba, hardcoded font-family, missing Storybook stories, duplicate patterns, and complex.map()blocks. Fix all violations before proceeding.
Files:
web/src/__tests__/components/ui/drawer.test.tsxweb/src/components/layout/Sidebar.tsxweb/src/components/ui/drawer.tsxweb/src/__tests__/components/layout/Sidebar.test.tsx
web/src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
web/src/**/*.tsx: Do NOT recreate status dots inline -- use<StatusBadge>. Do NOT build card-with-header layouts from scratch -- use<SectionCard>. Do NOT create metric displays with inline text -- use<MetricCard>. Do NOT render initials circles manually -- use<Avatar>. Do NOT create complex (>8 line) JSX inside.map()-- extract to a shared component.
Web dashboard: importcnfrom@/lib/utilsfor conditional class merging. Never use bare string concatenation for Tailwind classes.
Files:
web/src/__tests__/components/ui/drawer.test.tsxweb/src/components/layout/Sidebar.tsxweb/src/components/ui/drawer.tsxweb/src/__tests__/components/layout/Sidebar.test.tsx
web/src/components/ui/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
When creating new shared components: place in
web/src/components/ui/with kebab-case filename. Create a.stories.tsxfile alongside with all states (default, hover, loading, error, empty). Export props as a TypeScript interface. Use design tokens exclusively -- no hardcoded colors, fonts, or spacing. Importcnfrom@/lib/utilsfor conditional class merging.
Files:
web/src/components/ui/drawer.tsx
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:40:15.387Z
Learning: Applies to web/src/**/*.stories.{tsx,ts} : Storybook 10: import paths changed -- use `storybook/test` (not `storybook/test`), `storybook/actions` (not `storybook/addon-actions`).
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:40:15.387Z
Learning: Applies to web/.storybook/preview.ts : Storybook 10 a11y testing: use `parameters.a11y.test: 'error' | 'todo' | 'off'` (replaces old `.element` and `.manual`). Set globally in `preview.tsx` to enforce WCAG compliance on all stories.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:40:15.387Z
Learning: Applies to web/src/components/ui/**/*.{tsx,ts} : When creating new shared components: place in `web/src/components/ui/` with kebab-case filename. Create a `.stories.tsx` file alongside with all states (default, hover, loading, error, empty). Export props as a TypeScript interface. Use design tokens exclusively -- no hardcoded colors, fonts, or spacing. Import `cn` from `@/lib/utils` for conditional class merging.
📚 Learning: 2026-03-30T10:40:15.387Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:40:15.387Z
Learning: Applies to web/src/**/*.stories.{tsx,ts} : Storybook 10: import paths changed -- use `storybook/test` (not `storybook/test`), `storybook/actions` (not `storybook/addon-actions`).
Applied to files:
web/src/__tests__/components/ui/drawer.test.tsxweb/src/__tests__/components/layout/Sidebar.test.tsx
📚 Learning: 2026-03-30T10:40:15.387Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:40:15.387Z
Learning: Applies to web/src/**/*.{tsx,ts} : Shadows and borders: use token variables (`var(--so-shadow-card-hover)`, `border-border`, `border-bright`). NEVER hardcode shadow or border values.
Applied to files:
web/src/components/layout/Sidebar.tsxweb/src/components/ui/drawer.tsx
📚 Learning: 2026-03-30T10:40:15.387Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:40:15.387Z
Learning: Applies to web/src/**/*.tsx : Do NOT recreate status dots inline -- use `<StatusBadge>`. Do NOT build card-with-header layouts from scratch -- use `<SectionCard>`. Do NOT create metric displays with inline text -- use `<MetricCard>`. Do NOT render initials circles manually -- use `<Avatar>`. Do NOT create complex (>8 line) JSX inside `.map()` -- extract to a shared component.
Applied to files:
web/src/components/layout/Sidebar.tsxweb/src/components/ui/drawer.tsx
📚 Learning: 2026-03-30T10:40:15.387Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:40:15.387Z
Learning: Applies to web/src/components/ui/**/*.{tsx,ts} : When creating new shared components: place in `web/src/components/ui/` with kebab-case filename. Create a `.stories.tsx` file alongside with all states (default, hover, loading, error, empty). Export props as a TypeScript interface. Use design tokens exclusively -- no hardcoded colors, fonts, or spacing. Import `cn` from `@/lib/utils` for conditional class merging.
Applied to files:
web/src/components/ui/drawer.tsx
📚 Learning: 2026-03-30T10:40:15.387Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:40:15.387Z
Learning: Applies to web/src/**/*.{tsx,ts} : Spacing tokens: use density-aware tokens (`p-card`, `gap-section-gap`, `gap-grid-gap`) or standard Tailwind spacing. NEVER hardcode pixel values for layout spacing.
Applied to files:
web/src/components/ui/drawer.tsx
📚 Learning: 2026-03-30T10:40:15.387Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:40:15.387Z
Learning: Applies to web/src/**/*.{tsx,ts} : Design tokens: use Tailwind semantic classes (`text-foreground`, `bg-card`, `text-accent`, `text-success`, `bg-danger`) or CSS variables (`var(--so-accent)`). NEVER hardcode hex values in `.tsx`/`.ts` files.
Applied to files:
web/src/components/ui/drawer.tsx
📚 Learning: 2026-03-30T10:40:15.387Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:40:15.387Z
Learning: Applies to web/src/**/*.tsx : Web dashboard: import `cn` from `@/lib/utils` for conditional class merging. Never use bare string concatenation for Tailwind classes.
Applied to files:
web/src/components/ui/drawer.tsx
📚 Learning: 2026-03-30T10:40:15.387Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:40:15.387Z
Learning: Applies to web/src/**/*.{tsx,ts} : ALWAYS reuse existing components from `web/src/components/ui/` before creating new ones. These are the shared building blocks that every page composes from.
Applied to files:
web/src/components/ui/drawer.tsx
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to web/src/__tests__/**/*.{ts,js} : Dashboard testing: Vitest unit tests organized by feature under `web/src/__tests__/`. Use fast-check for property-based testing (`fc.assert` + `fc.property`).
Applied to files:
web/src/__tests__/components/layout/Sidebar.test.tsx
🔇 Additional comments (2)
web/src/components/layout/Sidebar.tsx (1)
83-93: Nice mount-skip fix for close-on-navigate.Tracking the previous pathname avoids the bogus close on first render while preserving the route-change dismissal behavior.
web/src/__tests__/components/layout/Sidebar.test.tsx (1)
337-349: Nice use of property-based coverage.Driving the route set from
ROUTESmakes future static-route additions inherit the close-on-navigate invariant automatically.
- Extract trimmedTitle variable reused for both accessibleName resolution and header conditional so whitespace-only titles skip header rendering - Add focus-restore regression test: verifies focus returns to the opener element when the Drawer closes Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
🤖 I have created a release *beep* *boop* --- ## [0.5.1](v0.5.0...v0.5.1) (2026-03-30) ### Features * add linear variant to ProgressGauge component ([#927](#927)) ([89bf8d0](89bf8d0)) * frontend security hardening -- ESLint XSS ban + MotionConfig CSP nonce ([#926](#926)) ([6592ed0](6592ed0)) * set up MSW for Storybook API mocking ([#930](#930)) ([214078c](214078c)) ### Refactoring * **web:** replace Sidebar tablet overlay with shared Drawer component ([#928](#928)) ([ad5451d](ad5451d)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Summary
Drawercomponent with 4 backward-compatible props (side, optionaltitle,ariaLabel,contentClassName) to support left-side rendering and headerless modeDrawercomponent usingside="left"DrawerPropsto enforce accessible name at compile time (WCAG 4.1.2)Behavioral improvements from using shared Drawer
aria-modal="true"(was missing)Closes #918
Test plan
npm --prefix web run test-- 2160 tests pass (179 files)npm --prefix web run type-check-- cleannpm --prefix web run lint-- zero warningsnpm --prefix web run dev, resize to tablet width (~768-1024px), verify sidebar overlay opens/closes (backdrop click, Escape, close button, close-on-navigate)Pre-reviewed by 5 agents (docs-consistency, frontend-reviewer, test-quality, issue-resolution-verifier, type-design-analyzer), 10 findings addressed
🤖 Generated with Claude Code