DataForm: Fix SelectControl size and spacing

[youknowriad]

Related #59745

What?

Small PR to improve the consistency of the size and spacing of the controls that are generated by the DataForm component. Specifically this fixes the size and spacing for the "select" control for string fields.

Before

After

Check the size and spacing of the status control.

[github-actions]

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: youknowriad <[email protected]>
Co-authored-by: mirka <[email protected]>
Co-authored-by: oandregal <[email protected]>
Co-authored-by: t-hamano <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[youknowriad]

The other small thing I'm noticing in the screenshots above is that the padding left of the first control (TextControl) is different than the other controls (NumberControl and SelectControl). I think this comes directly from the underlying components. So I'm wondering if you folks are aware of it cc @tyxla @mirka

[t-hamano]

the padding left of the first control (TextControl) is different than the other controls (NumberControl and SelectControl).

This issue has been reported in #57394

[mirka]

No problem if this PR merges first, but FYI these props are being added in bulk to all in-app SelectControls in #64283.

[Zealth57]

Not sure if this requires a separate Github issue or it can be discussed in this thread here... Looking through the docs in data form, the form elements seem like they could be made a little more intuitive. https://wordpress.github.io/gutenberg/?path=/docs/dataviews-dataform--docs. It looks like when trying to generate a dropdown, you pass the 'type' in as 'text' and then it creates a dropdown. It's not clear how to create other elements that use multiple options such as radio and checkboxes. I think the 'type' should match what you want to output, in the case of the documented examples the 'type' for author and status should be something like 'select'. If you wanted the status to be radio you'd do something like 'radio'.

We should also consider accessibility with the more advanced types such as checkboxes and radios. There should be additional properties for things like 'legendLabel' so things can be wrapped in valid markup with a legend.

Lastly, for 'type' of 'integer', I wonder if 'number' is more appropriate since that's the actual type in html5 so as a consumer of the component I'd naturally think of 'number' as the value and not 'integer'.

[youknowriad]

It looks like when trying to generate a dropdown, you pass the 'type' in as 'text' and then it creates a dropdown. It's not clear how to create other elements that use multiple options such as radio and checkboxes. I think the 'type' should match what you want to output, in the case of the documented examples the 'type' for author and status should be something like 'select'. If you wanted the status to be radio you'd do something like 'radio'.

We definitely didn't invest much in docs yet as it's bit early but the idea is that if you pass elements array in the field definition, it becomes a select be it a "text" or an "integer.

If you wanted the status to be radio you'd do something like 'radio'.

Obviously, we'd need this kind of customization but we're starting very small and iterating/adding controls types (radio, toggles, ...) as we implement UIs in Core (quick edit focus for now).

We should also consider accessibility with the more advanced types such as checkboxes and radios. There should be additional properties for things like 'legendLabel' so things can be wrapped in valid markup with a legend.

I agree that we need ways to "group" fields... I've heard a few things already:

• fieldsets
• tabs
• ...

We also need support for things like "description", "help" for fields.

Lastly, for 'type' of 'integer', I wonder if 'number' is more appropriate since that's the actual type in html5 so as a consumer of the component I'd naturally think of 'number' as the value and not 'integer'.

We discussed this with @oandregal already, I think it's not settled but André had some good arguments.

[oandregal]

Lastly, for 'type' of 'integer', I wonder if 'number' is more appropriate since that's the actual type in html5 so as a consumer of the component I'd naturally think of 'number' as the value and not 'integer'.

The type field is used for various things: sorting, validation, and also rendering the control for edit the field. Text and integer work well for all those functions while radio wouldn't. Integers and doubles are also different in terms of validation, for example. The current types aim to match the data type (e.g.: like in the REST API) and not HTML controls/specs.

Of course, we've just started. We will need to build more nuances to decide whether to render a radio or a select, how to group fields together, etc.

[Zealth57]

The REST types are the way they are because they’re independent of form data. You can send data derived from any source to REST and have it validated which is why “Type” works well in that context. When defining “Type” in DataForm, it’s confusing because html form elements have the “type” attribute. Also it’s not intuitive or obvious what input will be output when I put in “type” of “integer”. What if I want a drop down of elements and validate all as integer? I think validation, sorting, all that is totally separate from “Type”. Maybe we should do “validateAs” as a separate property and that’s what gets “integer”.

I know this is early and documentation isn’t done but that doesn’t mean we can’t properly account for future elements. Using type “select” for drop downs now helps make this easier to use and starts to give a pattern for the future. Making text output select now seems like we will have to either make a breaking change later or have a not very intuitive experience as a dev. Documentation is nice but even better is when documentation is not required because something is so easy and intuitive.