feat(core): add useTableRowStatus plugin#3757
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR Analysis ReportModified ComponentsBanner
Chat
CodeBlock
DateTimeInput
FileInput
MobileNav
NumberInput
PowerSearch
SideNav
Table
authoring
Bundle Size Summary
Accessibility AuditStatus: 7 accessibility violation(s) found — 1 critical, 6 serious. Banner - 1 issue(s)
Chat - 1 issue(s)
FileInput - 3 issue(s)
PowerSearch - 1 issue(s)
Table - 1 issue(s)
Generated by PR Enrichment workflow | View full report |
26a7041 to
701978c
Compare
|
@ernestt can I have your design eyes here pls |
701978c to
19ce3e2
Compare
ernestt
left a comment
There was a problem hiding this comment.
Thanks again for continuing all the plugins! @humbertovirtudes Curious why we are diverging from the original version that shows either a status dot or icon? My 2 worries with this design are
- Status is conveyed through color alone
- The shape of this is similar to out tab selection state. Might be interpreted as selected row?
the vertical color feels less invasive / space consuming that a status dot. However if status dot is preferred for consistency, Im down |
Prepends a narrow column that renders a full-height colored bar on each
row's leading edge — a compact per-row status signal (error, warning,
unread, etc.) without a dedicated status column.
- getStatus(item) maps a row to {color, label?} (any CSS color/token +
optional accessible label), or null for no indicator.
- Storybook stories (token colors + raw colors), docsite hook doc +
example block, unit tests, and a Table Lab toggle.
Address review feedback (cixzhang) and self-review findings: - Fix invalid token in the flagship @example: 'var(--color-icon-error)' does not exist. The API now accepts semantic color NAMES first-class. - Semantic colors (cixzhang): color accepts 'success'|'error'|'warning'| 'accent'|'red'|'orange'|'green'|'yellow'|'blue'|'gray', mapped to theme tokens; raw CSS color strings still pass through as an escape hatch. - Icon signifier (cixzhang): optional `icon` renders an Icon as the status signifier so status is conveyed by shape + color, not color alone — more accessible when multiple statuses coexist. Column widens to fit the icon. - a11y: role="img" + aria-label only when a label is provided; documented that omitting label yields a color-only (inaccessible) indicator. - Document that getStatus should be memoized (useCallback) for a stable plugin identity; updated doc.mjs + config JSDoc. - Tests (8, all green): semantic-color→token mapping, raw-CSS escape hatch, icon-mode renders an svg + keeps the accessible name, label-omitted → no role=img (color-only bar still renders), and empty-data (header, no bars). - Updated the Storybook Default story and Table Lab example to the idiomatic semantic-color + icon form. Tags: ai-generated This diff was generated with AI assistance (Navi). The implementation and tests were produced by an AI agent; the human author reviewed and approved the changes.
Switch the default indicator from a full-height leading-edge bar to a centered status dot, per review feedback: - the bar's shape read as the tab selection state - a dot matches the original status-dot / icon pattern Icon mode is unchanged (shape + color for a11y). Updated tests, docs, story, CLI template, and Table Lab wording accordingly.
19ce3e2 to
490c4ab
Compare
|
Good call @ernestt, switched it. The default indicator is now a centered status dot rather than the full-height leading-edge bar, which addresses both concerns:
|
|
Following up with the specific conventions behind my two worries — both trace directly to the Design Conventions wiki, so this is less a taste call than a convention gap. 1. Status by color alone. The System Interaction States section states it as a hard rule: "Status always pairs a semantic token with an icon — never color alone." The Color foundation flags the same thing as a smell (🟡 "Color alone — status by hue with no icon/text"). Right now 2. Collision with the selection language + the edge-bar tell. Two conventions here:
The overarching principle is "Minimize visuals per state… Reuse an existing representation before inventing a new one" — which is really why I asked why we diverged from the original dot/icon version. That version was already convention-compliant: a leading status icon/dot (shape + semantic token) signals per-row status without color-alone and without borrowing the selection edge-bar. I'd lean toward going back to that as the primary treatment, keeping the color as reinforcement rather than the sole signal. |
|
@ernestt should be good to land now yes? |
cixzhang
left a comment
There was a problem hiding this comment.
Thanks for making the updates! This is starting to look closer to what we have internally. @ernestt does this address your comments?
I left a few more items for your consideration. Main blocking one is whether we should force label to be required. I'm not sure if there's another way we can ensure accessible context for unlabeled statuses.
| /** Optional icon rendered as the signifier instead of the dot (shape as an a11y differentiator). */ | ||
| icon?: IconName; | ||
| /** Accessible label; announced via role="img". Recommended. */ | ||
| label?: string; |
There was a problem hiding this comment.
Hmm do we want to make this required instead? Without the label I'm not sure how we would make the status accessible for AT users.
There was a problem hiding this comment.
Good call — made it required. label is now a required field on TableRowStatus, so a status can never be conveyed by color alone; it's announced to assistive tech via role="img" and also shown on hover. Updated the type, the render path, the doc, and the tests in 90a6088.
There was a problem hiding this comment.
Yes this looks good from my side. Thanks for fixing!
| {...stylex.props(styles.wrap)} | ||
| role={role} | ||
| aria-label={status.label} | ||
| title={status.label}> |
There was a problem hiding this comment.
What do you think about applying tooltip instead of the native title for more visual consistency?
There was a problem hiding this comment.
Done — switched from the native title to the Tooltip component for visual consistency with the rest of the system. The indicator (dot or icon) is now wrapped in <Tooltip content={label}>. In 90a6088.
Make the accessible label required so a row status is never conveyed by color alone, and render it through Tooltip instead of the native title attribute for visual consistency. Addresses review feedback.
Surface the legal color values and icon names in the getStatus prop doc instead of naming the TableRowStatusColor and IconName aliases, so a reader without an IDE can see what is valid.
# Conflicts: # packages/core/src/Table/index.ts


What
Adds
useTableRowStatus— a plugin that prepends a narrow column rendering a full-height colored bar on each row's leading edge: a compact per-row status signal (error, warning, unread, etc.) without a dedicated status column.Design
getStatus(item)maps a row to{color, label?}— any CSS color (token or raw) plus an optional accessible label — ornullfor no indicator.__rowStatuscolumn with an empty header;transformBodyCellzeroes the cell padding so a4pxabsolutely-positioned bar spans the row's full height on the leading edge.role="img"+aria-label/titlewhen a label is given, so status is announced and hoverable.Includes
Core/TableRowStatus):Default(theme-token colors, mixed statuses incl. "no bar") andRawColors(hex).useTableRowStatushook page + a "Status Bars" example block.null, and color applied to the bar.Validation