Avoid divitis

[afercia]

We should avoid to print out too many <div> elements.

See in the screenshot below what, at the moment, the API outputs for a section with two fields:

As I see it, there's no need for both the settings-fields and the settings-field divs. The real need is to distinguish fields that are within a section from the other ones, in order to style them properly. The settings-section already gives us what we need.

Unless I'm missing something, the inner divs could be safely removed.

I'm also concerned about the settings-field-control span element, used as a wrapper for the actual output from the field callback. Styling should be applied directly on the fields. I'm not really sure we need this additional <span>.

Note: when form controls are logically related and can be grouped under a "question" that relates to them all, the wrapper element should be a <fieldset> element with a <legend>. This is 99% true in the case of a group of radio buttons and should be evaluated on a case by case basis for other groups.

[felixarntz]

As I see it, there's no need for both the settings-fields and the settings-field divs. The real need is to distinguish fields that are within a section from the other ones, in order to style them properly. The settings-section already gives us what we need.

Unless I'm missing something, the inner divs could be safely removed.

I don't agree that the above is a valid case of "divitis". While there are quite some nested divs, I think they are necessary to provide the required structure: In the above example, you're right that the settings-fields div could be omitted, but often a settings-section div will include a heading and other elements before the settings-fields div. An additional wrapper like the settings-fields div ensures we can reliably target things like settings-field:first-child: If settings-fields didn't exist, this would not be possible.

I'm also concerned about the settings-field-control span element, used as a wrapper for the actual output from the field callback. Styling should be applied directly on the fields. I'm not really sure we need this additional <span>.

This is necessary in my opinion since we cannot control the output for the fields themselves, since it is produced in callbacks (often by external developers). By providing markup such as the settings-field-control element, we can target the actual fields in a reliable way. I generally agree that we should preferably target the fields, but in a dynamic structure filled by callbacks like that one it is not possible I think.

Note: when form controls are logically related and can be grouped under a "question" that relates to them all, the wrapper element should be a <fieldset> element with a <legend>. This is 99% true in the case of a group of radio buttons and should be evaluated on a case by case basis for other groups.

Are you saying we need to apply <fieldset> elements to some of the settings sections? Something I was always curious about is whether <fieldset>s can be nested. What would happen if a section is a fieldset, but then it contains a radio group (another fieldset)?

[afercia]

I kindly disagree. While I understand the programmatic-oriented approach, this is just not the way markup and CSS should be used. We're basically replicating the same potential issues given by the table cells, wrapping anything within some element.

Ideally, markup should always be minimal. Adding several, not semantic, divs and spans just for the sake of styling would bring us back in time by 15 years or so :) WordPress should always show best practices, and I'm afraid I wouldn't support defining the current output a best practice.

Let's try to have a good balance between ease of use and clean, minimal markup and CSS. Let's analyse what the settings API need in this context.

This project aims to provide standardized callbacks for a set of form fields and allow custom fields. About the former group, we know which fields will be output and we can just target them with CSS. WordPress currently does that using the type selector, and I don't see the need to change that. Say in the future support for other fields will be

About the :first-child argument, there are a number of ways to select the first element within a parent element whether or not it's preceded by a heading, the first one that comes into my mind is :first-of-type but it's also possible to use other selectors like targeting specifically only the elements preceded by a heading (something like heading + element).

About custom fields, we just can't predict what devs will use. I doubt a wrapping <span> can help so much, and since custom fields will be generated by custom callbacks, devs can add their wrappers if they really need them.

Overall, I understand the good intention to cover all the possible cases, but that would be inevitably based on assumptions. Instead, I'd strongly recommend to keep everything as minimal as possible.

[felixarntz]

You're starting to convince me. So as far as I understand you're suggesting the following:

Remove the settings-fields div
My remaining argument to consider leaving in the first is that there can be cases where fields should be printed without a settings section. In that case a settings-section doesn't make sense, and just putting all the fields into somewhere without a wrapper makes it hard to do advanced stuff in JS in a standardized way, such as dynamically adding fields. I don't think this is a strong argument, but I feel like having a settings-fields wrapper element provides more standardization than not having it. Let's think about that further.

