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

[EuiDescribedFormGroup] Reintroduce fieldset/legend #3032

Closed
myasonik opened this issue Mar 10, 2020 · 10 comments
Closed

[EuiDescribedFormGroup] Reintroduce fieldset/legend #3032

myasonik opened this issue Mar 10, 2020 · 10 comments

Comments

@myasonik
Copy link
Contributor

Originally, it was div[role=group] with no legend.

Then in #2783 we changed it to use fieldset/legend with some aria-hidden and ScreenReaderOnly gymnastics to solve all the a11y and design issues.

Then in #2888 we discovered we might have made the a11y worse so we largely reverted the change in #2989.

We still want to introduce fieldset and legend into our form layouts but we gotta be careful about how...

Maybe we need to first come up with a plan for #2493 before tackling this? That would, potentially, give us a more prescriptive usage pattern for developers to use which might loosen up the abstract nature of the a11y and design issues...

@myasonik
Copy link
Contributor Author

CC @cchaos @miukimiu

@cchaos
Copy link
Contributor

cchaos commented Mar 12, 2020

I'm not sure this is connected to #2493 since this issue is directed at the EuiDescribedFormGroup which is really input/input row agnostic.

So right now the component uses EuiFlexGroup directly to layout the left/right sides. So we get this HTML:

<div role="group" class="euiDescribedFormGroup">
  <div class="euiFlexGroup">
    <div class="euiFlexItem">
      <h3 class="euiDescribedFormGroup__title">
        <!--- TITLE --->
      </h3>
      <div class="euiDescribedFormGroup__description">
        <!--- DESCRIPTION --->
      </div>
    </div>
    <div class="euiFlexItem">
       <!--- FORM ROW --->
    </div>
  </div>
</div>

We could potentially nix the EuiFlexGroup in favor of writing our own layout styles with our own elements to render something like:

<fieldset class="euiDescribedFormGroup">
  <legend>
    <h3 class="euiDescribedFormGroup__title">
      <!--- TITLE --->
    </h3>
    <div class="euiDescribedFormGroup__description">
      <!--- DESCRIPTION --->
    </div>
  </legend>
  <div class="euiDescribedFormGroup__formRow">
      <!--- FORM ROW --->
  </div>
</fieldset>

@myasonik
Copy link
Contributor Author

Do we want the description in the <legend>?

I imagine the description can be quite long which we might not want reread with every form control.

@cchaos
Copy link
Contributor

cchaos commented Mar 16, 2020

I thought screenreaders only read the legend on the first input that gets focused then as they move through the fieldset, it doesn't reread the legend. At least that's how it's working with Mac's VO.

@myasonik
Copy link
Contributor Author

JAWS, Window's most popular screen reader, adds it after each form element

@cchaos
Copy link
Contributor

cchaos commented Mar 16, 2020

Then, I supposed, yes, you could just wrap the heading and not the description in the <legend>

@cchaos
Copy link
Contributor

cchaos commented Mar 16, 2020

Oh wait, sorry, I forgot this whole chain of thought. Nope, if <legend> has to be the first descendent of <fieldset> and we can't include the description, then my plan fails. 🤷‍♀

@myasonik
Copy link
Contributor Author

Then maybe we do include it in the <legend> and count a potentially long/wordy description as the price we pay for all things we're trying to balance?

I think it would be better than what we have now at the very least...

@cchaos cchaos changed the title Reintroduce fieldset/legend into EuiDescribedFormGroup [EuiDescribedFormGroup] Reintroduce fieldset/legend Mar 18, 2020
@snide snide mentioned this issue Mar 9, 2021
41 tasks
@github-actions
Copy link

👋 Hi there - this issue hasn't had any activity in 6 months. If the EUI team has not explicitly expressed that this is something on our roadmap, it's unlikely that we'll pick this issue up. We would sincerely appreciate a PR/community contribution if this is something that matters to you! If not, and there is no further activity on this issue for another 6 months (i.e. it's stale for over a year), the issue will be auto-closed.

@github-actions
Copy link

❌ Per our previous message, this issue is auto-closing after having been open and inactive for a year. If you strongly feel this is still a high-priority issue, or are interested in contributing, please leave a comment or open a new issue linking to this one for context.

@cee-chen cee-chen closed this as not planned Won't fix, can't repro, duplicate, stale Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants