@wordpress/dataviews: migrate card layout to @wordpress/ui Card and CollapsibleCard#76282
@wordpress/dataviews: migrate card layout to @wordpress/ui Card and CollapsibleCard#76282
Conversation
|
Size Change: +113 kB (+1.64%) Total Size: 7 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 52dcd6b. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/22914089510
|
There was a problem hiding this comment.
Pull request overview
Migrates @wordpress/dataviews DataForm card layout (and related stories) from @wordpress/components Card primitives to the newer @wordpress/ui Card and CollapsibleCard, aligning DataViews/DataForm with the Base UI + DS-token-based card implementation.
Changes:
- Refactor DataForm card layout to use
Card.*for standard cards andCollapsibleCard.*for collapsible cards, simplifying toggle/ARIA handling. - Update card layout SCSS to match the new header structure and typography responsibilities.
- Update Storybook stories to use
@wordpress/uicards and note expected visual differences.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/dataviews/src/components/dataform-layouts/card/index.tsx | Migrates DataForm card layout rendering to @wordpress/ui Card/CollapsibleCard and restructures header/body composition. |
| packages/dataviews/src/components/dataform-layouts/card/style.scss | Updates card layout styles for the new header content wrapper and removes old heading mixin usage. |
| packages/dataviews/src/dataviews/stories/with-card.tsx | Updates DataViews “WithCard” story to wrap content with Card.Root/Header/Title/Content. |
| packages/dataviews/src/dataviews/stories/free-composition.tsx | Updates “FreeComposition” story to use @wordpress/ui Card primitives (and drops unsupported variant). |
| packages/dataviews/CHANGELOG.md | Adds an Unreleased entry documenting the migration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| label: 'Customer Name', | ||
| type: 'text', | ||
| // TODO: should use `<Text />` when ready? | ||
| render: ( { item } ) => <span>{ item.name }</span>, |
There was a problem hiding this comment.
Rendering plain text makes it difficult to style it, both in general terms (we need an HTML element to target with CSS rules) and more specifically since Card and CollapsibleCard don't include any particular text styles. If we're sticking to DS primitives, Text (which is WIP, but practically ready) will be the right component for the job :)
There was a problem hiding this comment.
Or maybe, should we change the text item type to use the Text component by default?
There was a problem hiding this comment.
I guess my question was: is any style missing for this particular field?
|
API-wise this is working well (different combinations can be tested via storybook). |
|
Validation is working as expected as well. |
|
Other than this, this PR looks good to me and it's ready to ship. |
|
Another aspect is that we may want to remove this rule gutenberg/packages/dataviews/src/dataviews/style.scss Lines 130 to 137 in d410e57 |
28d0fbf to
d3d10da
Compare
|
Update: header flickr (customer card):The flicker can be mitigated by setting a minimum height on the card header, although:
For now, I set a min height of card wrapper for dataviews: misaligned (header) & missspaced (dataviews doesn't take the full width):I can see three main differences:
Toggle button label:Opened #76329 to tackle separately Other style changes:I guess that overall differences in card spacing are an expected consequence of swapping card component. On top of individual CHANGELOG entries, we should probably prepare a detailed dev note for the next release, since dataviews/dataforms will go through other similar visual changes, as we refactor to use more Re. the summary badge, I un-did that change — I guess we can refactor all |
| .components-card__body:has(> .dataviews-picker-wrapper) { | ||
| padding: $grid-unit-10 0 0; | ||
| overflow: hidden; // Prevent cells with white backgrounds overflowing the card | ||
| } |
There was a problem hiding this comment.
@oandregal I'd definitely like to remove these styles:
- they won't apply to the new card anyway (and
@wordpress/uicomponents don't expose classnames) - we can remove horizontal padding by using
Card.FullBleed - I wonder if, instead, we should remove internal padding of the dataviews UI (since we're the dataviews package, it feels more appropriate to override internal dataviews components)
- how can we test scenarios in which
overflow: hiddenwould come in handy?
In general, the bulk of the question is that it's a bit tricky for dataviews/dataform to apply these overrides, since consumers can literally pass any UI components (like shown in packages/dataviews/src/dataviews/stories/with-card.tsx). I almost feel that, if this is a pattern that dataviews wants to encourage / promote, we should consider making some dedicated higher-level components?
also cc @WordPress/gutenberg-components for thoughts on styles and APIs composition .
There was a problem hiding this comment.
Given the with card story has been migrated to use the new card, and it is now officially the composition pattern we'd support, do we have any reason to maintain legacy styles to remain compatible with the old card? Could we remove them and mark this as a breaking change?
There was a problem hiding this comment.
Re. the padding:
- As Jay also mentioned, probably the cleanest approach would be to remove padding from
<Dataviews />. It will also ensure that, regardless ofCard's padding, dataviews embedded inCardwill always be correctly aligned - As an alternative, the current approach (ie. wrapping dataviews in
Card.FullBleed) is the second best. Apart from announcing the visually breaking change in CHANGELOGS, would we need to refactor any other usages in Gutenberg?
Re. the overflow: hidden
Not sure exactly when these styles would be handy. And if we still need to apply them, we'd need to rework the Storybook example, adding those styles explicitly (and logging a similar breaking change)
There was a problem hiding this comment.
Apart from announcing the visually breaking change in CHANGELOGS, would we need to refactor any other usages in Gutenberg?
No, there's no cases in Gutenberg where we wrap DataViews in a card.
There was a problem hiding this comment.
Re. the padding: probably the cleanest approach would be to remove padding from
Historically, this has proved tricky. I don't remember the details off the top of my head, but I do remember a couple of threads with many folks involved. Given this change only affects the storybook, there's no Gutenberg use case for this, and the official card component is now the wordpress/ui one, I'd recommend the second approach in this PR.
Though, I do think we should explore making DataViews padding agnostic (it becomes a consumer decision). My point is that it is a separate concern that merits its own dedicated exploration.
There was a problem hiding this comment.
@oandregal anything you could gather re. overflow: hidden?
Starting without the border seems fine to me. We can add it back later if necessary, but I suppose that would be better handled in a separate PR.
I still think the cleanest solution here is for dataviews to not supply any padding at all. I appreciate this is a tricky change, but its the one that makes the most sense to me. Dataviews should inherit padding from its container, not supply its own imo. Otherwise we're going to run into this issue with all containers; dialogs, tabs, cards, page, accordions, and so on... I don't remember if we added a css variable for padding in dataviews, but perhaps that would be another approach? Then we can set padding to 0 when dataviews is rendered inside a card. Or do the full-bleed thing and set the padding to match the card. |
|
Took the PR for a spin (haven't looked at the code yet).
I'm reading this is intentional change that's part of the Design System, and that's something we'd need to address at some point. 👍 to ship. For reference, these are the differences (left is after, right is before):
Compared to how it worked, this looks shippable to me (left is after, right is before):
|
|
@ciampo I'm trying to understand where this PR stands. With a mindset of solving one problem at a time and iterate quickly, and given these testing scenarios, are there any more changes you'd like to address here? I know we talked about maintaning the exact spacing so this PR is just a replacement from the old to the new card. That approach would make me more comfortable. However, I understand you're arguing these changes are in line with the DS, is that correct? If it is, I'm fine shipping and iterating on the remaining aspects we've discussed (fix mobile view, explore tweaking the spacing via theme tokens/style prop, etc.). |
Next steps before merging:
Other follow-up work that shouldn't block this PR:
What do you think? |
…leCard Replace @wordpress/components Card, CardBody, CardHeader with @wordpress/ui Card.Root, Card.Header, Card.Content, Card.Title and CollapsibleCard.Root, CollapsibleCard.Header, CollapsibleCard.Content. The collapsible behavior previously implemented manually with useState and toggle callbacks is now delegated to CollapsibleCard's built-in defaultOpen/onOpenChange API. The header layout (title, summary fields, validation badge) and the body content are extracted into dedicated sub-components for clarity. The card-specific SCSS is simplified by removing the heading-large mixin (now handled by Card.Title) and the header-label class. Made-with: Cursor
Update with-card.tsx and free-composition.tsx stories to use Card.Root, Card.Header, Card.Content, and Card.Title from @wordpress/ui instead of Card, CardHeader, and CardBody from @wordpress/components. Made-with: Cursor
Made-with: Cursor
Address review feedback: - Wrap header content (title, validation badge, summary fields) in a flex container to preserve the horizontal layout that existed before the refactoring. - Use controlled `open` prop instead of `defaultOpen` on CollapsibleCard, and restore the useEffect that syncs local state when the isOpened layout config changes externally (e.g. via Storybook controls). Made-with: Cursor
Made-with: Cursor
431bee1 to
52dcd6b
Compare








What?
Part of #76100
Migrate the dataform card layout component and DataViews story files from using
Card,CardBody,CardHeaderfrom@wordpress/componentsto the newerCardandCollapsibleCardfrom@wordpress/ui.Why?
The
@wordpress/uipackage recently addedCardandCollapsibleCardcomponents (#76252), built on Base UI with CSS modules and design system tokens. This PR adopts them in the@wordpress/dataviewspackage, which is one of the first consumers.How?
Main component (
dataform-layouts/card/index.tsx)Card.Root,Card.Header,Card.Content, andCard.Titlefrom@wordpress/uiCollapsibleCard.Root,CollapsibleCard.Header,CollapsibleCard.Contentfrom@wordpress/ui, which handles expand/collapse behavior internally via Base UI's Collapsible primitivesHeaderContentsub-component wrapped in a flex container for proper horizontal layoutBodyContentsub-componentopenprop is used onCollapsibleCard.Rootto keep local state in sync (needed for summary field visibility rules anduseReportValidity)aria-expanded/aria-controls/aria-labelledbyattributes — these are now handled by Base UI's Collapsible@wordpress/composeuseInstanceIddependency (no longer needed)@wordpress/iconsdependency (chevron icons now handled by CollapsibleCard)@wordpress/componentsButtondependency (toggle button now handled by CollapsibleCard)Card styles (
card/style.scss)heading-large()mixin — replaced byCard.Title's built-in typography.dataforms-layouts-card__field-header-labelclass.dataforms-layouts-card__field-header-contentflex container for the header row layoutStory files
with-card.tsx: Migrated fromCard/CardHeader/CardBodytoCard.Root/Card.Header/Card.Title/Card.Contentfree-composition.tsx: Migrated similarly. Note: thevariant="secondary"prop on one Card instance is dropped since@wordpress/uiCard doesn't support variants — this is an expected visual change aligned with the design systemBundle size note
This PR increases the bundle size of
block-editor,editor, andedit-siteby ~35-40 kB each. This is expected: the old code importedCard/CardBody/CardHeaderfrom@wordpress/components(which is externalized viawpScript: true), while the new code imports from@wordpress/ui(which is bundled inline viawpScript: false). The@wordpress/uiCard/CollapsibleCard components and their@base-ui/reactdependencies now get included in each consumer's bundle. This is an inherent characteristic of@wordpress/ui's current packaging and will be resolved when@wordpress/uitransitions towpScript: true.Testing Instructions
npx jest --config test/unit/jest.config.js packages/dataviews-- all 221 tests should passDataForm / LayoutCardstory and test all Storybook control combinations (withHeader,isCollapsible,isOpened,withSummary)DataViews / WithCardstory -- verify card wrappingDataViews / FreeCompositionstory -- verify cards in custom layoutNext Steps
24pxvs20px) separately