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

Fix issue with fieldset and css grid #953

Closed
matthewcarleton opened this issue Nov 17, 2018 · 5 comments
Closed

Fix issue with fieldset and css grid #953

matthewcarleton opened this issue Nov 17, 2018 · 5 comments
Labels
wontfix This will not be worked on

Comments

@matthewcarleton
Copy link
Contributor

matthewcarleton commented Nov 17, 2018

Currently when using fieldset in our horizontal grid the layout is broken (this seems to be a bug with css grid) so the following code:

<fieldset class="pf-c-form__group">
         <legend class="pf-c-form__label">Can we follow up via email?</legend>
    <div class="pf-c-form__horizontal-group">
      <div class="pf-c-check">
                  <input class="pf-c-check__input" type="radio" id="horizontal-radio1" name="horizontal-radios">
<label class="pf-c-check__label" for="horizontal-radio1">Yes</label>
       </div>
      <div class="pf-c-check">
                  <input class="pf-c-check__input" type="radio" id="horizontal-radio2" name="horizontal-radios">
       <label class="pf-c-check__label" for="horizontal-radio2"> No</label>
        </div>
      </div>
  </fieldset>

Is this:

<div class="pf-c-form__group">
         <label class="pf-c-form__label" for="horizontal-radio1">Can we follow up via email?</label>
    <div class="pf-c-form__horizontal-group">
      <div class="pf-c-check">
                  <input class="pf-c-check__input" type="radio" id="horizontal-radio1" name="horizontal-radios">
<label class="pf-c-check__label" for="horizontal-radio1">Yes</label>
       </div>
      <div class="pf-c-check">
                  <input class="pf-c-check__input" type="radio" id="horizontal-radio2" name="horizontal-radios">
       <label class="pf-c-check__label" for="horizontal-radio2"> No</label>
        </div>
      </div>
  </div>

This works but causes an issue with a11y. The first <label> has the same id as the first .pf-c-check <label> which fails out a11y check. We need to investigate the best way to fix this.
#949 removes the offending for=horizontal-radio1 from the first <label> but will now causes this to fail in a screen reader.

@jgiardino
Copy link
Contributor

My recommendation would be to apply the class pf-c-form__horizontal-group to the <fieldset> element and apply the sr-only styles to the <legend>.

The visible label that uses the class pf-c-form__label could just be a <span>.

@mcoker mcoker added the P3 label Apr 12, 2019
@jgiardino
Copy link
Contributor

As an alternate suggestion to my previous comment, the <fieldset> could also be labeled by the span.pf-c-form__label instead of including the <legend> element. This might be the cleaner solution.

For example:

<fieldset class="pf-c-form__group" aria-labelledby="fieldset-label">
    <span class="pf-c-form__label" id="fieldset-label">Can we follow up via email?</span>
    <div class="pf-c-form__horizontal-group">
        <div class="pf-c-check">
            <input class="pf-c-check__input" type="radio" id="horizontal-radio1" name="horizontal-radios">
            <label class="pf-c-check__label" for="horizontal-radio1">Yes</label>
       </div>
        <div class="pf-c-check">
        </div>
    </div>
</fieldset>

@stale
Copy link

stale bot commented Jul 29, 2020

This issue has been automatically marked as stale because it has not had activity in the last 60 days. It will be closed in 30 days if no further activity occurs.

@stale stale bot added the wontfix This will not be worked on label Jul 29, 2020
@mcoker
Copy link
Contributor

mcoker commented Aug 24, 2020

leave open

@stale
Copy link

stale bot commented Oct 24, 2020

This issue has been automatically marked as stale because it has not had activity in the last 60 days. It will be closed in 30 days if no further activity occurs.

@stale stale bot added the wontfix This will not be worked on label Oct 24, 2020
@stale stale bot closed this as completed Dec 5, 2020
@mcarrano mcarrano removed this from Unplanned in PatternFly Feature Roadmap Jan 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants