Described form group a11y

[elizabetdev]

Summary

Closes #762

When using just one EuiFormRow inside a EuiDescribedFormGroup the EuiDescribedFormGroup description text was served as the help text for the input. This was causing the text to be read twice by screen readers.

Once when users were reading the description and again when users are inside the input. This input was getting an aria-describedby with the id of the EuiDescribedFormGroup description.

I think it's not a good idea to use the EuiDescribedFormGroup description prop as a way to display help text for the input. If users want to create a help text for the input they should use a EuiFormRow help-text prop or an aria-label on the form element.

The EuiDescribedFormGroup description should be used to provide an additional text and not as a help text for a form element.

Breaking change

• Removed idAria prop from EuiDescribedFormGroup

Suggested upgrade

• Remove idAria prop usage on any EuiDescribedFormGroup components being used in your application.

Checklist

• Check against all themes for compatibility in both light and dark modes
• Checked in mobile
• Checked in IE11 and Firefox
• Props have proper autodocs
• Added documentation examples
• Added or updated jest tests
• Checked for breaking changes and labeled appropriately
• Checked for accessibility including keyboard-only and screenreader modes
• A changelog entry exists and is marked appropriately

[myasonik]

Will this cause a breaking change in Kibana?

My comments are for further improvements but shouldn't block this PR. If they can't get into this PR, let's make a new issue for them.

Also, reading #762, I think there's a good suggestion in there to limit EuiTitle to only allow h1 - h6 children. If we're going to close that ticket, we should make a new one for that as well.

[andreadelrio]

I'll leave the review of the a11y stuff to Michael. Just added a couple of small suggestions.

Also, now that you're in that code maybe we can change the size here (title of the "Multiple fields" example) to s. Right now "Multiple Fields" kind of looks like it's another section (at 28px) and at least for me, since it was the first time looking at that page in our docs it was confusing

to this

[snide]

Agree with @myasonik. Fieldset feels better if possible. Removing the prop will make this a technical breaking change. When we're removed a prop like this we've generally flipped a coin as to whether we mark it as breaking since the downstream components would still work fine and at works will spit out a console error. This is used so commonly in Kibana though that it's probably best to mark it as such. When you get ready to merge this please write up the upgrade path in your PR description. I'd assume that should simply be "Remove the idAria prop...etc"

[snide]

LGTM. Did another review of the code and tested the output in a Screen reader.

[cchaos]

@miukimiu Quick question, why is the title prop needed here?

[elizabetdev]

I copied the code from the InnerText snippet and I must have missed it. I can remove it on #2810 or create a different PR.

[cchaos]

Cool, yeah I think in #2810 would be fine