feat(ui): Add RowList compound component#13
Conversation
Implements RowList — a structured column list lighter than a full DataTable. Uses CSS Grid driven by column definitions for aligned header + row layout, with visual-only sort indicators and full List foundation support (selection, keyboard nav, density variants).
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new RowList component (implementation, types, CSS, tests, and Storybook stories), exports it from the row-list package, and re-exports it from the main components index. Changes
Sequence DiagramsequenceDiagram
autonumber
actor User
participant Header as "RowListHeader"
participant Context as "RowListContext"
participant Parent as "ParentComponent"
participant Root as "RowListRoot"
User->>Header: click sortable header cell
Header->>Context: read current sortState
Header->>Header: compute next sort direction
Header->>Parent: call onSortChange(newSortState)
Parent->>Parent: update sortState prop
Parent->>Root: re-render with updated props
Root->>Context: provide updated sortState
Context->>Header: Header receives updated sortState and updates aria-sort
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/base-ui/src/components/row-list/RowList.stories.tsx`:
- Around line 322-328: The large-list story uses Math.random() to generate the
duration in the largeData array, causing non-deterministic renders; update the
duration generation in the largeData constant so it is derived deterministically
from the index (i) instead (for example use a function of i like (i * 7) % 120
or i % 120) so each entry's duration is stable across runs—modify the largeData
definition (the duration field) to use a deterministic computation instead of
Math.random().
- Around line 91-124: The render callback for WithSorting currently calls
useState (sortState/setSortState) which violates rules-of-hooks; extract that
logic into a capitalized React component (e.g., WithSortingComponent or
WithSortingStory) that accepts the same props (spread args) and contains the
useState<SortState> hook, the sorted computation, and returns the RowList JSX
using sortState and setSortState (passed to RowList via sortState and
onSortChange). Then change WithSorting.render to simply return
<WithSortingComponent {...args} /> (preserving style width and other props) so
hooks run inside a proper component.
In `@packages/base-ui/src/components/row-list/types.ts`:
- Line 41: Replace the empty interface declaration "RowListHeaderProps" with a
type alias to avoid the empty-object lint rule: change the export from an empty
interface to a type alias using HTMLAttributes<HTMLDivElement> (i.e., export
type RowListHeaderProps = HTMLAttributes<HTMLDivElement>), and ensure any
imports/usages of RowListHeaderProps remain valid.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 18860584-59b9-428f-acb0-0f04c0afab3a
📒 Files selected for processing (7)
packages/base-ui/src/components/index.tspackages/base-ui/src/components/row-list/RowList.module.csspackages/base-ui/src/components/row-list/RowList.stories.tsxpackages/base-ui/src/components/row-list/RowList.test.tsxpackages/base-ui/src/components/row-list/RowList.tsxpackages/base-ui/src/components/row-list/index.tspackages/base-ui/src/components/row-list/types.ts
- Extract WithSorting render into a proper component to fix hooks violation - Use deterministic duration generation instead of Math.random() - Replace empty RowListHeaderProps interface with type alias
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/base-ui/src/components/row-list/RowList.stories.tsx`:
- Around line 209-213: The stories for DensityVariants and ProblemsPanel pass
density explicitly to RowList but then spread meta.args (which sets density:
'default') after {...args}, causing the explicit density to be overridden; fix
by ensuring explicit props win—move {...args} before the explicit density prop
or remove density from meta.args in those stories so RowList receives the
intended density; update the RowList usage in the DensityVariants and
ProblemsPanel story definitions (and the repeated block around the other
occurrence) to place {...args} before density or strip density from meta.args.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d7e43227-a2a2-460c-83a2-e2d84ac4d04b
📒 Files selected for processing (2)
packages/base-ui/src/components/row-list/RowList.stories.tsxpackages/base-ui/src/components/row-list/types.ts
… stories
Move {...args} spread before explicit density prop in DensityVariants
and ProblemsPanel stories so the intended density value wins.
…ignment List.Item applies padding-inline on the outer element, which shifts the grid inward relative to the Header row. Reset to 0 since each Cell already applies its own padding-inline, matching the header cells.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/base-ui/src/components/row-list/RowList.module.css`:
- Around line 22-35: .HeaderCell is missing min-width: 0 which prevents grid
items from shrinking and stops text-overflow: ellipsis from working; update the
.HeaderCell rule to include "min-width: 0" (same approach used in .Cell) so long
header labels can truncate properly while keeping existing display, padding, and
overflow rules.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 204c654e-377b-440e-aa90-ad65de712011
📒 Files selected for processing (1)
packages/base-ui/src/components/row-list/RowList.module.css
…lity - Reset inherited gap from List.Item so row columns align with header - Add muted text color on disabled rows to make them clearly distinguishable alongside the base opacity treatment
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/base-ui/src/components/row-list/RowList.module.css (1)
22-35:⚠️ Potential issue | 🟡 MinorMissing
min-width: 0prevents text truncation in header cells.
.HeaderCellis missingmin-width: 0, while.Cell(line 77) correctly includes it. Without this, grid items default tomin-width: auto, preventing them from shrinking below their content size. Long header labels won't triggertext-overflow: ellipsis—the cell will overflow its grid column instead.🔧 Proposed fix
.HeaderCell { display: flex; align-items: center; gap: var(--ov-space-inline-xs, 4px); + min-width: 0; min-height: var(--_ov-item-height); padding-inline: var(--_ov-item-padding-inline); font-size: var(--ov-font-size-caption);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/base-ui/src/components/row-list/RowList.module.css` around lines 22 - 35, .HeaderCell is missing min-width: 0 which prevents grid items from shrinking and stops text-overflow: ellipsis from working; update the .HeaderCell rule (alongside existing .Cell behavior) to include min-width: 0 so long header labels can truncate correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/base-ui/src/components/row-list/RowList.module.css`:
- Around line 22-35: .HeaderCell is missing min-width: 0 which prevents grid
items from shrinking and stops text-overflow: ellipsis from working; update the
.HeaderCell rule (alongside existing .Cell behavior) to include min-width: 0 so
long header labels can truncate correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6ebb1fd6-3a97-4b66-ab38-cf94516f8273
📒 Files selected for processing (1)
packages/base-ui/src/components/row-list/RowList.module.css
Without min-width: 0, grid items won't shrink below content size, preventing text-overflow: ellipsis from working on long header labels.
Summary
RowListcompound component — a tabular list with structured columns, lighter than a full DataTablecolumnsprop for aligned header + row layout with a shared--_ov-row-grid-columnsvariableonSortChangecallback — consumer owns data sortingList.Rootfoundation, inheriting selection (single/multiple), keyboard navigation, density variants, and disabled stateCompound Slots
RowList(Root),Header,Viewport,Item,Cell,Group,GroupHeader,Separator,Empty,LoadingTest plan
pnpm typecheckpassespnpm test— all 331 tests passLists/RowListsection renders all 8 stories (BasicColumns, WithSorting, SingleSelect, MultiSelect, DensityVariants, ProblemsPanel, DisabledRows, LargeList)Summary by CodeRabbit