Core will wrap radios and checkboxes in a fieldset. This is important because assistive technology (such as screen readers) uses the fieldset legend to label the grouping of radios and checkboxes.

Problem/Motivation

The other type fields for radios and checkboxes do not receive the same type of wrapping fieldset as the radios and checkboxes elements in core.

Proposed resolution

Wrap the child elements in a fieldset, similar to how core does it.

I put together a sample module to fix the issue. It might be helpful as a reference for fixing the issue in this module. https://github.com/mfairchild365/a11yformsfix

CommentFileSizeAuthor
#16 2951002-16.patch732 bytesjrockowitz
#15 2951002_cosmetic_issue.png43.16 KBAnonymous (not verified)
#7 2951002-7.patch10.56 KBjrockowitz

Comments

mfairchild365 created an issue. See original summary.

jrockowitz’s picture

jrockowitz commented

First off, I am totally open to any accessibility improvements. Someone may have to take the lead and start providing patches to improve accessibility.

Since the Webform module's other type elements contain radios and checkboxes that are already wrapped in a fieldset will adding a parent fieldset to these element's make things even more confusing for screen readers. Keep in mind that the Webform module can't change how core handles radios and checkboxes.

mfairchild365’s picture

mfairchild365 commented

Great questions. If you look at the module that I created, it applies the `preRenderCompositeFormElement()` trait to the other type elements and removes the default label from displaying. Core (for some reason) does not seem to wrap the radio buttons within the other type with a fieldset, so you only end up with one fieldset. So it isn't an issue.

I happy to help contribute a fix, but I will need some help in determining the best way to accomplish this. I'm not sure if my module is doing it correctly.

This is the resulting resulting HTML from my module (notice that there is only one fieldset):

<fieldset data-drupal-selector="edit-radios-other" id="edit-radios-other--wrapper" class="fieldgroup form-composite required js-form-item form-item js-form-wrapper form-wrapper">
      <legend>
    <span class="fieldset-legend js-form-required form-required">radios other</span>
  </legend>
  <div class="fieldset-wrapper">
            

<!-- THEME DEBUG -->
<!-- THEME HOOK: 'form_element' -->
<!-- BEGIN OUTPUT from 'themes/acbnebraska/templates/form/form-element.html.twig' -->
<div class="js-webform-radios-other webform-radios-other js-form-item form-item js-form-type-webform-radios-other form-type-webform-radios-other js-form-item-radios-other form-item-radios-other dcf-c-form-group">
        

<!-- THEME DEBUG -->
<!-- THEME HOOK: 'radios' -->
<!-- BEGIN OUTPUT from 'themes/acbnebraska/templates/form/radios.html.twig' -->
<div id="edit-radios-other-radios" class="js-webform-radios webform-options-display-one-column form-radios">

<!-- THEME DEBUG -->
<!-- THEME HOOK: 'form_element' -->
<!-- BEGIN OUTPUT from 'themes/acbnebraska/templates/form/form-element.html.twig' -->
<div class="js-form-item form-item js-form-type-radio form-type-radio js-form-item-radios-other-radios form-item-radios-other-radios dcf-c-form-group">
        

<!-- THEME DEBUG -->
<!-- THEME HOOK: 'input__radio' -->
<!-- FILE NAME SUGGESTIONS:
   x input--radio.html.twig
   x input--radio.html.twig
   * input.html.twig
-->
<!-- BEGIN OUTPUT from 'themes/acbnebraska/templates/form/input--radio.html.twig' -->
<input data-drupal-selector="edit-radios-other-radios-one" type="radio" id="edit-radios-other-radios-one" name="radios_other[radios]" value="one" class="form-radio dcf-c-input-control">
<!-- END OUTPUT from 'themes/acbnebraska/templates/form/input--radio.html.twig' -->


        

