Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop useless text faking a label in horizontal forms example #31904

Merged
merged 4 commits into from Oct 19, 2020

Conversation

ffoodd
Copy link
Member

@ffoodd ffoodd commented Oct 15, 2020

Because those example are often used as sources through copy-pasting, avoid using fake labels with text in divs seems a best practice in our docs.

We might as well turn it into a label but this could lead to wrong pattern usage too, since it would require two pertinent labels.
Since we're only documenting horizontal layout using grid, switching this an offest use case feels better.

If you agree with that, it should be backported to v4 as well.

@patrickhlauke
Copy link
Member

patrickhlauke commented Oct 15, 2020

i'm actually wondering...as there's an invalid fieldset/legend for the radios there before it, if we could do two things:

  • fix up the fieldset/legend to be valid (the legend needs to be the first child of the fieldset
<fieldset class="row mb-3">
  <legend class="col-form-label col-sm-2 pt-0">Radios</legend>
  <div class="col-sm-10">
    <div class="form-check">
      <input class="form-check-input" type="radio" name="gridRadios" id="gridRadios1" value="option1" checked="">
      <label class="form-check-label" for="gridRadios1">
        First radio
      </label>
    </div>
    <div class="form-check">
      <input class="form-check-input" type="radio" name="gridRadios" id="gridRadios2" value="option2">
      <label class="form-check-label" for="gridRadios2">
        Second radio
      </label>
    </div>
    <div class="form-check disabled">
      <input class="form-check-input" type="radio" name="gridRadios" id="gridRadios3" value="option3" disabled="">
      <label class="form-check-label" for="gridRadios3">
        Third disabled radio
      </label>
    </div>
  </div>
</fieldset>
  • use the same approach for the checkbox example, i.e. something like
<fieldset class="row mb-3">
  <legend class="col-form-label col-sm-2 pt-0">Checkbox</legend>
  <div class="col-sm-10">
    ...
  </div>
</fieldset>

@patrickhlauke
Copy link
Member

randomly, i think there were other instances of borked fieldset/legend constructs somewhere that came up during automated scans ... maybe those can be tackled in a similar way?

@ffoodd
Copy link
Member Author

ffoodd commented Oct 15, 2020

@patrickhlauke Indeed!

FWIW, the "checkbox" one would feel very wrong to me, using fieldset and legend for a single checkbox. And as I said, it's an opportunity to show an .offset-* utility too, since this specific example is for grid usage with forms.

But I'll try to tackle fieldsets accordingly, the report on @XhmikosR' PR should help to spot other borked occurrences.

@ffoodd
Copy link
Member Author

ffoodd commented Oct 15, 2020

Only found a single occurrence flagged by pa11y-ci, in docs/overview#disabled where we used an aria-label instead of a fieldset. Even though I know it's pretty good from an accessibility perspective, a fieldset feels better in our docs.

Easy to revert a commit if I'm wrong ;)

@XhmikosR XhmikosR added this to Inbox in v5.0.0-alpha3 via automation Oct 19, 2020
v5.0.0-alpha3 automation moved this from Inbox to Approved Oct 19, 2020
@XhmikosR XhmikosR merged commit e0a3b7e into main Oct 19, 2020
v5.0.0-alpha3 automation moved this from Approved to Shipped Oct 19, 2020
@XhmikosR XhmikosR deleted the main-fod-docs-horizontal-forms branch October 19, 2020 08:38
@XhmikosR
Copy link
Member

This will probably need a manual backport, any help is welcome :)

@XhmikosR XhmikosR added this to Inbox in v4.6.0 via automation Oct 19, 2020
@XhmikosR XhmikosR added the v4 label Oct 19, 2020
ffoodd added a commit that referenced this pull request Oct 19, 2020
ffoodd added a commit that referenced this pull request Oct 19, 2020
* docs(forms): use a legend for fieldset instead of aria-label

* docs(forms): fix incorrect legend nesting in fieldset
@XhmikosR XhmikosR moved this from Inbox to Cherry-picked/Manually backported in v4.6.0 Oct 19, 2020
XhmikosR pushed a commit that referenced this pull request Oct 20, 2020
* docs(forms): use a legend for fieldset instead of aria-label

* docs(forms): fix incorrect legend nesting in fieldset
XhmikosR pushed a commit that referenced this pull request Oct 27, 2020
* docs(forms): use a legend for fieldset instead of aria-label

* docs(forms): fix incorrect legend nesting in fieldset
XhmikosR pushed a commit that referenced this pull request Oct 27, 2020
* docs(forms): use a legend for fieldset instead of aria-label

* docs(forms): fix incorrect legend nesting in fieldset
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
v4.6.0
Shipped
v5.0.0-alpha3
  
Shipped
Development

Successfully merging this pull request may close these issues.

None yet

3 participants