Add predictable ID for help fields, aria describedby attributes #2386

ArnoSaine · 2021-05-19T19:53:51Z

Reasons for making this change

Make screen reader read field errors, description and help text.

Fixes #959

Checklist

I'm updating documentation
- I've checked the rendering of the Markdown text I've added
I'm adding or updating code
- I've added and/or updated tests
- I've updated docs if needed
I'm adding a new feature
- I've updated the playground with an example use of the feature

Deploy preview

Once CI finishes, you should be able to view a deploy preview of the playground from this PR at this link: https://deploy-preview-[PR_NUMBER]--rjsf.netlify.app/

epicfaace · 2021-05-19T22:58:06Z

Thanks for the PR. I wonder if we can update the changes here with the latest changes just added in #2360. Specifically, can you make sure that all the help / description / title field ids are using the id utility functions you defined in utils.js? Specifically, we might need to change packages/semantic-ui/src/FieldTemplate/FieldTemplate.js, for example.

Can we also add snapshot tests as needed for the affected themes, because now the help fields have different ids?

- Add aria-describedby and id attributes - Use util to create `${id}__description` values

- Add more aria-describedby attributes - Fix use of same id in FieldTemplate

ArnoSaine · 2021-05-21T00:03:34Z

Thank you for the suggestions. I searched for other mentions of __description, __help and __errors. packages/semantic-ui/src/FieldTemplate/FieldTemplate.js was the only one not using the utility function. it's now fixed.

I updated the affected snapshots and added more material-ui updates.

packages/core/src/utils.js

epicfaace · 2021-05-21T13:26:29Z

packages/core/src/components/fields/SchemaField.js

-      className="control-label"
-      htmlFor={id}
-      aria-describedby={ariaDescribedBy(id)}>
+    <label className="control-label" htmlFor={id}>


Why was the original aria-describedby here incorrect?

I added it by mistake here: d332de1

Original is correct: <input> has id, <label> has for. aria-describedby goes to the <input>.

epicfaace · 2021-05-22T02:29:57Z

