Conversation
… positions to 'none' for better UI consistency.
…nberg into add/dataviews-card-layout
…nberg into add/dataviews-card-layout
|
Flaky tests detected in b119908. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/17495985145
|
9f8cb83 to
f9e1609
Compare
|
Size Change: +699 B (+0.04%) Total Size: 1.93 MB
ℹ️ View Unchanged
|
dinhtungdu
left a comment
There was a problem hiding this comment.
The PR is testing well on my end. I have a question: At this point, we have multiple layouts, and the logic of those layouts shares many similarities. Is it worth some extraction (in future PRs)?
I also found some issues:
Are the double labels here expected? No, right?
I think we need a basic responsive layout here. On mobile, the field should be full-width.
| const LayoutRowComponent = ( { | ||
| alignment, | ||
| }: { | ||
| alignment: 'none' | 'start' | 'center' | 'end'; |
There was a problem hiding this comment.
Row layout type doesn't has none option. I think we need to use the same type here, right?
There was a problem hiding this comment.
This file is empty, is that expected?
| return ( | ||
| <VStack className="dataforms-layouts-row__header" spacing={ 4 }> | ||
| <HStack alignment="center"> | ||
| <Heading level={ 2 } size={ 13 }> |
There was a problem hiding this comment.
Is the heading here always at the same level and size, even when rows can be nested? I can see that the Regular layout has the same implementation, so this is not a question specifically for Row.
packages/dataviews/src/types.ts
Outdated
| }; | ||
| export type NormalizedRowLayout = { | ||
| type: 'row'; | ||
| spacing: number; |
There was a problem hiding this comment.
| spacing: number; |
|
@oandregal Thanks for your help! For me, this approach works well even if this does not make the wrapper customizable for third-party layouts, but we can iterate. I think that removing the
@dinhtungdu I agree, but I’d suggest shipping this PR as is and handling layout improvements in follow-ups. Adding wrap affects current stories since inputs are set to 100% width: Screen.Capture.on.2025-09-05.at.12-45-08.mp4I’d merge this PR, gather feedback, and expand layout options based on real use cases. Thoughts? |
@gigitux Sounds good to me! |
Can you expand on this? I'd like to get clarity on this before shipping. We don't have any 3rd party layout for now, but the idea is layouts provide |
dinhtungdu
left a comment
There was a problem hiding this comment.
With the latest updates, this is LGTM! Thank you so much for working on this!
Sorry for not being clear (I'm still recovering from the flu 😅). I intended to say that this is a first step toward allowing third party layouts, but we need to figure out what the API will look like and we can do in the upcoming iterations.
Make sense! 💯 |
| @@ -0,0 +1,3 @@ | |||
| .dataforms-layouts-row__field-control { | |||
There was a problem hiding this comment.
Hey @gigitux I don't see this file being imported anywhere. This is causing the UI of the elements to not take the full width. I did a [local override] for now but I think it is better to import this file to the adjacent index.tsx file. cc: @ismaeldcom
There was a problem hiding this comment.
The file is imported correctly here.
Maybe I think that you need to import dataviews CSS in your SCSS file with:
@use "@wordpress/dataviews/build-style/style.css" as dataviews;
What
This PR introduces a new layout for the DataForm component. The row layout allows grouping fields horizontally in a single line.
How
Add 'row' as a new layout type alongside existing 'regular', 'panel', and 'card'.
The row layout supports:
Example
Screen.Capture.on.2025-08-27.at.14-28-06.mov