feat: Per-column table skeleton placeholders via renderCell - #4879
feat: Per-column table skeleton placeholders via renderCell#4879gethinwebster wants to merge 4 commits into
Conversation
gethinwebster
changed the title
There was a problem hiding this comment.
Pull request overview
Adds a new renderCell callback to the Table skeleton configuration to allow per-column skeleton placeholders (e.g., multi-line cells, status indicators, action areas), while keeping the existing single-line skeleton as the default/fallback.
Changes:
- Extends
TableProps.SkeletonConfigtoSkeletonConfig<T = any>and adds optionalrenderCell(column) => ReactNode. - Wires
renderCellthroughInternalTableintoSkeletonRows, rendering custom per-column skeleton content with fallback to the default skeleton. - Adds unit tests covering custom rendering, fallback behavior, and
aria-hiddencontainment; updates documenter snapshot and adds a dev page example.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/table/skeleton-rows.tsx | Uses renderCell to render per-column skeleton content (with fallback). |
| src/table/internal.tsx | Passes skeleton.renderCell through to SkeletonRows in both initial and progressive skeleton rendering paths. |
| src/table/interfaces.tsx | Introduces generic SkeletonConfig<T> and documents/defines the new renderCell API. |
| src/table/tests/skeleton.test.tsx | Adds unit tests for renderCell behavior and accessibility placement. |
| src/tests/snapshot-tests/snapshots/documenter.test.ts.snap | Updates generated snapshot output for the new generic skeleton type and docs. |
| pages/table/skeleton-render-cell.page.tsx | Adds a dev page demonstrating default vs per-column skeleton placeholders. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address Copilot review: renderCell returns ReactNode (incl. null); use an explicit undefined check instead of ?? so a null return renders an empty placeholder. Document the null vs undefined distinction and add a test.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4879 +/- ##
=======================================
Coverage 97.64% 97.64%
=======================================
Files 957 958 +1
Lines 31200 31243 +43
Branches 11504 11534 +30
=======================================
+ Hits 30464 30506 +42
- Misses 729 730 +1
Partials 7 7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
pages/table/skeleton-render-cell.page.tsx:41
- This status column cell renderer is complex enough that keeping it on one line hurts readability and is inconsistent with the multi-line formatting used for other non-trivial cells in this file (for example the
descriptioncolumn). Reformatting also helps avoid print-width churn in future diffs.
{
id: 'status',
header: 'Status',
cell: item => <StatusIndicator type={item.status === 'error' ? 'error' : 'success'}>{item.status}</StatusIndicator>,
},
| } | ||
|
|
||
| export interface FixedSkeletonConfig { | ||
| interface BaseSkeletonConfig<T> { |
There was a problem hiding this comment.
Leaving as-is intentionally. The declaration-emit concern doesn't apply here: BaseSkeletonConfig lives in the same module/namespace as the interfaces that extend it, so there's no TS4023 "private name" issue — confirmed by build/build (which runs declaration emit) passing green. It's also not against convention: this repo already has non-exported Base* helper interfaces inside *Props namespaces (e.g. BaseUtility inside TopNavigationProps). Keeping it non-exported is deliberate — it exists only to share the single optional renderCell prop across the fixed/auto configs, and exporting it would add a public type to the API surface for no consumer benefit.
Description
Adds an optional
renderCellfunction to the Tableskeletonconfig so teams can provide a per-column skeleton placeholder for cells whose settled content isn't a single line of text (multi-line cells, status indicators, actions, etc.). A uniform one-line bar mismatches those shapes and causes a layout jump when data lands;renderCelllets the placeholder match the final cell shape so the load-to-settle transition stays stable.skeleton={{ totalRows, renderCell: (column) => ReactNode }}— available on both the fixed and auto configs via a shared base.SkeletonConfigis now generic (SkeletonConfig<T>, defaulting toany), so existingTableProps.SkeletonConfigreferences are unaffected.undefinedfor a column to fall back to the default single-line skeleton (so a single central function can customize only the columns that need it).Skeletoncomponent. It renders inside the existingaria-hiddenskeleton<tr>, so it is not announced to screen readers; JSDoc guidance: do not render focusable/interactive elements.renderCellis optional; omitting it preserves today's single-line skeleton exactly.Draft for design review of the API shape.
Related links, issue #, if available: n/a
How has this been tested?
src/table/__tests__/skeleton.test.tsx: custom content rendered per column,undefined-> default-skeleton fallback, and custom content staying insidearia-hiddenrows. Full skeleton suite: 22/22 pass.pages/table/skeleton-render-cell.page.tsxcompares the default single-line skeleton against a column-shaped one (two-line description, status indicator, actions button).tsc --noEmitclean for the changed files.Review checklist
Correctness
skeletonandrenderCell; dev page added.renderCelloptional,SkeletonConfig<T = any>keeps existing refs valid.aria-hiddenrow (test asserts this); JSDoc warns against focusable elements.Security
Testing