packages/material-ui/src/FieldTemplate/FieldTemplate.tsx

            {rawErrors.map((error, i: number) => {
              return (
                <ListItem key={i} disableGutters={true}>
-                  <FormHelperText id={id}>{error}</FormHelperText>
+                  <FormHelperText>{error}</FormHelperText>


Why remove the id here?

It was inside a .map. If there were multiple errors, it was not unique. Same id was also used with the rawHelp text. I moved the id to the parent <List> element and added suffix to both help and error list ids.

Thanks! Can you add a short section in the "v3 upgrade guide" doc (within the documentation) that describes some of these breaking changes in id's? Just a quick list of what exactly were changed, so that someone who upgrades to v3 will know how the ids will change in the form.

Could we consider this id change to be an internal fix? Element ids must be unique within the document and it is an undocumented feature. Here's a screenshot of the issue from the playground:

In this PR we let the input use the generated id (no change). To fix the case of non-unique ids and make aria-describedby behave correctly, we suffix help and errors ids (in material-ui FieldTemplate).

I think it's better to consider it a breaking change, because this did lead to correct behavior in the past if we just had a single p element (in which case we'd have a correct, unique id for the p element).

packages/core/index.d.ts

epicfaace · 2022-05-12T00:54:15Z

Hi @ArnoSaine it's been a while, do you remember where we are with this PR? Is this ready except for making it a breaking change / adding it to a v5.x upgrade guide?

Fixes rjsf-team#959 by reimplementing rjsf-team#2386 making it consistent across all themes - In `@rjsf/utils`, added new `descriptionId()`, `errorId()`, `helpId()`, `titleId()` and `ariaDescribedByIds()` functions with unit tests - In all themes: - Updated the `FieldErrorTemplate` to use the `errorId()` function to generate its id - Updated the `FieldHelpTemplate` to use the `helpId()` function to generate its id - Updated the `ObjectFieldTemplate` to use the `titleId()` and `descriptionId()` functions to generate the ids for the `TitleFieldTemplate` and `DescriptionFieldTemplate` respectively - Updated the `BaseInputTemplate` to pass the `ariaDescribedByIds()` value to `aria-describedby` on the "input" component - For all the widgets that generated a user-accessible "input", passed in the `ariaDescribedByIds()` call into the `aria-describedby` on the "input" component - Updated all the test snapshots to update the `title` and `description` ids to the new values and to add the new `aria-describeby` properties - Updated the documentation to indicate the existance of these new functions - Updated the `5.x migration guide` to document the potential change of ids for Title, Description and `RadioWidget` and `CheckboxWidget` - Updated the `CHANGELOG.md` accordingly

Fixes rjsf-team#959 by reimplementing rjsf-team#2386 making it consistent across all themes - In `@rjsf/utils`, added new `descriptionId()`, `errorId()`, `helpId()`, `titleId()` and `ariaDescribedByIds()` functions with unit tests - In all themes: - Updated the `FieldErrorTemplate` to use the `errorId()` function to generate its id - Updated the `FieldHelpTemplate` to use the `helpId()` function to generate its id - Updated the `ObjectFieldTemplate` to use the `titleId()` and `descriptionId()` functions to generate the ids for the `TitleFieldTemplate` and `DescriptionFieldTemplate` respectively - Updated the `BaseInputTemplate` to pass the `ariaDescribedByIds()` value to `aria-describedby` on the "input" component - For all the widgets that generated a user-accessible "input", passed in the `ariaDescribedByIds()` call into the `aria-describedby` on the "input" component - Updated all the test snapshots to update the `title` and `description` ids to the new values and to add the new `aria-describeby` properties - Updated the documentation to indicate the existance of these new functions - Updated the `5.x migration guide` to document the potential change of ids for Title, Description and `RadioWidget` and `CheckboxWidget` - Also fixed the incorrectly named `__error` to be `__errors` in the `ErrorSchema` examples - Updated the `CHANGELOG.md` accordingly

heath-freenome · 2023-01-15T03:56:54Z

Reimplemented in #3380

Fixes rjsf-team#959 by reimplementing rjsf-team#2386 making it consistent across all themes - In `@rjsf/utils`, added new `descriptionId()`, `errorId()`, `helpId()`, `titleId()` and `ariaDescribedByIds()` functions with unit tests - In all themes: - Updated the `FieldErrorTemplate` to use the `errorId()` function to generate its id - Updated the `FieldHelpTemplate` to use the `helpId()` function to generate its id - Updated the `ObjectFieldTemplate` to use the `titleId()` and `descriptionId()` functions to generate the ids for the `TitleFieldTemplate` and `DescriptionFieldTemplate` respectively - Updated the `BaseInputTemplate` to pass the `ariaDescribedByIds()` value to `aria-describedby` on the "input" component - For all the widgets that generated a user-accessible "input", passed in the `ariaDescribedByIds()` call into the `aria-describedby` on the "input" component - Updated all the test snapshots to update the `title` and `description` ids to the new values and to add the new `aria-describeby` properties - Updated the documentation to indicate the existance of these new functions - Updated the `5.x migration guide` to document the potential change of ids for Title, Description and `RadioWidget` and `CheckboxWidget` - Also fixed the incorrectly named `__error` to be `__errors` in the `ErrorSchema` examples - Updated the `CHANGELOG.md` accordingly

…ts (#3380) * fix: reimplement 2386 to add aria-describedby elements onto all inputs Fixes #959 by reimplementing #2386 making it consistent across all themes - In `@rjsf/utils`, added new `descriptionId()`, `errorId()`, `helpId()`, `titleId()` and `ariaDescribedByIds()` functions with unit tests - In all themes: - Updated the `FieldErrorTemplate` to use the `errorId()` function to generate its id - Updated the `FieldHelpTemplate` to use the `helpId()` function to generate its id - Updated the `ObjectFieldTemplate` to use the `titleId()` and `descriptionId()` functions to generate the ids for the `TitleFieldTemplate` and `DescriptionFieldTemplate` respectively - Updated the `BaseInputTemplate` to pass the `ariaDescribedByIds()` value to `aria-describedby` on the "input" component - For all the widgets that generated a user-accessible "input", passed in the `ariaDescribedByIds()` call into the `aria-describedby` on the "input" component - Updated all the test snapshots to update the `title` and `description` ids to the new values and to add the new `aria-describeby` properties - Updated the documentation to indicate the existance of these new functions - Updated the `5.x migration guide` to document the potential change of ids for Title, Description and `RadioWidget` and `CheckboxWidget` - Also fixed the incorrectly named `__error` to be `__errors` in the `ErrorSchema` examples - Updated the `CHANGELOG.md` accordingly * - Fix test snapshot * - Responded to reviewer feedback by adding and using `optionId()` and `examplesId()` - Added an optional flag to `ariaDescribedByIds()` that, if true, appends the examples id onto the string * - Minor tweak to the `5.x migration guide`

… all inputs (rjsf-team#3380) * fix: reimplement 2386 to add aria-describedby elements onto all inputs Fixes rjsf-team#959 by reimplementing rjsf-team#2386 making it consistent across all themes - In `@rjsf/utils`, added new `descriptionId()`, `errorId()`, `helpId()`, `titleId()` and `ariaDescribedByIds()` functions with unit tests - In all themes: - Updated the `FieldErrorTemplate` to use the `errorId()` function to generate its id - Updated the `FieldHelpTemplate` to use the `helpId()` function to generate its id - Updated the `ObjectFieldTemplate` to use the `titleId()` and `descriptionId()` functions to generate the ids for the `TitleFieldTemplate` and `DescriptionFieldTemplate` respectively - Updated the `BaseInputTemplate` to pass the `ariaDescribedByIds()` value to `aria-describedby` on the "input" component - For all the widgets that generated a user-accessible "input", passed in the `ariaDescribedByIds()` call into the `aria-describedby` on the "input" component - Updated all the test snapshots to update the `title` and `description` ids to the new values and to add the new `aria-describeby` properties - Updated the documentation to indicate the existance of these new functions - Updated the `5.x migration guide` to document the potential change of ids for Title, Description and `RadioWidget` and `CheckboxWidget` - Also fixed the incorrectly named `__error` to be `__errors` in the `ErrorSchema` examples - Updated the `CHANGELOG.md` accordingly * - Fix test snapshot * - Responded to reviewer feedback by adding and using `optionId()` and `examplesId()` - Added an optional flag to `ariaDescribedByIds()` that, if true, appends the examples id onto the string * - Minor tweak to the `5.x migration guide`

ArnoSaine requested a review from agustin107 as a code owner May 19, 2021 19:53

epicfaace changed the title ~~Add aria describedby attributes~~ Add predictable ID for help fields, aria describedby attributes May 19, 2021

ArnoSaine added 3 commits May 20, 2021 23:02

aria-describedby updates

d332de1

- Add aria-describedby and id attributes - Use util to create `${id}__description` values

Add @rjsf/core utils types

d5f3aca

Add errors to aria-describedby

1ad0ec8

ArnoSaine force-pushed the add-aria-describedby-attributes branch from d5acb0d to 1ad0ec8 Compare May 20, 2021 20:04

ArnoSaine added 4 commits May 20, 2021 23:19

Use util to get HelpField id

387fec7

Create id outside Help component

3670acb

Update snapshots

38848bf

Id and aria updates

ce29612

- Add more aria-describedby attributes - Fix use of same id in FieldTemplate

epicfaace self-assigned this May 21, 2021

epicfaace reviewed May 21, 2021

View reviewed changes

packages/core/src/utils.js Outdated Show resolved Hide resolved

packages/core/src/utils.js Outdated Show resolved Hide resolved

ArnoSaine added 3 commits May 21, 2021 11:49

Return a string from utils.ariaDescribedBy

f2ca5ce

Remove incorrect aria-describedby from <label>

3f71aa4

Create id outside ErrorList component

b680bb6

epicfaace reviewed May 21, 2021

View reviewed changes

epicfaace reviewed May 22, 2021

View reviewed changes

ArnoSaine added 2 commits May 22, 2021 13:34

Add id util function comments

536e3e4

Merge branch 'master' into add-aria-describedby-attributes

2828117

heath-freenome added the p1 Important to fix soon label Aug 12, 2022

heath-freenome added the v5 refactor Needs refactor due to v5 breaking changes label Aug 29, 2022

heath-freenome mentioned this pull request Jan 15, 2023

fix: reimplement #2386 to add aria-describedby elements onto all inputs #3380

Merged

8 tasks

heath-freenome closed this Jan 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add predictable ID for help fields, aria describedby attributes #2386

Add predictable ID for help fields, aria describedby attributes #2386

ArnoSaine commented May 19, 2021 •

edited

Loading

epicfaace commented May 19, 2021 •

edited

Loading

ArnoSaine commented May 21, 2021

epicfaace May 21, 2021

ArnoSaine May 21, 2021 •

edited

Loading

epicfaace May 22, 2021

ArnoSaine May 22, 2021

epicfaace May 22, 2021

ArnoSaine May 24, 2021

epicfaace Feb 26, 2022

epicfaace commented May 12, 2022

heath-freenome commented Jan 15, 2023

Add predictable ID for help fields, aria describedby attributes #2386

Add predictable ID for help fields, aria describedby attributes #2386

Conversation

ArnoSaine commented May 19, 2021 • edited Loading

Reasons for making this change

Checklist

Deploy preview

epicfaace commented May 19, 2021 • edited Loading

ArnoSaine commented May 21, 2021

epicfaace May 21, 2021

Choose a reason for hiding this comment

ArnoSaine May 21, 2021 • edited Loading

Choose a reason for hiding this comment

epicfaace May 22, 2021

Choose a reason for hiding this comment

ArnoSaine May 22, 2021

Choose a reason for hiding this comment

epicfaace May 22, 2021

Choose a reason for hiding this comment

ArnoSaine May 24, 2021

Choose a reason for hiding this comment

epicfaace Feb 26, 2022

Choose a reason for hiding this comment

epicfaace commented May 12, 2022

heath-freenome commented Jan 15, 2023

ArnoSaine commented May 19, 2021 •

edited

Loading

epicfaace commented May 19, 2021 •

edited

Loading

ArnoSaine May 21, 2021 •

edited

Loading