feat(ui): introduce Card and EmptyState components, upgrade Charts#276
feat(ui): introduce Card and EmptyState components, upgrade Charts#276
Conversation
kylengn
requested review from
EnzalRad,
dvxam and
noe-charmet
as code owners
|
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 WalkthroughIntroduces a new composable Card component with subcomponents (CardHeader, CardTitle, CardDescription, CardContent, CardFooter, CardAction), a new EmptyState component with default and compact variants, and significantly refactors existing BarChart and LineChart components to include Card wrappers, empty state handling, improved hover interactions, and enhanced axis/grid/tooltip control. Adds utility functions for tooltip normalization, color selection, and hover opacity calculation. Updates DataTable with loading state skeleton rendering and improves styling for chart tooltips and table rows. Includes comprehensive Storybook stories for all new and updated components. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
libs/react/ui/src/components/table/data-table.tsx (1)
205-206: InconsistentCardContentstyling between loading and loaded states.The loading state applies
rounded-8 overflow-hiddentoCardContent(line 206) while the loaded state only hasp-0(line 236). This may cause a subtle visual difference when transitioning from loading to loaded.Consider aligning the classes for consistency:
🔎 Proposed fix
- <CardContent className="p-0"> + <CardContent className="rounded-8 overflow-hidden p-0">Also applies to: 235-236
libs/react/ui/src/components/card/card.stories.tsx (1)
71-81: Consider extracting the common header layout pattern.The header layout structure with
CardActionis duplicated between theWithActionandLoginFormstories. While duplication in stories is acceptable for clarity, you might consider extracting this pattern into a reusable render helper if more stories follow this pattern.Example refactor
const renderCardHeaderWithAction = (title: string, description: string, actionLabel: string) => ( <CardHeader> <div className="flex items-start justify-between gap-16"> <div className="flex flex-col gap-4 flex-1"> <CardTitle>{title}</CardTitle> <CardDescription>{description}</CardDescription> </div> <CardAction> <Button variant="transparent" size="sm"> {actionLabel} </Button> </CardAction> </div> </CardHeader> );Also applies to: 96-106
libs/react/ui/src/components/card/card.tsx (1)
1-84: Optional: Consider adding JSDoc comments for better developer experience.While the component names are clear and self-documenting, adding JSDoc comments with usage examples could help developers understand the intended composition patterns (e.g., how CardAction fits within CardHeader, when to use CardFooter vs CardAction).
Example JSDoc pattern
+/** + * Main card container with padding, border, and rounded corners. + * Use with CardHeader, CardContent, and CardFooter for structured layouts. + * @example + * <Card> + * <CardHeader> + * <CardTitle>Title</CardTitle> + * </CardHeader> + * <CardContent>Content here</CardContent> + * </Card> + */ export function Card({className, children, ...props}: CardProps) {
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
libs/react/ui/src/components/card/card.stories.tsxlibs/react/ui/src/components/card/card.tsxlibs/react/ui/src/components/card/index.tslibs/react/ui/src/components/dashboard/components/charts/bar-chart.stories.tsxlibs/react/ui/src/components/dashboard/components/charts/bar-chart.tsxlibs/react/ui/src/components/dashboard/components/charts/chart-tooltip.tsxlibs/react/ui/src/components/dashboard/components/charts/colors.tslibs/react/ui/src/components/dashboard/components/charts/index.tslibs/react/ui/src/components/dashboard/components/charts/line-chart.stories.tsxlibs/react/ui/src/components/dashboard/components/charts/line-chart.tsxlibs/react/ui/src/components/dashboard/components/charts/utils.tslibs/react/ui/src/components/dashboard/index.tslibs/react/ui/src/components/empty-state/empty-state.stories.tsxlibs/react/ui/src/components/empty-state/empty-state.tsxlibs/react/ui/src/components/empty-state/index.tslibs/react/ui/src/components/index.tslibs/react/ui/src/components/table/data-table.tsxlibs/react/ui/src/components/table/table.stories.components.tsxlibs/react/ui/src/components/table/table.tsx
🧰 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/card/index.tslibs/react/ui/src/components/empty-state/index.tslibs/react/ui/src/components/table/table.tsxlibs/react/ui/src/components/index.tslibs/react/ui/src/components/dashboard/components/charts/colors.tslibs/react/ui/src/components/table/table.stories.components.tsxlibs/react/ui/src/components/dashboard/components/charts/index.tslibs/react/ui/src/components/empty-state/empty-state.tsxlibs/react/ui/src/components/empty-state/empty-state.stories.tsxlibs/react/ui/src/components/dashboard/index.tslibs/react/ui/src/components/table/data-table.tsxlibs/react/ui/src/components/dashboard/components/charts/utils.tslibs/react/ui/src/components/dashboard/components/charts/bar-chart.tsxlibs/react/ui/src/components/dashboard/components/charts/line-chart.stories.tsxlibs/react/ui/src/components/dashboard/components/charts/line-chart.tsxlibs/react/ui/src/components/dashboard/components/charts/chart-tooltip.tsxlibs/react/ui/src/components/dashboard/components/charts/bar-chart.stories.tsxlibs/react/ui/src/components/card/card.tsxlibs/react/ui/src/components/card/card.stories.tsx
🧬 Code graph analysis (11)
libs/react/ui/src/components/table/table.stories.components.tsx (4)
libs/react/ui/src/components/dashboard/components/charts/bar-chart.stories.tsx (1)
EmptyState(169-181)libs/react/ui/src/components/dashboard/components/charts/line-chart.stories.tsx (1)
EmptyState(152-164)libs/react/ui/src/components/empty-state/empty-state.tsx (1)
EmptyState(13-63)libs/react/ui/src/components/table/table.stories.tsx (1)
EmptyState(57-68)
libs/react/ui/src/components/empty-state/empty-state.tsx (2)
libs/react/ui/src/components/icon/icon.tsx (2)
IconName(77-77)Icon(86-90)libs/react/ui/src/components/typography/text.tsx (1)
Text(27-50)
libs/react/ui/src/components/empty-state/empty-state.stories.tsx (1)
libs/react/ui/src/components/empty-state/empty-state.tsx (1)
EmptyState(13-63)
libs/react/ui/src/components/dashboard/components/charts/utils.ts (3)
libs/react/ui/src/components/dashboard/components/charts/chart-tooltip.tsx (1)
ChartTooltipContentProps(4-15)libs/react/ui/src/components/dashboard/components/charts/colors.ts (3)
ChartColor(11-11)chartColors(1-9)chartColorsList(13-21)libs/react/ui/src/components/dashboard/index.ts (1)
ChartColor(7-7)
libs/react/ui/src/components/dashboard/components/charts/bar-chart.tsx (3)
libs/react/ui/src/components/dashboard/components/charts/chart-tooltip.tsx (1)
ChartTooltipContent(17-60)libs/react/ui/src/components/dashboard/components/charts/utils.ts (3)
normalizeTooltipPayload(11-30)getChartColor(32-34)getHoverOpacity(36-41)libs/react/ui/src/components/empty-state/empty-state.tsx (1)
EmptyState(13-63)
libs/react/ui/src/components/dashboard/components/charts/line-chart.stories.tsx (1)
libs/react/ui/src/components/dashboard/components/charts/line-chart.tsx (2)
LineChart(49-187)LineChartProps(33-47)
libs/react/ui/src/components/dashboard/components/charts/line-chart.tsx (7)
libs/react/ui/src/components/dashboard/index.ts (1)
LineChartProps(7-7)libs/react/ui/src/components/card/card.tsx (4)
Card(7-19)CardHeader(23-29)CardTitle(33-39)CardContent(68-74)libs/react/ui/src/components/tooltip/tooltip.tsx (1)
Tooltip(114-114)libs/react/ui/src/components/dashboard/components/charts/chart-tooltip.tsx (1)
ChartTooltipContent(17-60)libs/react/ui/src/components/dashboard/components/charts/utils.ts (3)
normalizeTooltipPayload(11-30)getChartColor(32-34)getHoverOpacity(36-41)libs/react/ui/src/components/dashboard/components/charts/line-chart.stories.tsx (1)
EmptyState(152-164)libs/react/ui/src/components/empty-state/empty-state.tsx (1)
EmptyState(13-63)
libs/react/ui/src/components/dashboard/components/charts/chart-tooltip.tsx (1)
libs/react/ui/src/components/typography/text.tsx (1)
Text(27-50)
libs/react/ui/src/components/dashboard/components/charts/bar-chart.stories.tsx (1)
libs/react/ui/src/components/dashboard/components/charts/bar-chart.tsx (2)
BarChart(51-182)BarChartProps(33-49)
libs/react/ui/src/components/card/card.tsx (1)
libs/react/ui/src/components/typography/text.tsx (1)
Text(27-50)
libs/react/ui/src/components/card/card.stories.tsx (8)
libs/react/ui/src/components/card/card.tsx (7)
Card(7-19)CardHeader(23-29)CardTitle(33-39)CardDescription(43-54)CardContent(68-74)CardFooter(78-84)CardAction(58-64)libs/react/ui/src/components/dashboard/components/charts/bar-chart.stories.tsx (1)
Default(25-40)libs/react/ui/src/components/dashboard/components/charts/line-chart.stories.tsx (1)
Default(25-40)libs/react/ui/src/components/empty-state/empty-state.stories.tsx (1)
Default(16-23)libs/react/ui/src/components/typography/text.tsx (1)
Text(27-50)libs/react/ui/src/components/button/button.tsx (1)
Button(50-91)libs/react/ui/src/components/label/label.tsx (1)
Label(13-15)libs/react/ui/src/components/input/input.tsx (1)
Input(24-43)
🔇 Additional comments (42)
libs/react/ui/src/components/table/table.tsx (1)
41-47: Verifyborder-radiusrenders correctly on<tr>elements.The
last:rounded-b-8utility may not visually apply to<tr>elements in standard table layouts, asborder-radiusis typically ignored on table rows withborder-collapse: collapse. This usually requires eitherborder-collapse: separateon the table or applying the radius to the<td>cells directly (e.g.,last:first:rounded-bl-8 last:last:rounded-br-8on cells).If the table already uses separate border mode or a CSS layout where this works, feel free to disregard.
libs/react/ui/src/components/table/data-table.tsx (3)
16-19: LGTM!Clean addition of the necessary imports for the new loading state and Card wrapper functionality.
156-170: LGTM!Explicit type annotations improve clarity and the
stopPropagationcorrectly prevents row click events when interacting with selection checkboxes.
253-284: LGTM!Good implementation with proper accessibility for clickable rows (keyboard support with Enter/Space) and a sensible default
EmptyStatefallback.libs/react/ui/src/components/dashboard/index.ts (1)
7-7: LGTM!Clean extension of the public type exports to include
ChartColor.libs/react/ui/src/components/empty-state/index.ts (1)
1-1: LGTM!Standard barrel export for the EmptyState module.
libs/react/ui/src/components/dashboard/components/charts/index.ts (1)
5-5: LGTM!Properly extends the charts public API to include utility exports.
libs/react/ui/src/components/card/index.ts (1)
1-1: LGTM!Standard barrel export for the Card module.
libs/react/ui/src/components/table/table.stories.components.tsx (1)
5-5: LGTM!Excellent refactor to use the new standardized
EmptyStatecomponent instead of custom markup. This improves consistency and maintainability across the UI library.Also applies to: 25-30
libs/react/ui/src/components/index.ts (1)
7-7: LGTM!Clean expansion of the public API to include the new Card, Dashboard, and EmptyState modules.
Also applies to: 12-12, 18-18
libs/react/ui/src/components/dashboard/components/charts/colors.ts (3)
7-7: LGTM!Good addition of the
ambercolor to expand the chart color palette.
19-20: LGTM!Properly updated
chartColorsListto include the newamberand updatedneutralcolors.
8-8: No verification necessary — neutral color is not currently used by any chart stories or implementations.The
neutralcolor in thechartColorsListserves only as a fallback when rendering more than 6 data series without explicit color assignments. Existing chart implementations (bar-chart and line-chart) all use explicit colors or rely on fallback colors through thegetChartColor()utility. The Storybook stories for both BarChart and LineChart do not test theneutralcolor, and no production usage of this fallback scenario is documented.libs/react/ui/src/components/dashboard/components/charts/chart-tooltip.tsx (3)
1-3: LGTM! Clean import organization.The
Textcomponent import is properly added for consistent typography rendering.
29-32: Good use of design system tokens and Text component.The tooltip container styling and label rendering are consistent with the design system. The
labelFormatterfallback pattern is appropriate.
38-54: Well-structured hover interaction and consistent typography.The conditional opacity class elegantly handles the hover state, and the
Textcomponents withtabular-numsensure proper alignment for numeric values.libs/react/ui/src/components/empty-state/empty-state.stories.tsx (2)
4-14: Well-structured Storybook meta configuration.The meta configuration follows Storybook best practices with
autodocstags and centered layout.
16-73: Comprehensive story coverage for EmptyState variants.The stories effectively demonstrate the component's API, including default behavior, custom content, compact variant, and optional props. Good use of fixed-width containers for consistent visual testing.
libs/react/ui/src/components/dashboard/components/charts/utils.ts (2)
11-30: Solid payload normalization with good type safety.The function handles undefined payloads gracefully and provides safe type coercion. One minor consideration:
Number(p.value)will produceNaNfor non-numeric strings, which will then be formatted and displayed. This is acceptable behavior since the formatter is user-provided and should handle expected value types.
32-41: Clean utility functions for color and hover state management.Both functions are pure, well-typed, and handle edge cases correctly. The modulo operation in
getChartColorensures safe cycling through the color list.libs/react/ui/src/components/empty-state/empty-state.tsx (3)
6-11: Well-designed props interface extending native div attributes.The
EmptyStatePropsinterface properly extendsComponentProps<'div'>, allowing consumers to pass additional div props while providing type-safe component-specific props.
21-31: Clear variant-based styling logic.The conditional class assignments for
containerClasses,iconContainerClasses, andiconSizeare easy to follow. This approach works well for two variants.
42-60: Good conditional rendering and typography hierarchy.The optional title and description with appropriate text sizing (
smfor title,xsfor description) creates a clear visual hierarchy. Thespace-y-4only applies in default variant which is correct.libs/react/ui/src/components/dashboard/components/charts/line-chart.stories.tsx (2)
4-24: Well-organized Storybook configuration and sample data.The meta configuration and shared
sampleDataprovide a solid foundation for the stories. The data structure supports multi-series visualization testing.
25-164: Excellent story coverage demonstrating LineChart capabilities.The stories thoroughly cover the component's API including title, colors, axis visibility, hidden lines, formatters, and empty state handling. The
satisfies LineChartPropsensures type safety for story args.libs/react/ui/src/components/dashboard/components/charts/bar-chart.stories.tsx (4)
4-24: Consistent Storybook setup matching LineChart stories pattern.The meta configuration and sample data structure align well with the LineChart stories, maintaining consistency across the chart components.
91-107: Good demonstration of stacked bars feature.The
StackedBarsstory effectively showcases thestackIdproperty for grouping bars into stacked visualizations.
109-125: Clear demonstration of rounded bar corners.The
RoundedBarsstory withbarRadius: [4, 4, 0, 0]clearly shows how to customize bar corner rounding.
169-181: EmptyState story validates empty data handling.Good coverage of the empty state scenario with
data: [].libs/react/ui/src/components/dashboard/components/charts/line-chart.tsx (5)
1-21: Clean imports for enhanced chart functionality.The imports are well-organized, bringing in Card components for layout, EmptyState for no-data scenarios, and chart utilities for color/hover management.
25-47: Well-typed props interface with controlled API surface.The
Picktypes for axis and grid props expose only the relevant configuration options, keeping the API clean. Thehideproperty onLineChartLineenables selective series visibility.
62-72: Comprehensive empty state detection.The logic correctly identifies empty scenarios: no data, no visible lines, or all-zero values. The all-zero check is a good UX decision since a chart with only zeros provides no meaningful visualization. Negative values are correctly handled.
127-157: Well-implemented hover interaction for line series.The hover state management across both
LineandactiveDotensures consistent interaction behavior. ThestrokeOpacityadjustment provides clear visual feedback for the hovered series.
169-183: Clean legend implementation with consistent coloring.The legend correctly reuses
getChartColorto ensure color consistency with the chart lines. The conditional rendering (!isEmpty && visibleLines.length > 0) is appropriate.libs/react/ui/src/components/dashboard/components/charts/bar-chart.tsx (5)
1-21: Consistent imports matching LineChart structure.The import organization mirrors the LineChart component, ensuring consistency across the chart suite.
25-49: Solid type definitions for BarChart configuration.The
BarChartBarinterface properly extendsBarPropswhile adding chart-specific properties. ThestackIdproperty enables stacked bar visualizations, andbarRadiustuple type matches Recharts' corner radius specification.
66-76: Empty state detection consistent with LineChart.The logic mirrors the LineChart implementation, ensuring consistent behavior across chart types. While there's some duplication, extracting this to a shared utility might be overkill for two components.
133-152: Clean Bar rendering with proper stacking and hover support.The Bar components are rendered with consistent color resolution, hover opacity, and stacking via
stackId. ThebarRadiusandmaxBarSizeprops provide visual customization.
155-178: Appropriate EmptyState and legend rendering.The
barChartBoxLineicon is contextually appropriate for the bar chart empty state. The legend implementation matches the LineChart pattern, maintaining consistency across the chart suite.libs/react/ui/src/components/card/card.tsx (3)
1-19: LGTM! Solid foundation for the Card component.The imports are appropriate, and the main Card component provides a well-structured container with sensible defaults (padding, border, flex layout). The type-safe props pattern and className merging with
cnutility follow React best practices.
21-64: Excellent composable header components.The CardHeader family demonstrates good design:
- Proper type extensions (
CardTitleProps extends ComponentProps<typeof Header>andCardDescriptionProps extends ComponentProps<typeof Text>) preserve full prop typing from the wrapped typography components.- Sensible defaults (
variant='h3'for CardTitle,size='sm'for CardDescription) establish visual hierarchy.- Gap sizes create appropriate spacing (gap-4 within header vs gap-16 at Card level).
ml-autoon CardAction enables flexible right-aligned actions.
66-84: Well-designed content and footer sections.CardContent's
flex-1appropriately fills remaining space in the flex column layout, and CardFooter provides sensible horizontal alignment withitems-center. The consistent component pattern and prop handling make this family easy to compose.
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
| <span className="size-8 shrink-0 rounded-full" style={{backgroundColor: item.color}} /> | ||
| {item.name && ( | ||
| <span className="text-xs text-foreground-neutral-subtle">{item.name}</span> | ||
| <Text size="xs" className="text-foreground-neutral-subtle"> |
There was a problem hiding this comment.
These Text elements should have the compact property to match the Figma design.
There was a problem hiding this comment.
Text's compact props = true by default
There was a problem hiding this comment.
Ok my bad was not expecting this behavior
There was a problem hiding this comment.
I don't seem to find a clear part of the Figma design which would reference those components.
They most likely should exist, but it looks like we would need to discuss with the @y405 to check what such a component would entail.
Typically this screenshot of a full card in the Storybook
However some cards seem to have a line for header/footer + title is different between the card with the table, the cards with KIP, and the card with a graph.
There was a problem hiding this comment.
@noe-charmet so what should I fix here? Card components are not available in the Design System, but we should have it to be a consistent wrapper component. The Charts and DataTable now are wrapped with Card components, however, since Modal had been made before we added Card, so it's been wrapped by its own Modal components, e.g Header, Content, Footer.
There was a problem hiding this comment.
I think this requires you to have a chat with Yael on how this should be consistent (or not).
There was a problem hiding this comment.
Yep, I will connect with him on that. Meanwhile, is this PR good to merge or not? I need to release to continue working on the visibility PR using these components.
There was a problem hiding this comment.
Ok this can be reworked in. a followup pr
2 participants
Summary by CodeRabbit
Release Notes
New Features
Improvements
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.