DataForm: Support multiple layouts and introduce the panel layout #64299
Conversation
This file contains 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
youknowriad
added
[Type] Experimental
youknowriad
requested review from
oandregal,
joshuatf,
jorgefilipecosta and
ntsekouras
youknowriad
commented
Aug 6, 2024
| TextProps, | ||
| 'size' | 'isBlock' | 'color' | 'weight' | ||
| > & { | ||
| export type HeadingProps = Omit< TextProps, 'isBlock' | 'color' | 'weight' > & { |
There was a problem hiding this comment.
The "size" definitely impacts the heading rendering, so it's a valid HeadingProps. cc @ciampo
|
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. |
youknowriad
commented
Aug 6, 2024
| @@ -497,3 +499,10 @@ export interface SupportedLayouts { | |||
| grid?: Omit< ViewGrid, 'type' >; | |||
| table?: Omit< ViewTable, 'type' >; | |||
| } | |||
|
|
|||
| export interface DataFormProps< Item > { | |||
There was a problem hiding this comment.
This goes against what I said before about Props being collocated. That said, this one is being used in three places. Maybe we can add the DataViewsProps here as well.
|
Size Change: +1.59 kB (+0.09%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
|
I just realized there were mockups for quick edit at #55101, I assumed the design direction for quick edit was #63624. If we want to support both "layouts", we'd need to clarify how any component works in both. Adapting components to work in two designs is a bit more work and I am slightly concerned about controlling scope for 6.7. If we don't plan to use both layouts immediately (6.7), I'd suggest we focus on a single one for now. The next two fields I plan to add to quick edit ("dates" and "status") are good examples of how "regular" and "panel" aren't always a 1:1 match. For dates, I was working at #64267. In a form-like design, we'd want dates to render along these lines: 
However, if we want to mimick the document inspector, we'd want something more like (note that it's not embedding the "regular" control in a modal, it's a different experience/component): 
|
Thinking on how to make it a 1:1 match or being able to automatically generate a variety of forms based on the existing "status" field in the document inspector: 
|
|
I agree that it's not easy to support both at the same time. The main layout that we need for 6.7 is the "panel" one. But I see the "regular" renderer as something that forces us to think the abstraction properly and avoid optimizing too much for one renderer (panel) and break the abstractions. Also, I believe we'll quickly need the regular form renderer for the work on "DataViews for Tags and Categories and for the Media library". We want forms to create these things. |
ntsekouras
reviewed
Aug 6, 2024
| data={ data } | ||
| field={ field } | ||
| onChange={ onChange } | ||
| hideLabelFromVision |
There was a problem hiding this comment.
Should this be part of the form config?
With @oandregal's comment, I'm not sure if the Edit function needs the form config and render accordingly what is needed, similarly with the render function of fields that are using the viewType. 🤔
There was a problem hiding this comment.
hideLabelFromVision should be enabled by default in this "panel" layout. So for me, no it's not a form config, it's more a config to the "edit" functions (I would prefer to rename these "controls" rather than "edit"). In other words, "controls" should accept a config and be able to render controls with or without label.
|
I updated the storybook story to be able to pick the layout. I think this one is ready for me. |
oandregal
reviewed
Aug 7, 2024
| ) } | ||
| onClick={ onToggle } | ||
| > | ||
| <field.render item={ data } /> |
There was a problem hiding this comment.
Haven't yet looked deeply into the code but isn't this where we render the item for the panel? I wonder why items with elements don't render properly in the storybook (see author as number and status as the lowercase value):
Gravacao.do.ecra.2024-08-07.as.14.19.10.mov
There was a problem hiding this comment.
In the storybook, none of those fields provide a render function, hence they end up using the default getValue (returns the data[id]). This sounds like something to improve: if a field has elements, its default render function should be the label's element, not the value's element.
There was a problem hiding this comment.
This sounds like something to improve: if a field has elements, its default render function should be the label's element, not the value's element.
Indeed that could be a good improvement.
oandregal
reviewed
Aug 7, 2024
youknowriad
force-pushed
the
add/dataform-layout
branch
from
46efc74
to
bb731d7
Compare
|
Fixed the title field. Its render function was not handling the case of edited entities. |
This was referenced Aug 26, 2024
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.
Related #55101
What?
In the quick edit panel that is being developed in the "pages" dataviews of the site editor, we want to use a design that is similar to the design we have in the "document inspector panel" of the block editor.
To do so, the current PR introduces a "type" (or the notion of layouts) to the
DataFormcomponent. This allows this component to be used to render regular forms or panels.Todo
Testing Instructions
1- Enable the "quick edit" experiment.
2- Open the "table" layout of the "pages" dataviews in the site editor
3- Click the "quick edit" toggle
4- Check the design of the panel and use it.