Add Table component#15
Conversation
|
📚 Storybook preview: https://pr-15-propel-storybook.vamsi-906.workers.dev |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new semantic, compound Table component to @plane/propel, including Storybook stories that exercise basic rendering and sortable header behavior (including aria-sort).
Changes:
- Adds
Tablecompound component primitives (Table,TableHeader,TableBody,TableRow,TableHead,TableCell) with Figma-aligned styling tokens and asortableheader variant. - Implements sortable header affordance via a button + lucide chevrons, reflecting state through
aria-sort. - Adds Storybook stories with interaction tests covering table roles and sortable header toggling.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/propel/src/components/table/index.tsx | Implements the Table compound component API and sortable header behavior. |
| packages/propel/src/components/table/table.stories.tsx | Adds Storybook stories + play tests validating roles and sortable interactions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Removed defaultVariants; TableHead variant now required. |
bhaveshraja
left a comment
There was a problem hiding this comment.
Added examples of 2 types of tables.
We also need capability to click a cell and update the value with the help of dropdown component.
Page nation is another component, can be used in table if required.
Address design review on PR #15 (Figma 4017-653): - Two variants on the root `Table` via a required `variant` prop, read by the cells through context: `table` (row dividers only) and `spreadsheet` (full grid — every cell bordered). Both share the Figma cell metrics (38px, px-3 py-2, header on layer-1, body on surface-1) and a rounded outer border. - `TableEditableCell`: a click-to-edit cell whose value + trailing chevron is the trigger for a propel Dropdown, picking a new value that updates the row in place (Figma "Account type" editable cell). Owns the Dropdown root + trigger; the consumer passes the DropdownContent. - Stories: Default (Table), Spreadsheet, Sortable, EditableCells (click-cell → dropdown → value updates), all with the 4017-653 design link. Pagination is intentionally left as a separate component to compose, per review.
|
@bhaveshraja addressed the review (Figma
Stories: Default (Table), Spreadsheet, Sortable, EditableCells, all linked to |
|
Added keyboard play tests: Enter/Space sorts (aria-sort cycles), editable cell opens the Dropdown via keyboard and Escape closes it. |
|
@bhaveshraja ready for re-review, all the changes-requested items are addressed (CI green):
Could you take another look and clear the changes-requested? |
Designer follow-up on #15: pagination can be used with the table. The Table and Pagination stay separate components, so the story keeps the full dataset, renders only the current page of rows, and lets the Pagination below drive page and page size (changing the size resets to page 1 and updates the range label). Verified in Storybook (table variants, editable cell, and pagination) plus a play test that advances a page and asserts the rows swap.
|
All three points are covered now:
Rebased onto main so it picks up the merged Dropdown and Pagination. Checked all of it in Storybook (both variants, the editable-cell dropdown, and paging through the rows). |
bhaveshraja
left a comment
There was a problem hiding this comment.
- There should be a border: borer-subtle around the table.
- Changing to a different page using pagination is resizing the table's width. The table's width shouldn't change.
- Incase of long vertical scroll, headers have to be sticking to the top. And for long horizontal scroll, Option to make first and last column sticky.
- Option to have leading and trailing icons in cells. Can be an avatar too. Kind of slot.
- Cell can also have only icon, eg. like more icon - to have options related to the row item.
- Row height should be 44px, current with editable cell its much bigger.
- Hover state and selected state should be added for editable cell.
Few of these are tittle tricky to show in figma due to its capabilities. Happy to get on to a call and explain.
…cell states Addresses design review on #15: - Rows are 44px. The editable cell was forcing rows to ~55px because its p-0 override lost to the shared cell padding under cx; padding now lives on each cell type, so the editable cell is truly full-bleed and its 44px button sets the height. - Editable cells get an explicit open/selected state (data-[popup-open]) next to the existing hover and focus states. - The WithPagination demo gets a fixed width so the table no longer resizes between pages (it is w-full and was shrink-to-fitting each page's content under centered layout). The table already has the border-subtle outer border.
|
Got the straightforward ones done:
The other three I'd rather do with you on that call, since they're the ones that are tricky to read off Figma: leading/trailing slots in cells (icon or avatar), an icon-only action cell for the per-row more menu, and the sticky header plus sticky first/last column behavior on scroll. Whenever works for you. |
…on cell (#15) Addresses the design review (Figma 5196-4084): 1. The outer border-subtle now shows: the table is wrapped in a rounded, bordered, scrollable frame (a <table> can't clip its own rounded corners, so the border was invisible before). 2. Sticky header on vertical scroll, plus an opt-in pinned first/last column on horizontal scroll via each cell's pinned start/end. The frame is keyboard-scrollable (tabindex) only while it overflows, so it adds no stray tab stop and satisfies the axe scrollable-region rule. 3. TableCell gains inlineStartNode/inlineEndNode slots for a leading/trailing icon or Avatar (the Name column). 4. New TableActionCell: an icon-only (default ellipsis) row-options cell that opens a Dropdown, mirroring TableEditableCell. 5. Rows sit on layer-2 / layer-2-hover; actionable cells are layer-transparent / layer-transparent-hover so the cell hover reads distinctly over the row hover. Closes #15
…erlay scrollbar, selected cell) Round of fixes from the PR #15 design review: - Spreadsheet variant no longer draws a double border. The scroll frame owns the single outer ring, so cells only draw interior dividers (inline-end + bottom, with the last column/row dropping theirs). This also clears the squared-off corners that poked past the rounded frame. - Header text now uses text-tertiary to match the Figma "Table header" token (was text-secondary on plain headers). - Scroll frame is now a Base UI ScrollArea, so the scrollbar overlays the content instead of reserving a 12px gutter. The header is no longer clipped and the table width stays constant whether or not the scrollbar is showing (also keeps width stable across pagination). Sticky header + pinned columns still work because the ScrollArea Viewport is the real scroll container; it also gets tabIndex=0 only while it overflows, which keeps the scrollable-region-focusable a11y rule satisfied without the manual ResizeObserver. - Pinned columns draw a hairline on their inline edge so they read as separated from the content scrolling under them (logical border, RTL-safe). - Editable cell gains a selected prop: a persistent layer-transparent-selected tint a step above hover, for marking the active cell. The story wires it to the last-opened cell. Verified in the browser across light/dark and LTR/RTL: single clean grid, tertiary header, centered avatars, 44px rows, overlay scrollbar (0 reserved space), constant width across pages, sticky header + pinned columns. vp check and the full test suite (736 tests, axe gate across 4 themes) pass.
|
Thanks for the detailed pass. Went through all the points against the current branch and pushed the fixes. Here's where each one landed (newest first):
Verified everything in the browser in light and dark, LTR and RTL. Full test suite passes (axe a11y gate runs across all four themes). The only one I'd flag for your eyes is 2., since I couldn't reproduce the misalignment. If you can point me at the exact screenshot/row, happy to take another look. |
The TableHead variant axis (default/sortable) no longer defaults to "default" via cva. variant is now a required prop, so every header cell declares its visual treatment explicitly. sort/onSort remain optional (additive, only meaningful for sortable headers).
Address design review on PR #15 (Figma 4017-653): - Two variants on the root `Table` via a required `variant` prop, read by the cells through context: `table` (row dividers only) and `spreadsheet` (full grid — every cell bordered). Both share the Figma cell metrics (38px, px-3 py-2, header on layer-1, body on surface-1) and a rounded outer border. - `TableEditableCell`: a click-to-edit cell whose value + trailing chevron is the trigger for a propel Dropdown, picking a new value that updates the row in place (Figma "Account type" editable cell). Owns the Dropdown root + trigger; the consumer passes the DropdownContent. - Stories: Default (Table), Spreadsheet, Sortable, EditableCells (click-cell → dropdown → value updates), all with the 4017-653 design link. Pagination is intentionally left as a separate component to compose, per review.
SortableKeyboard: Tab to the sortable header button, then Enter/Space cycle the sort (none -> ascending -> descending -> none) with aria-sort and onSort following along; asserts the th scope=col semantics. EditableCellKeyboard: Tab+Enter opens the portaled Dropdown, Arrow Down + Enter selects a value and the cell updates in place, and Escape closes the menu and returns focus to the cell trigger. The portaled menu is queried from the document body by unique item text with waitFor for open/close. Both tagged [!dev,!autodocs,!manifest] so they run under test only.
Designer follow-up on #15: pagination can be used with the table. The Table and Pagination stay separate components, so the story keeps the full dataset, renders only the current page of rows, and lets the Pagination below drive page and page size (changing the size resets to page 1 and updates the range label). Verified in Storybook (table variants, editable cell, and pagination) plus a play test that advances a page and asserts the rows swap.
Same Markdown-in-autodocs issue as Popover/Tabs/Dropdown: the indented JSX example let the wrapper tag leak out of the code box. Wrap it in a ```tsx fence.
…cell states Addresses design review on #15: - Rows are 44px. The editable cell was forcing rows to ~55px because its p-0 override lost to the shared cell padding under cx; padding now lives on each cell type, so the editable cell is truly full-bleed and its 44px button sets the height. - Editable cells get an explicit open/selected state (data-[popup-open]) next to the existing hover and focus states. - The WithPagination demo gets a fixed width so the table no longer resizes between pages (it is w-full and was shrink-to-fitting each page's content under centered layout). The table already has the border-subtle outer border.
…on cell (#15) Addresses the design review (Figma 5196-4084): 1. The outer border-subtle now shows: the table is wrapped in a rounded, bordered, scrollable frame (a <table> can't clip its own rounded corners, so the border was invisible before). 2. Sticky header on vertical scroll, plus an opt-in pinned first/last column on horizontal scroll via each cell's pinned start/end. The frame is keyboard-scrollable (tabindex) only while it overflows, so it adds no stray tab stop and satisfies the axe scrollable-region rule. 3. TableCell gains inlineStartNode/inlineEndNode slots for a leading/trailing icon or Avatar (the Name column). 4. New TableActionCell: an icon-only (default ellipsis) row-options cell that opens a Dropdown, mirroring TableEditableCell. 5. Rows sit on layer-2 / layer-2-hover; actionable cells are layer-transparent / layer-transparent-hover so the cell hover reads distinctly over the row hover. Closes #15
The RichRows action-menu test asserted toBeVisible right after findByRole, racing the menu's open scale-fade in CI. Wrap it in waitFor like the other menu tests.
…erlay scrollbar, selected cell) Round of fixes from the PR #15 design review: - Spreadsheet variant no longer draws a double border. The scroll frame owns the single outer ring, so cells only draw interior dividers (inline-end + bottom, with the last column/row dropping theirs). This also clears the squared-off corners that poked past the rounded frame. - Header text now uses text-tertiary to match the Figma "Table header" token (was text-secondary on plain headers). - Scroll frame is now a Base UI ScrollArea, so the scrollbar overlays the content instead of reserving a 12px gutter. The header is no longer clipped and the table width stays constant whether or not the scrollbar is showing (also keeps width stable across pagination). Sticky header + pinned columns still work because the ScrollArea Viewport is the real scroll container; it also gets tabIndex=0 only while it overflows, which keeps the scrollable-region-focusable a11y rule satisfied without the manual ResizeObserver. - Pinned columns draw a hairline on their inline edge so they read as separated from the content scrolling under them (logical border, RTL-safe). - Editable cell gains a selected prop: a persistent layer-transparent-selected tint a step above hover, for marking the active cell. The story wires it to the last-opened cell. Verified in the browser across light/dark and LTR/RTL: single clean grid, tertiary header, centered avatars, 44px rows, overlay scrollbar (0 reserved space), constant width across pages, sticky header + pinned columns. vp check and the full test suite (736 tests, axe gate across 4 themes) pass.
b219666 to
3303b0e
Compare
| /** | ||
| * Visual treatment. `sortable` renders a clickable button with a sort chevron; | ||
| * set it whenever you pass `sort`. | ||
| */ | ||
| variant: NonNullable<VariantProps<typeof tableHeadVariants>["variant"]>; |




Component
Adds the Table compound component to
@plane/propel, rebuilt from Figma node 4017-653 ("Table", header/data cells 38px,px-4 py-2, subtle row dividers).Parts / subcomponents
A compound API of plain semantic elements, each
OmitingclassName/stylefrom its intrinsic props:Table(<table>), root, renders onsurface-1withtext-13primary body textTableHeader(<thead>)TableBody(<tbody>)TableRow(<tr>), subtle bottom divider, hover tint, last-row divider hiddenTableHead(<th>),variantdefault(bg-layer-1/text-secondary,text-12semibold) vssortableTableCell(<td>)Stories declare these via the
subcomponentsmeta (mirrors the avatar-group pattern).Sortable header
TableHead variant="sortable"renders the label as an interactive button with a lucide sort chevron (ChevronsUpDown/ChevronUp/ChevronDown,aria-hidden) and reflects order througharia-sort(none/ascending/descending) via thesort/onSortprops.Figma
Tokens mapped from node 4017-653 master components ("Table header" 4032-7901, "Table cell" 4032-7892):
bg-surface-1,bg-layer-1,border-subtle,text-primary/text-secondary/text-tertiary,text-icon-secondary,text-12/text-13, 38px height,px-4 py-2. No arbitrary hex;cva+ semantic propel utilities only.Verification
vp install, okvp check, all formatted, no lint/type errorsvp run -r test(Playwright chromium), 14 passed, incl. 2 new Table tests (rows/cells/columnheader roles; sortable header togglesaria-sorton click)vp run -r build, build complete, attw: No problems found (pre-existing publint./hooks/*warnings are unrelated)Dependencies
Blocked by: #16 (Dropdown, editable cells). Optionally composes: #30 (Pagination). Changes requested; the Table + Spreadsheet variants + editable cells are queued behind #16.