Inputfield::setAttributeInMarkup() shouldn't be used in some cases

chriswthomson commented 3 days ago

We use the Inputfield/InputfieldForm classes to render front-end HTML forms, amending the markup/classes options to fit.

We had the following set for InputfieldRadios and InputfieldCheckboxes:

['item_label' => '<label class="uk-form-label{class}">{out}</label>']

This was to prevent the 'for' attribute being output on the label as it generates this HTML validation error:

Error: The value of the for attribute of the label element must be the ID of a non-hidden form control.

The 'for' attribute for these labels don't match an input with that id.

From what I can see, the culprit is the recently added InputfieldWrapper::setAttributeInMarkup(), which is called on all visible labels regardless of the markup string (line 901).

There are two existing overrides, skipLabel and quietMode, but neither of these are sufficient - we want to render the label, and we might need the additional things quiteMode suppresses.

I'm not sure the best solution here, but perhaps a 'noLabelFor' setting on the wrapper which the Radios and Checkboxes Inputfields could set automatically, which could prevent the forcing of the for attribute?

ryancramerdesign commented 11 hours ago

@chriswthomson I'm not sure I fully understand. Can you tell me how to reproduce the issue with a simple test case of some sort? I'm just looking for some way to observe it. Thanks.

chriswthomson commented 10 hours ago

Hi @ryancramerdesign,

If you take a look at Page/Radios field's HTML, you'll see something like:

<li class="Inputfield InputfieldPage Inputfield_field_name collapsed5 InputfieldNoFocus uk-grid-margin uk-first-column InputfieldColumnWidthFirst InputfieldStateWasCollapsed" id="wrap_Inputfield_field_name" style="">
    <label class="InputfieldHeader uk-form-label InputfieldStateToggle" for="Inputfield_field_name">
        Label<i title="Toggle open/close" class="toggle-icon fa fa-fw fa-angle-down" data-to="fa-angle-down fa-angle-right"></i>
    </label>
    <div class="InputfieldContent uk-form-controls">
        <div class="InputfieldRadios">
            <ul class="InputfieldRadiosFloated pw-clearfix ui-helper-clearfix">
                ...radios
            </ul>
        </div>
    </div>
</li>

The for="Inputfield_field_name" on the main label shouldn't be there. There isn't an input with that ID.

processwire / processwire-issues

Inputfield::setAttributeInMarkup() shouldn't be used in some cases #1982