<!-- THEME DEBUG -->
<!-- THEME HOOK: 'form_element_label' -->
<!-- BEGIN OUTPUT from 'themes/acbnebraska/templates/form/form-element-label.html.twig' -->
<label for="edit-radios-other-radios-one" class="option dcf-c-label">one</label>
<!-- END OUTPUT from 'themes/acbnebraska/templates/form/form-element-label.html.twig' -->


      </div>

<!-- END OUTPUT from 'themes/acbnebraska/templates/form/form-element.html.twig' -->



<!-- THEME DEBUG -->
<!-- THEME HOOK: 'form_element' -->
<!-- BEGIN OUTPUT from 'themes/acbnebraska/templates/form/form-element.html.twig' -->
<div class="js-form-item form-item js-form-type-radio form-type-radio js-form-item-radios-other-radios form-item-radios-other-radios dcf-c-form-group">
        

<!-- THEME DEBUG -->
<!-- THEME HOOK: 'input__radio' -->
<!-- FILE NAME SUGGESTIONS:
   x input--radio.html.twig
   x input--radio.html.twig
   * input.html.twig
-->
<!-- BEGIN OUTPUT from 'themes/acbnebraska/templates/form/input--radio.html.twig' -->
<input data-drupal-selector="edit-radios-other-radios-two" type="radio" id="edit-radios-other-radios-two" name="radios_other[radios]" value="two" class="form-radio dcf-c-input-control">
<!-- END OUTPUT from 'themes/acbnebraska/templates/form/input--radio.html.twig' -->


        

<!-- THEME DEBUG -->
<!-- THEME HOOK: 'form_element_label' -->
<!-- BEGIN OUTPUT from 'themes/acbnebraska/templates/form/form-element-label.html.twig' -->
<label for="edit-radios-other-radios-two" class="option dcf-c-label">two</label>
<!-- END OUTPUT from 'themes/acbnebraska/templates/form/form-element-label.html.twig' -->


      </div>

<!-- END OUTPUT from 'themes/acbnebraska/templates/form/form-element.html.twig' -->



<!-- THEME DEBUG -->
<!-- THEME HOOK: 'form_element' -->
<!-- BEGIN OUTPUT from 'themes/acbnebraska/templates/form/form-element.html.twig' -->
<div class="js-form-item form-item js-form-type-radio form-type-radio js-form-item-radios-other-radios form-item-radios-other-radios dcf-c-form-group">
        

<!-- THEME DEBUG -->
<!-- THEME HOOK: 'input__radio' -->
<!-- FILE NAME SUGGESTIONS:
   x input--radio.html.twig
   x input--radio.html.twig
   * input.html.twig
-->
<!-- BEGIN OUTPUT from 'themes/acbnebraska/templates/form/input--radio.html.twig' -->
<input data-drupal-selector="edit-radios-other-radios-other-" type="radio" id="edit-radios-other-radios-other-" name="radios_other[radios]" value="_other_" class="form-radio dcf-c-input-control">
<!-- END OUTPUT from 'themes/acbnebraska/templates/form/input--radio.html.twig' -->


        

<!-- THEME DEBUG -->
<!-- THEME HOOK: 'form_element_label' -->
<!-- BEGIN OUTPUT from 'themes/acbnebraska/templates/form/form-element-label.html.twig' -->
<label for="edit-radios-other-radios-other-" class="option dcf-c-label">Other...</label>
<!-- END OUTPUT from 'themes/acbnebraska/templates/form/form-element-label.html.twig' -->


      </div>

<!-- END OUTPUT from 'themes/acbnebraska/templates/form/form-element.html.twig' -->

</div>

<!-- END OUTPUT from 'themes/acbnebraska/templates/form/radios.html.twig' -->



<!-- THEME DEBUG -->
<!-- THEME HOOK: 'form_element' -->
<!-- BEGIN OUTPUT from 'themes/acbnebraska/templates/form/form-element.html.twig' -->
<div class="js-webform-radios-other-input webform-radios-other-input js-form-item form-item js-form-type-textfield form-type-textfield js-form-item-radios-other-other form-item-radios-other-other form-no-label dcf-c-form-group" style="display: none;">
        

