-
Notifications
You must be signed in to change notification settings - Fork 4.6k
DataForm - Card Layout: implement summary field #71576
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DataForm - Card Layout: implement summary field #71576
Conversation
661436a to
a5d71d5
Compare
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
a5d71d5 to
2a68e6e
Compare
|
Thank you for working on this @gigitux ! I wanted to note that the current design (at least for Settings) removes the summary when the card is open. I just wanted to know if this is the default behaviour we should support? |
|
The
So all of these scenarios are valid: {
summary: 'flight-details' // only visible on close
}{
summary: [ 'flight-details', 'seat-number' ] // only visible on close
}{
summary: [
'flight-details', // only visible on close
{ id: 'requires-visa-badge', visibility: 'always' } // always visible
]
} |
…ataform-card-layout-allow-having-summarybadge
What do you think if the {
id: 'customer-card',
label: '',
children: [...],
summary: ['email', 'birthday'],
layout: {
type: "card",
summaryVisibility: "always"
}
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just moved the function from panel layout.
123da82 to
fcb5eca
Compare
packages/dataviews/src/types.ts
Outdated
| export type SimpleFormField = { | ||
| id: string; | ||
| layout?: Layout; | ||
| summary?: string | string[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that even SimpleFormField should support summary prop, no?
fcb5eca to
caee7ea
Compare
…ataform-card-layout-allow-having-summarybadge
|
Flaky tests detected in 9df0b31. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/18220009386
|
|
@oandregal I implemented this approach. Let me know what you think about: Screen.Capture.on.2025-09-25.at.12-09-29.mov |
|
Size Change: +717 B (+0.04%) Total Size: 1.95 MB
ℹ️ View Unchanged
|
packages/dataviews/CHANGELOG.md
Outdated
|
|
||
| - Introduce a new `DataViewsPicker` component. [#70971](https://github.com/WordPress/gutenberg/pull/70971) and [#71836](https://github.com/WordPress/gutenberg/pull/71836). | ||
| - DataForm: Add support for elements validation in array fields [#71194](https://github.com/WordPress/gutenberg/pull/71194) | ||
| - DataForm: Add support for summary fields in card layouts, allowing display of condensed information in card headers. [#71576](https://github.com/WordPress/gutenberg/pull/71576) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be moved to a new "breaking changes" section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed with 5b3694f
| layout: { | ||
| type: 'panel', | ||
| summary: 'discussion', | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This wouldn't be necessary if, by default, the summary for a panel layout is the form field id:
This is how it works in this PR:
{
"id": "discussion",
"label": "Discussion",
"layout": {
"type": "panel",
"summary": "discussion"
},
"children": ["comment_status", "ping_status"]
}This is how it should work by default:
{
"id": "discussion",
"label": "Discussion",
"children": ["comment_status", "ping_status"]
}In other words, consumers only need to provide a summary if it's different from the form field id (discussion) in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with this approach, but it is a breaking change. Currently, when a summary isn't present, the first child is picked up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is already a breaking change (moving summary to layout), so it'd be nice to address this here to minimize the number of consecutive breaking changes.
That behavior (picking the first child for panel) was a fine start, but we've introduced summary because it wasn't enough or the best default. With today's status and knowledge, I find this logic more aligned with what we want:
- Summary is
summary, if it exists. - Otherwise, summary is the Field that matches the form field's id, if it exists.
- Otherwise, use the 1st child.
Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes sense. Improved the logic with: ff1fef6
| { summaryFields.length > 0 && layout.withHeader && ( | ||
| <div className="dataforms-layouts-card__field-summary"> | ||
| { summaryFields.map( | ||
| ( summaryField ) => | ||
| isSummaryFieldVisible( | ||
| summaryField, | ||
| layout.summary, | ||
| isOpen | ||
| ) && ( | ||
| <summaryField.render | ||
| key={ summaryField.id } | ||
| item={ data } | ||
| field={ summaryField } | ||
| /> | ||
| ) | ||
| ) } | ||
| </div> | ||
| ) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to operate only with the visible summary fields. It'll be clearer, and we'll prevent the dataforms-layout-card__field-summary wrapper from being rendered when all summary fields are invisible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed with 98c6be0.
| typeof layout.isOpened === 'boolean' | ||
| ? layout.isOpened | ||
| : true, | ||
| summary: layout.summary ?? [], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The layouts are not fully normalized:
- For the panel layout, convert it to an array of strings (or empty).
- For the card layout, convert it to an array of objects that encode its visibility. The default visibility is
when-collapsed.
By doing this, everything is simpler: the normalized layout types, there's a single point of normalization (remove code from layout components), and the API supports use cases like this for card layout (tax-summary visibility is when-collapsed, the default):
summary: [ 'tax-summary', { id: 'vat', visibility: 'always' } ] There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! I improved the logic via bb2e019.
|
@gigitux this is working great, thanks. I've left some final comments about API tweaks and implementation details. |
| withHeader, | ||
| summary: [ | ||
| { id: 'customer-summary', visibility: 'always' }, | ||
| { id: 'tax-summary', visibility: 'when-collapsed' }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this is possible (displaying a summary that is unrelated to the card), but it feels a bit weird given that the tax info is in a different card. Can you perhaps move this summary there or change the summary here? Not a strong opinion, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed with 0454fa0. I used plan as the second field.
| sum: CardSummaryField | ||
| ): NormalizedCardSummaryField => { | ||
| if ( typeof sum === 'string' ) { | ||
| return [ { id: sum, visibility: 'always' } ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel the default should be when-collapsed. Always on summaries for card should be rare (e.g., things to act on such as validation, etc.).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small nit: this little utility could take layout.summary as input and be defined outside the normalizeLayout for better readability and performance (so it's only created once, not every time normalizeLayout is called).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed with 00033ed
| withHeader, | ||
| summary: [ | ||
| { id: 'customer-summary', visibility: 'always' }, | ||
| { id: 'plan-summary', visibility: 'when-collapsed' }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be just 'plan-summary' to highlight the default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes sense: 755047d
…ataform-card-layout-allow-having-summarybadge
|
@oandregal I addressed your feedback! Can you take another look? Thanks! 🙇 |
oandregal
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is working nicely, thanks for your work here, Luigi.

What?
Adds
summaryFieldprop to DataForm card layouts that references an existing field definition to display in the header.Why?
Currently, card layouts in DataForm don't provide a way to display summary information in their headers. This makes it difficult for users to quickly scan and understand the content of collapsed cards. By adding support for summary badges, we improve the user experience while maintaining our architectural principle of keeping forms and views declarative (NEX-198).
How?
Summary Fields in Layout Object: Summary fields are now configured within the layout object instead of directly on the field, providing better organization and consistency.
CardLayout: Added optional summary property that accepts:
Testing Instructions
Testing Instructions for Keyboard
Screenshots
Screen.Capture.on.2025-09-10.at.10-44-33.mov