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

A more proper way to query form inline errors #634

Open
cloud-walker opened this issue Jun 14, 2020 · 4 comments
Open

A more proper way to query form inline errors #634

cloud-walker opened this issue Jun 14, 2020 · 4 comments

Comments

@cloud-walker
Copy link

cloud-walker commented Jun 14, 2020

References:

Describe the feature you'd like:

Until today, I've always used getByTestId to find a form inline error:

<form aria-label="form">
  <fieldset>
    <label for="email">Email</label>
    <input type="email" id="email" />
    <output data-testid="email-error" for="email">Email is required.</output>
  </fieldset>

  <fieldset>
    <label for="password">Password</label>
    <input type="password" id="password" />
    <output data-testid="password-error" for="password">Password is required.</output>
  </fieldset>

  <button type="submit">Submit</button>
</form>

But I'm always wondering if there is a more semantic / accessible way to both write the markup and find it in my tests.

In the html example I'm using the output tag with the for attribute (I'm still not very sure that its the proper use of this tag, lets discuss that 🤔), so maybe we could support a query like getByRole('status', {name: 'something'}) or another more getByLabelText style.

Suggested implementation:

I think that first we need to discuss the proper way to write an accessible inline form error and if possible to link it with its own input(s) (like a label).

Describe alternatives you've considered:

I've red in an article that a <div role="alert"> can be used, but is missing way to link it to the input(s), and AFAIK role="alert" is not the same as role="status" (the role from output tag).

Teachability, Documentation, Adoption, Migration Strategy:

Same as "Suggested Implementation"

@smeijer
Copy link
Member

smeijer commented Jun 14, 2020

We have a few options:

(links are pointing to playgrounds)

  1. screen.getByText(/email is required/i): This one works as-is on your markup. And thereby it's the best alternative I can provide you at this moment, for the getByTestId query.

  2. screen.getByRole('status'): It kind of works, but as you have two elements with that role, there are some challenges. It works as-is, when having only a single fieldset. You could use it in combination with a within, but I would favor the getByText in this scenario.

Now, what I don't understand:

  1. When I make the fieldset look like below (playground), I see that the accessibility improves. Chrome now shows a Described by attribute, and a Description: 'Email is required' under the accessibility tab.

    We can specify the name in a getByRole query, but we cannot specify the description. It would be awesome if we could add that. The query would then look like :getByRole('alert', { description: /email is required/i })

      <form aria-label="form">
        <fieldset>
          <label for="email">Email</label>
          <input type="email" id="email" aria-describedby="email-error" required="true" aria-invalid="true" />
          <div role="alert" id="email-error">
            Email is required.
          </div>
        </fieldset>
      </form>

@smeijer
Copy link
Member

smeijer commented Jun 14, 2020

A little research shows me that adding support 3 from my post above, is doable once eps1lon/dom-accessibility-api#210 lands.

I have something working, but I need to attend a family thing first. I will check a bit more later.

@cloud-walker
Copy link
Author

Really like your inputs, but I also think that the error message may not be unique as the form field id.

If the message was only "Required" for both password and email for example you are stuck with the multiple elements found issue.

@smeijer
Copy link
Member

smeijer commented Jun 14, 2020

If the message was only "Required" for both password and email for example you are stuck with the multiple elements found issue.

True. But so might be your user who depends on a screen reader.

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

No branches or pull requests

2 participants