<!-- THEME DEBUG -->
<!-- THEME HOOK: 'input__textfield' -->
<!-- FILE NAME SUGGESTIONS:
   x input--textfield.html.twig
   x input--textfield.html.twig
   * input.html.twig
-->
<!-- BEGIN OUTPUT from 'themes/acbnebraska/templates/form/input--textfield.html.twig' -->
<input data-drupal-selector="edit-radios-other-other" type="text" id="edit-radios-other-other" name="radios_other[other]" value="" size="60" maxlength="128" placeholder="Enter other..." class="form-text dcf-c-input-text">
<!-- END OUTPUT from 'themes/acbnebraska/templates/form/input--textfield.html.twig' -->


        </div>

<!-- END OUTPUT from 'themes/acbnebraska/templates/form/form-element.html.twig' -->


        </div>

<!-- END OUTPUT from 'themes/acbnebraska/templates/form/form-element.html.twig' -->


          </div>
</fieldset>
andrewmacpherson’s picture

andrewmacpherson commented
Issue tags: -a11y

accessibility is the preferred tag

jrockowitz’s picture

jrockowitz commented

@mfairchild365 Tweaking the rendered output could break some people implementation and the same time fixing accessibility issue would just justify making these types of changes.

If you are for the task, let's start out with a patch. Please make sure to see how the Bootstrap base theme handles your changes. You should also expect to be fixing a few broken tests.

jrockowitz’s picture

jrockowitz commented
Related issues: +#2951714: radio buttons and checkboxes should be grouped by fieldsets
jrockowitz’s picture

jrockowitz commented
Status: Active » Needs review
StatusFileSize
new 2951002-7.patch10.56 KB

@mfairchild365 Please review the attached patch.

This change is going to break webforms and websites with custom CSS selectors which target the 'webform-TYPE-other' elements.

If we all agree that it is worth making this accessibility improvement, then all we need to do is write a change record that explains why we are making this improvement.

In other tickets we might want to look at applying the same type of change to any webform or core element that groups sub-elements. This would include composites, email confirm, password confirm, datelist, datetime, etc...

mfairchild365’s picture

mfairchild365 commented

@jrockowitz your patch looks good to me! :) I also noticed that the other input[type="text"] doesn't have a valid label associated with it by default (you have to define one). A placeholder attribute is applied by default with the text "enter other...", perhaps an aria-label with the same value can be used if one is not explicitly defined? Let me know if I should create a different issue for this.

jrockowitz’s picture

jrockowitz commented

@mfairchild365 Please create a new issue for the aria-label.

Can you please create the 'Change record' with a decent explanation to why we are making this change?

If we link to https://www.w3.org/WAI/tutorials/forms/grouping/ in the change record it is going to set that stage for other accessibility related changes.

mfairchild365’s picture

mfairchild365 commented

I created the change record. Please review and let me know if anything should be added/changed. I'm not familiar with the process and haven't done this before.

jrockowitz’s picture

jrockowitz commented
Status: Needs review » Reviewed & tested by the community

@mfairchild365 That is perfect.

I am going to mark this RTBC but I can't apply the patch until I tag hotfix release in the next few days.

  • jrockowitz committed 47dcd40 on 8.x-5.x 
    Issue #2951002 by jrockowitz: other type fields should be wrapped with a...
jrockowitz’s picture

jrockowitz commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new 2951002_cosmetic_issue.png43.16 KB

This fix seems to add not only a fieldset on the whole component (e.g. id="edit-software-requirements--wrapper" ) but also around the checkboxes--wrapper (e.g. id="edit-software-requirements-checkboxes--wrapper" ). You can see this in the html below. By changing the fieldset with the checkboxes to a div, our cosmetic issue (see screen-shot below) goes away. Is there a reason we have a fieldset in a fieldset? While no cosmetic issue crops up in bootstrap vanilla, we get it in our boot-strap based theme https://github.com/wet-boew/wet-boew/

