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 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. WalkthroughThis PR comprehensively refactors KPI cards and table components to use Card-based layouts, introduces loading skeleton support and a query-driven KPI group builder, removes search props from the table wrapper in favor of header actions, and propagates scoped container rendering through table pagination and column components. The public dashboard API surface expands significantly with new exports and types. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
There was a problem hiding this comment.
Pull request overview
This PR adds a new generic KpiCardsGroupFromQuery component and refactors table components to support scoped container portals, along with various UI improvements to the dashboard table wrapper and related components.
- Adds
KpiCardsGroupFromQuerycomponent for generating KPI cards from query-like data structures - Introduces
scopedContainerprop to table components for controlling dropdown portal rendering - Refactors
TableWrapperto use Card components and simplifies its API by removing built-in search functionality
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| libs/react/ui/src/components/table/table.tsx | Removes last:border-b-0 class from table rows to fix border styling |
| libs/react/ui/src/components/table/table-pagination.tsx | Adds scopedContainer prop for controlling select dropdown portal placement and adds border-b-0 to pagination row |
| libs/react/ui/src/components/table/table-column-header.tsx | Adds scopedContainer prop for dropdown menu portal and wraps header text with Text component |
| libs/react/ui/src/components/table/data-table.tsx | Adds scopedContainer prop and passes it through to child components, updates CardContent border radius |
| libs/react/ui/src/components/dashboard/table/table-wrapper.tsx | Major refactor: removes built-in search functionality, migrates to Card components, and simplifies API by moving search/actions to headerActions prop |
| libs/react/ui/src/components/dashboard/pages/jobs-page.tsx | Removes comments and moves SearchInline/Button components to headerActions prop of TableWrapper |
| libs/react/ui/src/components/dashboard/pages/analytics-page.tsx | Removes comments and moves SearchInline/Button components to headerActions prop of TableWrapper |
| libs/react/ui/src/components/item/item.stories.tsx | Replaces inline kbd element with Kbd component |
| libs/react/ui/src/components/dashboard/index.ts | Adds exports for new KPI card types and removes inline comments |
| libs/react/ui/src/components/dashboard/components/kpi-card.tsx | Adds new KpiCardsGroupFromQuery generic component, loading state support to KpiCard, and improves styling for mobile/desktop layouts |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
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: 2
🧹 Nitpick comments (2)
libs/react/ui/src/components/table/table-column-header.tsx (1)
110-110: Unused ref:headerRefis created but never used.The
headerRefis assigned to the outer div on line 123 but is never read or passed anywhere. If it was intended as a fallback container for the dropdown, it's not being used. Consider removing it to avoid confusion.🔎 Proposed fix
export function TableColumnHeader<TData, TValue>({ column, title, className, scopedContainer, }: TableColumnHeaderProps<TData, TValue>) { - const headerRef = useRef<HTMLDivElement>(null); - if (!column.getCanSort()) {And on line 123:
- <div ref={headerRef} className={cn('flex items-center space-x-2', className)}> + <div className={cn('flex items-center space-x-2', className)}>If you no longer need
useRef, also remove the import:-import {useRef} from 'react';libs/react/ui/src/components/table/table-pagination.tsx (1)
80-80: Unused ref:paginationRefis created but never used.Similar to
TableColumnHeader, thepaginationRefis assigned toTableFooterbut is never read or utilized. If it was intended as a fallback container, it's not being used sincescopedContaineris passed directly toSelectContent.🔎 Proposed fix
}: TablePaginationProps<TData>) { - const paginationRef = useRef<HTMLTableSectionElement>(null); const currentPage = table.getState().pagination.pageIndex + 1;And on line 88:
- <TableFooter ref={paginationRef} className={className} {...props}> + <TableFooter className={className} {...props}>If you no longer need
useRef, also remove the import:-import {useRef} from 'react';
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
libs/react/ui/src/components/dashboard/components/kpi-card.tsxlibs/react/ui/src/components/dashboard/index.tslibs/react/ui/src/components/dashboard/pages/analytics-page.tsxlibs/react/ui/src/components/dashboard/pages/jobs-page.tsxlibs/react/ui/src/components/dashboard/table/table-wrapper.tsxlibs/react/ui/src/components/item/item.stories.tsxlibs/react/ui/src/components/table/data-table.tsxlibs/react/ui/src/components/table/table-column-header.tsxlibs/react/ui/src/components/table/table-pagination.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/item/item.stories.tsxlibs/react/ui/src/components/table/table-pagination.tsxlibs/react/ui/src/components/table/table-column-header.tsxlibs/react/ui/src/components/table/table.tsxlibs/react/ui/src/components/dashboard/pages/analytics-page.tsxlibs/react/ui/src/components/dashboard/pages/jobs-page.tsxlibs/react/ui/src/components/table/data-table.tsxlibs/react/ui/src/components/dashboard/table/table-wrapper.tsxlibs/react/ui/src/components/dashboard/components/kpi-card.tsxlibs/react/ui/src/components/dashboard/index.ts
🧬 Code graph analysis (7)
libs/react/ui/src/components/item/item.stories.tsx (1)
libs/react/ui/src/components/kbd/kbd.tsx (1)
Kbd(6-20)
libs/react/ui/src/components/table/table-pagination.tsx (2)
libs/react/ui/src/components/table/table.tsx (2)
TableFooter(95-95)TableRow(95-95)libs/react/ui/src/components/select/select.tsx (1)
SelectContent(212-212)
libs/react/ui/src/components/table/table-column-header.tsx (3)
libs/react/ui/src/components/typography/text.tsx (1)
Text(27-50)libs/react/ui/src/components/dropdown-menu/dropdown-menu.tsx (3)
DropdownMenu(388-388)DropdownMenuTrigger(389-389)DropdownMenuContent(391-391)libs/react/ui/src/components/button/button.tsx (1)
Button(50-91)
libs/react/ui/src/components/dashboard/pages/jobs-page.tsx (7)
libs/react/ui/src/components/dashboard/index.ts (1)
TableWrapper(38-38)libs/react/ui/src/components/dashboard/table/table-wrapper.tsx (1)
TableWrapper(106-154)libs/react/ui/src/components/dashboard/table/index.ts (1)
TableWrapper(6-6)libs/react/ui/src/components/table/table.stories.columns.tsx (1)
jobColumns(196-196)libs/react/ui/src/components/search/search-inline.tsx (1)
SearchInline(14-108)libs/react/ui/src/components/button/button.tsx (1)
Button(50-91)libs/react/ui/src/components/icon/icon.tsx (1)
Icon(86-90)
libs/react/ui/src/components/table/data-table.tsx (1)
libs/react/ui/src/components/card/card.tsx (1)
CardContent(68-74)
libs/react/ui/src/components/dashboard/table/table-wrapper.tsx (2)
libs/react/ui/src/components/card/card.tsx (4)
CardHeader(23-29)CardTitle(33-39)CardAction(58-64)CardContent(68-74)libs/react/ui/src/components/table/data-table.tsx (1)
DataTable(115-309)
libs/react/ui/src/components/dashboard/components/kpi-card.tsx (2)
libs/react/ui/src/components/typography/text.tsx (1)
Text(27-50)libs/react/ui/src/components/skeleton/skeleton.tsx (1)
Skeleton(6-14)
🔇 Additional comments (19)
libs/react/ui/src/components/item/item.stories.tsx (1)
6-6: LGTM!Good refactor to use the
Kbdcomponent instead of a raw<kbd>element. This ensures consistent styling across the codebase and leverages the component's built-in design tokens.Also applies to: 108-108
libs/react/ui/src/components/table/table.tsx (1)
37-50: LGTM!The styling change retains the bottom border on the last row while preserving the rounded corners. This aligns with the Card-based layout updates across table components.
libs/react/ui/src/components/table/table-column-header.tsx (1)
112-146: Good implementation of scoped container support.The
scopedContainerprop is correctly passed toDropdownMenuContent, and theTextcomponent usage provides consistent typography. The fallback toundefinedwhenscopedContaineris null is appropriate.libs/react/ui/src/components/dashboard/components/kpi-card.tsx (3)
24-49: LGTM!The
KpiCardcomponent is well-structured with proper loading state handling usingSkeleton. The Card wrapper and typography usage are consistent with the design system.
55-90: Good responsive implementation.The mobile-first approach with scroll-snap for swipeable cards and hidden scrollbars provides a solid UX. The responsive breakpoints for desktop flex layout are appropriate.
92-142: Well-documented generic interface.The
SelectorConfigandKpiCardsGroupFromQueryConfiginterfaces are flexible and well-documented, allowing for various query-like structures to drive KPI card rendering.libs/react/ui/src/components/table/data-table.tsx (2)
106-112: LGTM!The
scopedContainerprop is well-documented and correctly added to the component signature.Also applies to: 127-127
297-304: Proper prop threading.The
scopedContaineris correctly passed down toTablePagination, enabling scoped portal rendering for the pagination dropdown.libs/react/ui/src/components/dashboard/pages/jobs-page.tsx (2)
1-3: LGTM!Clean imports for the new header action components.
41-54: Good refactor to header-driven search.The
headerActionspattern centralizes search and column controls in the table header, improving consistency with other pages. TheSearchInlineis properly bound to the existingsearchQuerystate, and the icon button has appropriate accessibility attributes.libs/react/ui/src/components/dashboard/pages/analytics-page.tsx (2)
1-3: LGTM!Consistent imports with
jobs-page.tsxfor header action components.
143-159: Consistent header actions pattern.The implementation mirrors
jobs-page.tsx, maintaining consistency across dashboard pages. The search and column insertion controls are appropriately placed in the table header.libs/react/ui/src/components/table/table-pagination.tsx (2)
57-69: LGTM on scoped container implementation.The
scopedContainerprop is well-documented and correctly passed toSelectContent. The fallback toundefinedwhen null is appropriate.Also applies to: 77-77, 119-119
89-89: Border styling update.Adding
border-b-0to the pagination row prevents double borders when nested within Card-based layouts.libs/react/ui/src/components/dashboard/table/table-wrapper.tsx (3)
8-8: LGTM!Card component imports are correctly added to support the new Card-based layout.
63-76: LGTM!Well-documented new props. The
scopedContainerprop is particularly useful for scoped CSS styling in embedded dashboard scenarios.
125-134: Clean Card-based layout implementation.The responsive header with
flex-col md:flex-rowand the conditional rendering ofheaderActionswithinCardActionare well structured.libs/react/ui/src/components/dashboard/index.ts (2)
4-12: LGTM!The expanded KPI-related type exports and the new
KpiCardsGroupFromQuerycomponent export are well-organized. Separating type exports from value exports follows good TypeScript practices.
36-38: Summary inconsistency: AnalyticsPage and JobsPage are still exported.The AI-generated summary claims these exports were removed, but they're present in the final code at line 36. The code takes precedence—this is simply a summary inaccuracy.
…ing card properties
…mated value display
…ving unused code and simplifying props
…isplay using CountUp component
kylengn
changed the title
There was a problem hiding this comment.
Should this one have a story ?
There was a problem hiding this comment.
@noe-charmet added a story for it here fe2e5cf
2 participants
Summary by CodeRabbit
Release Notes
New Features
Refactor
Style
✏️ Tip: You can customize this high-level summary in your review settings.