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

Rendering a form multiple times in the same view results in ID clashes #447

Open
chawes13 opened this issue Oct 5, 2020 · 6 comments
Open
Labels
a11y Accessibility backlog

Comments

@chawes13
Copy link
Contributor

chawes13 commented Oct 5, 2020

Issue

  • When the same form is used several times in the same view, the input ids (defaults to name) will not be unique unless overrides are provided. Screen readers will typically read all labels corresponding to the id for the first input.

For example, the "Name" input is read 6 times on this view: https://share.getcloudapp.com/KouwQ51p & https://share.getcloudapp.com/YEuyvYJA

Potential Enhancement

  • Use a library like react-uid to append a unique id to the input's name by default

Workarounds

  • Manually give each input a unique id
    • Onerous for large forms and difficult to ensure consistency across developers
  • Don't render the same form multiple times in the same view or combine fields into one form (a la FieldArray)
    • Would require a significant amount of work on existing applications
  • Namespacing using a unique FormSection
    • Would then require contraints, submitFilters, etc. to be dynamic generated
@chawes13 chawes13 added the a11y Accessibility label Oct 5, 2020
@dpikt
Copy link
Contributor

dpikt commented Oct 7, 2020

@chawes13 this is an interesting problem. The solution you proposed seems like the best way to go. I had assumed that non-dynamic IDs were preferred over dynamic ones but it looks like that just isn't the case (https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/id):

This attribute's value is an opaque string: this means that web authors should not rely on it to convey human-readable information (although having your IDs somewhat human-readable can be useful for code comprehension, e.g. consider ticket-18659 versus r45tgfe-freds&$@).

Given that info, my only concern would be making sure default styles worked with the new IDs. And this would probably be a breaking change, if an app has any styles relying on the current ID naming scheme.

For a short-term workaround, the route of adding unique IDs to a form shouldn't be too time-consuming if you use a consistent prefix:

function MyForm ({ form }) {
    const uniqueId = (inputName) => form + '-' + inputName
    ... 
    <Field name="firstName" id={ uniqueId('firstName')  } ... />
}

@chawes13
Copy link
Contributor Author

chawes13 commented Oct 8, 2020

@dpikt What process should we use for including the fix for this issue in an upcoming major release? E.g., create an Asana task?

Unfortunately it is pretty time consuming. Every step in the survey renders multiple forms (sometimes the same form, sometimes different forms). Both scenarios run the risk of having an ID conflict.

function ContactForm () { 
  // ...
  <Field name="name" ... />
}

function ProgramForm () {
  // ...
  <Field name="name" ... />
}

function SurveyStep () {
  return (
    <>
      <ContactForm />
      {programs.map((p) => <ProgramForm key={p.id} .. />}
    </>
}

The former scenario makes it so that the form+'-'+inputName is not sufficiently unique. That being said, react-uid is already included as dependency for other features in the app so that could be used as well. Unfortunately this makes me realize that there's another issue, where the id that is generated for InputError is not guaranteed to be unique, which could also impact users who use a screenreader (e.g., inaccurately reporting that a field has an error)

id={ generateInputErrorId(name) }

@dpikt
Copy link
Contributor

dpikt commented Oct 8, 2020

Wouldn't form names still be unique in the first scenario since redux-form requires a unique key?

As far as resolving goes, I think a PR just for this issue would be sufficient. You could probably resolve the input error issue by allowing the auto-generated ID to be overridden and passing it in via LabeledField if you wanted to avoid a breaking change.

@chawes13
Copy link
Contributor Author

chawes13 commented Oct 8, 2020

🤦 right, right of course. Thanks for the clarification!

@chawes13
Copy link
Contributor Author

@dpikt Unfortunately this approach does not work on CheckboxGroup or RadioGroup inputs, as the explicit id gets passed to each individual input.

This behavior may be able to be extracted into a separate, isolated issue as I do not think we want this behavior in any circumstance. Thoughts?

@dpikt
Copy link
Contributor

dpikt commented Oct 14, 2020

@chawes13 agreed, that issue is separate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a11y Accessibility backlog
Projects
None yet
Development

No branches or pull requests

2 participants