Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe PR adds a comprehensive Tabs component with controlled/uncontrolled modes and animated indicators, introduces a modal overlay color token in the theme, adds an Changes
Sequence DiagramsequenceDiagram
participant Consumer as Consumer Component
participant Tabs as Tabs Provider
participant Context as TabsContext
participant TabsList as TabsList
participant Trigger as TabsTrigger
participant Contents as TabsContents
participant Content as TabsContent
Consumer->>Tabs: Render Tabs with value & onValueChange
activate Tabs
Tabs->>Context: Initialize context (activeValue, resolvedValue)
Tabs->>Trigger: Render first TabsTrigger
activate Trigger
Trigger->>Trigger: On Mount: registerTrigger(value)
Trigger->>Context: Update trigger registry
deactivate Trigger
Consumer->>Trigger: Click TabsTrigger
activate Trigger
Trigger->>Context: setActiveValue(newValue)
Trigger->>Context: Call onValueChange(newValue)
deactivate Trigger
Context->>TabsList: Trigger re-render
TabsList->>TabsList: Update indicator position (requestAnimationFrame)
Context->>Contents: Trigger re-render
Contents->>Contents: Find matching TabsContent by value
Contents->>Content: Render active TabsContent
Content->>Consumer: Display active content
deactivate Tabs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
libs/react/ui/src/components/tabs/tabs.tsx (3)
68-94: Minor duplication in initial tab selection logic.The initial tab selection logic exists in both the
useEffect(lines 68-79) andregisterTrigger(lines 85-88). While theinitialSetref prevents conflicts, you could consider consolidating this to a single location for clarity. The current implementation works correctly though.
159-182: ConsiderResizeObserverfor more robust indicator updates.The current implementation handles window resize but won't detect container-level size changes (e.g., sidebar toggle, parent flex changes). A
ResizeObserveronlistRef.currentwould be more robust. This is a nice-to-have enhancement for edge cases.useEffect(() => { const updateIndicator = () => { const activeTrigger = getTriggerElement(activeValue); if (activeTrigger && listRef.current) { const listRect = listRef.current.getBoundingClientRect(); const triggerRect = activeTrigger.getBoundingClientRect(); setIndicatorStyle({ left: triggerRect.left - listRect.left, width: triggerRect.width, }); } }; const rafId = requestAnimationFrame(() => { updateIndicator(); }); - window.addEventListener('resize', updateIndicator); + const resizeObserver = new ResizeObserver(updateIndicator); + if (listRef.current) { + resizeObserver.observe(listRef.current); + } return () => { cancelAnimationFrame(rafId); - window.removeEventListener('resize', updateIndicator); + resizeObserver.disconnect(); }; }, [activeValue, getTriggerElement]);
230-247: Add accessibility attributes for better screen reader support.The tab trigger is missing
aria-selectedand explicittype="button"attributes for proper accessibility and to prevent unintended form submissions.<motion.button ref={localRef} data-slot="tabs-trigger" role="tab" + type="button" + aria-selected={isActive} whileTap={{scale: 0.95}} onClick={() => handleValueChange(value)} data-state={isActive ? 'active' : 'inactive'}libs/react/ui/src/components/tabs/tabs.stories.tsx (2)
17-29: Consider rendering TabsContents in the Default story.The Default story only renders
TabsListwith triggers but omitsTabsContents. While this demonstrates the tab switching UI, it's inconsistent with the Controlled and MultipleTabs stories which both include content panels. Consider addingTabsContentswith correspondingTabsContentcomponents to provide a complete example.Apply this diff to add content panels:
export const Default: Story = { args: {defaultValue: 'analytics'} as never, render: () => ( <div className="bg-background-neutral-background p-24 w-[80vw]"> <Tabs defaultValue="analytics"> <TabsList className="gap-12 border-b border-neutral-strong"> <TabsTrigger value="analytics">Analytics</TabsTrigger> <TabsTrigger value="jobs">Jobs</TabsTrigger> </TabsList> + <TabsContents> + <TabsContent value="analytics"> + <div className="py-16"> + <p className="text-foreground-neutral-base">Analytics content</p> + </div> + </TabsContent> + <TabsContent value="jobs"> + <div className="py-16"> + <p className="text-foreground-neutral-base">Jobs content</p> + </div> + </TabsContent> + </TabsContents> </Tabs> </div> ), };
18-18: Consider removing theas nevertype assertions.The
as nevertype assertions on theargsproperty suggest a type mismatch between the Story args and the Tabs component props. These assertions bypass type safety and could hide legitimate type errors.If the Storybook
argsare not meant to control the component directly (since each story uses its ownrenderfunction), consider removing theargsproperty entirely or investigate the root cause of the type mismatch.Also applies to: 32-32, 65-65
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
libs/react/ui/index.css(3 hunks)libs/react/ui/src/components/badge/index.ts(1 hunks)libs/react/ui/src/components/dropdown-menu/index.ts(1 hunks)libs/react/ui/src/components/index.ts(1 hunks)libs/react/ui/src/components/modal/index.ts(1 hunks)libs/react/ui/src/components/modal/modal.stories.tsx(2 hunks)libs/react/ui/src/components/modal/modal.tsx(2 hunks)libs/react/ui/src/components/tabs/index.ts(1 hunks)libs/react/ui/src/components/tabs/tabs.stories.tsx(1 hunks)libs/react/ui/src/components/tabs/tabs.tsx(1 hunks)libs/react/ui/src/components/toast/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
We handle errors at the edge of our applications in most cases. Do not recommend to add error handling around every single function. We prefer them to bubble up and be handled at upper layers.
Files:
libs/react/ui/src/components/index.tslibs/react/ui/src/components/tabs/index.tslibs/react/ui/src/components/modal/modal.tsxlibs/react/ui/src/components/tabs/tabs.stories.tsxlibs/react/ui/src/components/modal/modal.stories.tsxlibs/react/ui/src/components/badge/index.tslibs/react/ui/src/components/tabs/tabs.tsxlibs/react/ui/src/components/modal/index.tslibs/react/ui/index.csslibs/react/ui/src/components/toast/index.tslibs/react/ui/src/components/dropdown-menu/index.ts
🧬 Code graph analysis (2)
libs/react/ui/src/components/tabs/tabs.stories.tsx (1)
libs/react/ui/src/components/tabs/tabs.tsx (5)
Tabs(302-302)TabsList(303-303)TabsTrigger(304-304)TabsContents(305-305)TabsContent(306-306)
libs/react/ui/src/components/modal/modal.stories.tsx (1)
libs/react/ui/src/components/modal/modal.tsx (1)
ModalContent(294-294)
⏰ 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: Agent
- GitHub Check: Continuous integration
🔇 Additional comments (14)
libs/react/ui/src/components/modal/modal.tsx (1)
128-128: LGTM! Clean addition ofoverlayClassNameprop.The prop is properly typed, destructured, and consistently passed to
ModalOverlayin both mobile and desktop rendering paths.Also applies to: 136-136, 144-144, 168-168
libs/react/ui/src/components/dropdown-menu/index.ts (1)
1-1: Consistent with the project's barrel export pattern.Wildcard re-exports simplify maintenance. Ensure the source module only exports intended public API to avoid accidental exposure of internal symbols.
libs/react/ui/src/components/badge/index.ts (1)
1-4: LGTM!Consistent refactor to wildcard re-exports across all badge submodules.
libs/react/ui/src/components/toast/index.ts (1)
1-2: LGTM!Consistent with the project-wide barrel export refactor.
libs/react/ui/index.css (1)
196-196: LGTM! New modal overlay token added correctly.The token follows the existing naming convention and is properly defined for both light/dark modes with the corresponding Tailwind theme alias. Using the same
alpha-black-88value in both themes is appropriate for modal overlays.Also applies to: 402-402, 748-748
libs/react/ui/src/components/tabs/tabs.tsx (3)
257-279: LGTM!The
TabsContentscomponent correctly filters children by matching thevalueprop against the active tab value.
286-299: LGTM with optional ARIA enhancement.The component correctly renders conditionally based on active state. For complete WAI-ARIA Tabs pattern compliance, consider adding
idandaria-labelledbyattributes to link panels with their triggers. This is optional for basic functionality.
301-314: LGTM!Clean and comprehensive export of all components, the hook, and associated types.
libs/react/ui/src/components/tabs/index.ts (1)
1-1: LGTM!The barrel export follows the established pattern for component modules in this codebase, consistent with the refactoring applied to modal, toast, and other components.
libs/react/ui/src/components/index.ts (1)
20-22: LGTM!The new exports for
moving-borderandtabsmodules follow the established wildcard re-export pattern and maintain alphabetical ordering.libs/react/ui/src/components/modal/index.ts (1)
1-1: LGTM!The refactoring to wildcard re-exports simplifies the module structure and aligns with the pattern applied across other components (tabs, toast, badge, dropdown-menu) in this PR.
libs/react/ui/src/components/tabs/tabs.stories.tsx (2)
31-62: LGTM!The Controlled story effectively demonstrates controlled usage with
valueandonValueChangeprops, displaying both tab triggers and content panels.
64-100: LGTM!The MultipleTabs story provides a comprehensive example of uncontrolled tabs usage with multiple tab panels, demonstrating the full component API.
libs/react/ui/src/components/modal/modal.stories.tsx (1)
143-143: CSS token configuration is correct and follows established patterns.The
overlayClassName="bg-background-modal-overlay"prop properly uses the new overlay customization feature. The CSS variable--background-modal-overlayis defined inlibs/react/ui/index.cssand mapped through the theme token system (--color-background-modal-overlay). This follows the same pattern as other successfully-used background tokens likebg-background-neutral-base,bg-background-components-base, andbg-background-field-basethroughout the codebase.
There was a problem hiding this comment.
Pull request overview
This PR adds a new Tabs component to the React UI library with support for both controlled and uncontrolled states, animated tab indicators, and integrates several related changes across the codebase.
Key Changes:
- New Tabs component with context-based state management, animated indicator, and framer-motion integration
- Refactored export patterns across multiple components (toast, modal, dropdown-menu, badge) to use
export *syntax - Enhanced Modal component with customizable overlay styling via
overlayClassNameprop - Added
--background-modal-overlaytheme token for consistent modal overlay styling
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| libs/react/ui/src/components/tabs/tabs.tsx | New Tabs component implementation with context API, controlled/uncontrolled modes, and animated indicator |
| libs/react/ui/src/components/tabs/tabs.stories.tsx | Storybook stories demonstrating default, controlled, and multiple tabs usage patterns |
| libs/react/ui/src/components/tabs/index.ts | Export file for Tabs component following the export * pattern |
| libs/react/ui/src/components/toast/index.ts | Refactored to use export * pattern instead of explicit named exports |
| libs/react/ui/src/components/modal/modal.tsx | Added overlayClassName prop to ModalContent for customizable overlay styling |
| libs/react/ui/src/components/modal/modal.stories.tsx | Updated stories to demonstrate new overlayClassName prop usage |
| libs/react/ui/src/components/modal/index.ts | Refactored to use export * pattern |
| libs/react/ui/src/components/dropdown-menu/index.ts | Refactored to use export * pattern |
| libs/react/ui/src/components/badge/index.ts | Refactored to use export * pattern |
| libs/react/ui/src/components/index.ts | Added exports for Tabs and MovingBorder components |
| libs/react/ui/index.css | Added --background-modal-overlay theme variable for light and dark modes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
2 participants
Summary by CodeRabbit
overlayClassNameprop.✏️ Tip: You can customize this high-level summary in your review settings.