The Wayback Machine - https://web.archive.org/web/20201109012251/https://github.com/testing-library/jest-dom/issues/256
Skip to content

/ jest-dom

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

Jump to bottom

toHaveErrorMessage #256

Open
JacobParis opened this issue Jun 3, 2020 · 3 comments
Open

toHaveErrorMessage #256

JacobParis opened this issue Jun 3, 2020 · 3 comments

Comments

@JacobParis
Copy link

@JacobParis JacobParis commented Jun 3, 2020 •
edited

Describe the feature you'd like:

I'd like an assertion for the aria-errormessage attribute. It accepts a list of id strings formatted identically to the format aria-describedby accepts.

https://www.w3.org/TR/wai-aria-1.1/#aria-errormessage

Browser support was poor a year ago but I can't seem to find any up-to-date information on its support level. There is some chance that this is unsupported enough to be not worth implementing, but on a long enough timeline I think it'll fit well here.

Usage

<input
  type="text"
  id="mortgage-amount"
  name="MORTGAGE_AMOUNT"
  aria-label="Mortgage Amount"
  aria-describedby="mortgage-amount-description"
  aria-invalid="true"
  aria-errormessage="mortgage-amount-error"
/>

<p id="mortgage-amount-error"> Must be a positive number </p>
expect().toHaveErrorMessage('Must be a positive number')

Suggested implementation:

The implementation would be nearly a clone of toHaveDescription except the elements it reads come from the aria-errormessage attribute instead of aria-describedby

It could also assert that aria-invalid is true

I'm willing to make a PR for inclusion but I'm new to open source so I wanted to get feedback up front

Describe alternatives you've considered:

Teachability, Documentation, Adoption, Migration Strategy:

toHaveErrorMessage

toHaveErrorMessage(text: string | RegExp)

This allows you to check whether the given element has an error message or not.

An element gets its error message via the
aria-errormessage attribute.
Set this to the id of one or more other elements. These elements may be nested
inside, be outside, or a sibling of the passed in element.

Whitespace is normalized. Using multiple ids will
join the referenced elements’ text content separated by a space.

When a string argument is passed through, it will perform a whole
case-sensitive match to the error message text.

To perform a case-insensitive match, you can use a RegExp with the /i
modifier.

To perform a partial match, you can pass a RegExp or use
expect.stringContaining("partial string").

Examples

<!-- with aria-errormessage and aria-invalid -->
<input type="text" aria-label="Username" aria-errormessage="username-error" aria-invalid="true"/>

<div id="username-error">
  Username must not contain spaces
</div>

<!-- with just aria-errormessage -->
<input type="text" aria-label="Email" aria-errormessage="email-error" />

<div id="email-error">
  Email must contain an '@' sign
</div>

<!-- with just aria-invalid -->
<input type="password" aria-label="Password" aria-invalid="true" />
// with aria-errormessage and aria-invalid
const usernameInput = getByLabelText('Username')

expect(usernameInput).toHaveErrorMessage('Error: Username must not contain spaces')
expect(usernameInput).toHaveErrorMessage(/Error/) // to partially match
expect(usernameInput).toHaveErrorMessage(expect.stringContaining('Error') // to partially match
expect(usernameInput).toHaveErrorMessage(/error/i) // to use case-insensitive match
expect(usernameInput).not.toHaveErrorMessage('Other error message')

// with just aria-errormessage
const emailInput = getByLabelText('Email')

expect(emailInput).not.toHaveErrorMessage()
expect(emailInput).not.toHaveErrorMessage('') // Missing aria-invalid will return false

// with just aria-invalid
const passwordInput = getByLabelText('Password')

expect(passwordInput).not.toHaveErrorMessage()
expect(passwordInput).toHaveErrorMessage('') // Missing or empty error message always becomes a blank string,
@gnapse
Copy link
Member

@gnapse gnapse commented Jun 3, 2020

Sure, this is really nice. Also, "Today I Learned" that this attribute and aria feature even existed. Thanks!

In reading the spec for this attribute, yes, you are right that this matcher should assert that aria-invalid="true". If it is not, even the presence of the attribute does not make it be recognized as pertinent.

Another thing that caught my eye from the spec is that the referenced element that holds the error message should be visible (screen reader visible at least). So I think the matcher should also validate that the referenced element does not have display: none. This however, may be error-prone. For example, the developer of the code may be hiding the error with a css class name that has the display: none rule in it, but when running in the tests usually css stylesheets are not loaded, so the element could still be hidden and this matcher would still pass.

But I think it does not hurt we still include the check. At the very least, we'd help authors catch the case where they hide/show the element by applying style="display: none|block" directly in the element. There's not much we can do about the potential false positives I mentioned above, but we could help catch other false positives. Thoughts?

In any case, I'm happy to take this myself, but I won't have time until about the next weekend or the other one. If you or anyone else is up to it, feel free to take a crack at it.

Thanks!
@JacobParis
Copy link
Author

@JacobParis JacobParis commented Jun 3, 2020

I'm down to write up an initial PR for review to cover at least the error message and invalid assertion.

If we include within scope that the error message be visible, is that as simple as calling toBeVisible? If there's a need for custom logic handling different cases where an element could be out of view, I feel like that's better left to a dedicated assertion which ideally other methods would be able to take advantage of
@gnapse
Copy link
Member

@gnapse gnapse commented Jun 3, 2020

I'm ok with leaving that part of the spec out of the initial version. But leaving it to a separate assertion would require the user writing the tests to grab that element's reference first, which would make it cumbersome and not something with which this assertion would be helping with. They' need to get into the implementation details that this new custom matcher would be trying to give them.

But yes, feel free to ignore that part. We can always improve it later.
@JacobParis JacobParis mentioned this issue Jun 3, 2020
Open
1 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants
@gnapse @JacobParis
You can’t perform that action at this time.