feat: address the Parcelle staff-app dogfood findings#89
Merged
Conversation
Everything below came out of migrating 14+ real tables (including a 23k-row
virtualized warehouse grid) onto yable, and each item is something that forced
an app-side workaround.
Bugs / sharp edges:
- Container underflow on fully sized tables. The `distribute` / `stretch`
underflow policies only grew auto-sized columns, so a table where every column
has an explicit `size` had no participants: it pinned to its sized total and
left a dead band on the right. Such tables now fall back to growing every
visible column, and a new `underflow: 'stretch-last'` hands the remainder to
the last column alone. Widths resolve through `columnSizing`, so the sticky
header, body, virtualized inner table, and pinned offsets stay on one
colgroup; the CSS workaround for this let the header and the virtualized body
resolve the slack independently and drift out of alignment.
- `clickableRows` gated `onRowClick`. `row:click` was only emitted when the
rendered Table had `clickableRows`, so wiring the handler through `useTable`
alone silently did nothing, while `row:dblclick` and `row:contextmenu` always
emitted. `row:click` now emits unconditionally and `clickableRows` is purely
the visual affordance, defaulting on when an `onRowClick` handler is present.
API and theming gaps:
- `table.getToggleAllRowsSelectedHandler()` and
`getToggleAllPageRowsSelectedHandler()` mirror `row.getToggleSelectedHandler()`.
Both ignore their event argument, so wiring one to `onChange` toggles instead
of reading the event object as a truthy `value` flag.
- Header and cell typography tokens: `--yable-font-family-header`,
`--yable-font-family-cell`, `--yable-font-weight-header`,
`--yable-header-text-transform`, `--yable-header-letter-spacing`. Every
bundled theme now declares its header typography through these instead of
hardcoding it on its own `.yable-th` rule, which outranked any token set on
the grid root.
- `align` on a column def emits `data-align` on header, body, and footer cells
(with tabular figures when right-aligned), in both the React and vanilla
renderers.
- `data-sorted="asc|desc"` on `.yable-th`, replacing
`:has(.yable-sort-indicator[data-active='true'])`.
- `--yable-header-backdrop-filter` for frosted sticky headers, and
`--yable-detail-accent-width` / `--yable-detail-accent-color` for a
master-detail panel edge. Both default to visual no-ops.
- `./package.json` is now exported from all four packages, so
`require.resolve('@zvndev/yable-react/package.json')` stops throwing
ERR_PACKAGE_PATH_NOT_EXPORTED.
Density needed no code: `<Table density>` and `.yable--density-*` already
replace hand-setting six spacing tokens, so the themes README now documents them.
Verified with new unit tests, new Playwright coverage for the underflow fix
(including header/body column alignment under a sticky header plus row
virtualization), and browser checks that each new token and attribute actually
takes effect.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes every item from the Parcelle staff-app dogfood pass: 14+ real tables migrated onto yable, including a 23k-row virtualized warehouse grid, master-detail boards, and inline blur-save editors. Each item below forced an app-side workaround.
Bugs / sharp edges
Container underflow on fully sized tables.
distribute/stretchonly ever grew auto-sized columns, so a table where every column has an explicitsizehad no participants: it pinned to its sized total and left a dead band on the right that no policy could close. Those tables now fall back to growing every visible column, and a newunderflow: 'stretch-last'hands the whole remainder to the last column alone.Widths resolve through
columnSizing, so the sticky header, body, virtualized inner table, and pinned offsets all read one colgroup. That matters: the app-side CSS workaround (min-width: 100%) let the sticky header and the virtualized body resolve the slack independently and drift out of column alignment. New Playwright coverage asserts every header/body column pair stays within 1px under a sticky header plus row virtualization.clickableRowsgatedonRowClick.row:clickwas only emitted when the rendered<Table>hadclickableRows, so passing the handler touseTablealone silently did nothing, whilerow:dblclickandrow:contextmenualways emitted unconditionally.row:clicknow emits unconditionally;clickableRowsis purely the visual affordance and defaults on when anonRowClickhandler is present.clickableRows={false}keeps the handler and drops the affordance.Behavior change worth noting on review: anyone using
clickableRows={false}to suppress the callback now gets the callback.API and theming gaps
table.getToggleAllRowsSelectedHandler()/getToggleAllPageRowsSelectedHandler()mirrorrow.getToggleSelectedHandler(). Both ignore their event argument, so wiring one toonChangetoggles rather than reading the event object as a truthyvalue. The old docs example (onChange={table.toggleAllPageRowsSelected}) had exactly that bug and is fixed.--yable-font-family-header,--yable-font-family-cell,--yable-font-weight-header,--yable-header-text-transform,--yable-header-letter-spacing. Every bundled theme now declares header typography through them instead of hardcoding it on its own.yable-thrule, which outranked any token set on the grid root. Density utilities that set--yable-font-size-headerreach the header for the first time.alignon a column def emitsdata-alignon header, body, and footer cells, with tabular figures when right-aligned. React and vanilla renderers both.data-sorted="asc|desc"on.yable-th, replacing:has(.yable-sort-indicator[data-active='true']).--yable-header-backdrop-filter(frosted sticky headers) and--yable-detail-accent-width/--yable-detail-accent-color(master-detail panel edge). Both default to visual no-ops../package.jsonexported from all four packages, sorequire.resolve('@zvndev/yable-react/package.json')stops throwingERR_PACKAGE_PATH_NOT_EXPORTED.Density needed no code:
<Table density>and.yable--density-*already replace hand-setting six spacing tokens, so the themes README documents them now.Verification
data-sortedattribute tests, and a themes suite that fails if any theme reintroduces hardcoded header typography.prefer-consterror inuseAutoColumnSizing.ts; fixed with a targeted disable since it sits in a file this PR edits.)🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.