Cosmetic issue with boot-strap based theme due to fix

<fieldset data-drupal-selector="edit-software-requirements" class="js-webform-checkboxes-other webform-checkboxes-other fieldgroup form-composite js-form-item form-item js-form-wrapper form-wrapper" id="edit-software-requirements--wrapper">
    <legend>
        <span class="fieldset-legend">Select as many software as needed for your project.</span>
    </legend>
    <div class="fieldset-wrapper">
        <fieldset data-drupal-selector="edit-software-requirements-checkboxes" class="fieldgroup form-composite js-webform-type-checkboxes webform-type-checkboxes js-form-item form-item js-form-wrapper form-wrapper" id="edit-software-requirements-checkboxes--wrapper">
            <legend>
                <span class="visually-hidden fieldset-legend">Select as many software as needed for your project.</span>
            </legend>
            <div class="fieldset-wrapper">
                <div id="edit-software-requirements-checkboxes" class="js-webform-checkboxes webform-options-display-one-column form-checkboxes">
                    <div class="form-item js-form-item form-type-checkbox js-form-type-checkbox form-item-software-requirements-checkboxes-sas js-form-item-software-requirements-checkboxes-sas checkbox">

                        <label for="edit-software-requirements-checkboxes-sas" class="control-label option">
                            <input data-drupal-selector="edit-software-requirements-checkboxes-sas" class="form-checkbox" id="edit-software-requirements-checkboxes-sas" name="software_requirements[checkboxes][SAS]" value="SAS" type="checkbox">SAS</label>

                    </div>
                    <div class="form-item js-form-item form-type-checkbox js-form-type-checkbox form-item-software-requirements-checkboxes-stata js-form-item-software-requirements-checkboxes-stata checkbox">

                        <label for="edit-software-requirements-checkboxes-stata" class="control-label option">
                            <input data-drupal-selector="edit-software-requirements-checkboxes-stata" class="form-checkbox" id="edit-software-requirements-checkboxes-stata" name="software_requirements[checkboxes][STATA]" value="STATA" type="checkbox">STATA</label>

                    </div>
                    <div class="form-item js-form-item form-type-checkbox js-form-type-checkbox form-item-software-requirements-checkboxes--other- js-form-item-software-requirements-checkboxes--other- checkbox">

                        <label for="edit-software-requirements-checkboxes-other-" class="control-label option">
                            <input data-drupal-selector="edit-software-requirements-checkboxes-other-" class="form-checkbox" id="edit-software-requirements-checkboxes-other-" name="software_requirements[checkboxes][_other_]" value="_other_" type="checkbox">Other...</label>

                    </div>
                </div>

            </div>
        </fieldset>
        <div class="js-webform-checkboxes-other-input webform-checkboxes-other-input form-item js-form-item form-type-textfield js-form-type-textfield form-item-software-requirements-other js-form-item-software-requirements-other form-group" style="display: none;">
            <label for="edit-software-requirements-other" class="control-label">Provide name of software and version.</label>

            <input data-drupal-selector="edit-software-requirements-other" class="form-text form-control" id="edit-software-requirements-other" name="software_requirements[other]" value="" size="60" maxlength="128" placeholder="Enter other..." type="text">

        </div>

    </div>
</fieldset>
jrockowitz’s picture

jrockowitz commented
Status: Closed (fixed) » Needs review
StatusFileSize
new 2951002-16.patch732 bytes

The attached patch removes the nested fieldsets.

  • jrockowitz committed ebc1092 on 8.x-5.x 
    Issue #2951002 by jrockowitz, TJPope: other type fields should be...
jrockowitz’s picture

jrockowitz commented
Status: Needs review » Fixed

I committed the patch. Please download the latest dev release to review.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.