Remove the settings-field-control span
I agree to remove that wrapper. However it might make sense to use that class on the actual field instead. We can set it as a default class that is passed to the callback. That would ensure that every Core callback as well as other custom callback using the "pre-provided" arguments (we will document those anyway) would benefit from having a standardized class.

[afercia]

I forgot to answer about the <fieldset> element, see also #5. It is possible to have nested fieldset elements, but it's not recommended. See also this post by Léonie Watson, for more information:
Using the fieldset and legend elements

Basically, the <fieldset> and <legend> elements tell screen readers that a group of form fields relate to each other, and also provide a label for the group.

Depending on the nature of the grouped fields, the quality of their label, and the "question" they ask, using a fieldset+legend might be absolutely necessary or less useful. Consider for example this case (taken from the wp-signup page from a WordPress multisite install):

In this case, tabbing to the radio buttons and then using the arrows will announce just "Yes" and "No". Users won't have any clue about the question they should answer to. The fieldset legend would solve this issue, because it is announced when tabbing into the fieldset.

Worth considering, most screen readers (wrongly?) announce the legend for any field inside the fieldset so it's better to avoid a very long legend text and keep it as short as possible.

In other cases, using a fieldset+legend might still make sense but if the question asked is already clear enough from the context and the field labels, it's less useful and could also introduce too much noise. It depends on a case by case basis. See the second example in the post linked above:

Without the <legend>Your address</legend> it wouldn't be so clear what "Street", "Town or city", etc. refers to.

In the following example instead, I've intentionally added a "Personal details" legend. Users get this page after they click on a "Add new user" link and the page already has a "Add New User" heading:

In this case, I'd say having the legend text "Personal details" repeated for each field:

Username (required) Personal details
Email (required) Personal details
First Name Personal details
etc.

would be noisy and a bit redundant, since "the question to answer to" and the context are already clear.

Back to the current output, I think this condition that prints out a fieldset or a div should be improved a bit:
$wrap = ! empty( $field['args']['fieldset'] ) ? 'fieldset' : 'div';
When there's no need for a fieldset, I'd say the API shouldn't print out anything. Instead, it is printing out a div that's used just to add some top padding. I'm pretty sure we can use just CSS for this, without any additional markup.

[afercia]

Remove the settings-field-control span
I agree to remove that wrapper. However it might make sense to use that class on the actual field instead.

I'd agree and personally prefer to target elements in the simplest possible way: with classes. I'm not sure we all agreed during yesterday's meeting on Slack though 🙂

[afercia]

Couple quick notes:

• removing the settings-field-control should also imply removing thins like <span class="radio-item"> 😉
• maybe the name settings-field (if we keep it) is a bit misleading, as I'd tend to get it as an HTML single form field element rather than an API "field" (i.e. option) that can contain multiple elements (e.g. radio buttons, checkboxes, descriptions, links)

[felixarntz]

Your explanations regarding when to use a fieldset make complete sense. I think we should keep fieldset support of the Settings API on a "settings-field" level and not support it for settings sections. Settings sections use headings to group fields, but using fieldsets won't make sense for most of these groups (as their legend would always be announced). I'd suggest we handle groups of fields that require a fieldset in a single field callback (as we currently do with radio buttons / multiple checkboxes, and the "address" field from your above example could be handled like that as well).

When there's no need for a fieldset, I'd say the API shouldn't print out anything. Instead, it is printing out a div that's used just to add some top padding. I'm pretty sure we can use just CSS for this, without any additional markup.

I think we should print always print an element at that location, whether it's a fieldset or not. The tree structure of classes should be similar for all fields and not dependent on whether a field is semantically different from another. Applying the margin is already an argument why leaving the div out would make styling less structural.

[felixarntz]

Summarizing today's discussion (see https://wordpress.slack.com/archives/C02RQBWTW/p1496073615671126), we agreed on using the following structure:

> div.settings-section
	> h2
	> div.settings-fields
		> (div|fieldset).settings-field
			> (span|legend).settings-field-title
			> (input|select|textarea|...).settings-field-control
		> (div|fieldset).settings-field
		> (div|fieldset).settings-field
> div.settings-section
> div.settings-section

Class names are subject to change (and they will change), but this is the markup / nesting levels planned.

#22 and #23 will handle the changes to ensure the above markup.

[felixarntz]

Closing this through #32.

Let's continue discussion through more specific issues, such as #37.