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

Accessibility-related matchers #55

Open
gnapse opened this issue Sep 4, 2018 · 10 comments
Open

Accessibility-related matchers #55

gnapse opened this issue Sep 4, 2018 · 10 comments
Labels
help wanted Extra attention is needed needs discussion We need to discuss this to come up with a good solution question Further information is requested

Comments

@gnapse
Copy link
Member

gnapse commented Sep 4, 2018

Describe the feature you'd like:

In the light of projects like Reach UI and react-beautiful-dnd and others like them, that have a focus on accessibility, I was thinking that it could be useful to test that stuff in the Dom have some features that help with accessibility.

I'm not sure though, if it'd be enough to test for aria- attributes with toHaveAttribute, or if there's something else we could do with custom matchers that would help even more. It does not help that I my self am fairly new to being aware and worried about a11y in my apps (shame on me!) but I'm catching up, and wanted to raise the issue here in hope for others to chime in and determine if there could be something jest-dom could do about it.

Suggested implementation:

Not sure yet. Not even sure if it's needed and kind of matchers we would need.

Describe alternatives you've considered:

Not sure of any. If you know of any, let me know in the comments.

Teachability, Documentation, Adoption, Migration Strategy:

TODO

@gnapse gnapse added help wanted Extra attention is needed question Further information is requested needs discussion We need to discuss this to come up with a good solution labels Sep 4, 2018
@kentcdodds
Copy link
Member

Prior art: jest-axe 👍

@smacpherson64
Copy link
Collaborator

smacpherson64 commented Sep 4, 2018

This sounds incredible!

@gnapse, since jest-dom is scoped to DOM elements toHaveAttribute does work for quite a bit.

*One prime area that is missing through is anything where it deals with more than one element: like aria-labelledby, aria-describedby, aria-controls!

@gnapse
Copy link
Member Author

gnapse commented Sep 4, 2018

@gnapse, since jest-dom is scoped to DOM elements toHaveAttribute does work for quite a bit.

Yes, the drawback is that in addition to tests being too verbose, people would need to know what to test for in order to test that a UI is "accessible". I was kinda hoping to provide something akin to .toBeAccessible() 😄 that would take of everything for me, and in the process of creating it, we would learn a lot. And users using it would learn too.

Apparently jest-axe takes care of that, or rather axe, which powers it (didn't know about it either). Taking a loot at it right now. Maybe that's the answer and we do not need to do anything. Though at the very least I'd like to link to the from the README. I'm going to play with it a bit to see how it goes.

@smacpherson64
Copy link
Collaborator

smacpherson64 commented Sep 4, 2018

Hahaha, kk, It would be really amazing to have the ability to have an in-depth automated accessibility test that was really accurate and walked you through how to resolve the issues.

I have been using axe (chrome plugin) and Lighthouse (in chrome dev tools audits tab) to find issues and they seem to work really well.

I was really excited to see something like this!

HTML from MDN

/* Assuming below
<div id="dialog" role="dialog" aria-labelledby="dialogheader">
    <h2 id="dialogheader">Choose a File</h2>
    ...Dialog contents
</div>
*/

expect(document.querySelector('#dialog')).toBeLabelled('Choose a File')

@gnapse
Copy link
Member Author

gnapse commented Sep 4, 2018

That's something, yeah. I wonder if dom-testing-library's getByLabelText gives you the dialog.

Another possibility is to test, for instance, that you're not applying onClick handlers to elements that are not interactive. Like <div onClick={...}> ... </div> without a role="button" in the div.

I assume these are the kinds of things that axe takes care of. Let's see.

kentcdodds pushed a commit to testing-library/dom-testing-library that referenced this issue Sep 4, 2018
**What**: Add queries for role dom attribute

<!-- Why are these changes necessary? -->

**Why**: As evidenced by [testing-library/jest-dom#55](testing-library/jest-dom#55) having a role attribute query would be helpful when attempting to get elements that don't fit the other queries.

My example was a dialog who did not have any other matching queries and due to the associated library. I was able to add data-testid but that added the attribute too low in the generated dom not to the container root as expected, this is a problem with the library not dom-testing-library.

<!-- How were these changes implemented? -->

**How**: I added new query selectors for the node attribute.

<!-- Have you done all of these things?  -->

**Checklist**:

<!-- add "N/A" to the end of each line that's irrelevant to your changes -->

<!-- to check an item, place an "x" in the box like so: "- [x] Documentation" -->

- [x] Documentation
- [x] Tests
- [x] Ready to be merged <!-- In your opinion, is this ready to be merged as soon as it's reviewed? -->
- [x] Added myself to contributors table <!-- this is optional, see the contributing guidelines for instructions -->

<!-- feel free to add additional comments -->
@smacpherson64
Copy link
Collaborator

smacpherson64 commented Sep 11, 2018

Hrm, the core of axe is really cool!
https://github.com/dequelabs/axe-core/blob/develop/lib/rules/label.json#L18-L29

For example if we did make a toBeLabelled matcher it should be able to handle:

Explicit Label

<label for="explicit">Label</label>
<input id="explicit" type="text">

Implicit Label

<label><input id="implicit" type="radio">Label</label>

Labelledby

<span id="labelledby">Label</span>
<input type="text" aria-labelledby="labelledby">

Direct Label

<input type="text" aria-label="Label">

Title

<input type="text" title="Label">

Also the following rules must be true: The explicit label should be visible, there should only be one explicit label, and the help text (E.G.: Title and Label) should not be same exact text.

I am wondering if it makes sense for any of these matchers to directly use axe validation (on a per matcher basis - so axe would be configured per matcher to only look for one thing). So for example toBeLabelled would use axe to make sure rules match valid labels, then do normal matcher actions.

We would need to be careful though about scoping (to a specific set of elements) and JSDOM:

https://github.com/dequelabs/axe-core
There is limited support for JSDOM. We will attempt to make all rules compatible with JSDOM but where this is not possible, we recommend turning those rules off. Currently the color-contrast rule is known not to work with JSDOM.

@eps1lon
Copy link
Member

eps1lon commented Sep 24, 2019

WRT to isAccessible: We now automatically exclude inaccessible elements in @testing-library/dom#getByRole with https://github.com/testing-library/dom-testing-library/blob/dbbea6ee514399d0b37690ce5c56bb21f5ae2cb3/src/role-helpers.js#L17

I think it's not public yet though. I'll start working on a chai matcher and report back how that worked.

@ghostd
Copy link

ghostd commented Apr 22, 2020

Hi,

On a professional project i needed a matcher to check if the form fields have an 'aria' description. I created this:

const matches = (textToMatch: string, matcher: string | RegExp) => {
  if (matcher instanceof RegExp) {
    return matcher.test(textToMatch);
  }

  return textToMatch.includes(matcher);
};

const getFieldName = (formField: HTMLElement) => {
  return formField.id || formField.getAttribute('name') || 'element';
};

export function toHaveDescriptionText(this: jest.MatcherContext, formField: HTMLElement, text: string | RegExp): jest.CustomMatcherResult {
  const ariaDescribedBy: string = formField.getAttribute('aria-describedby') || '';
  const elementIds: string[] = ariaDescribedBy.split(' ').filter(Boolean);
  const hasDescription = elementIds
    .map((id: string) => document.getElementById(id))
    .some((element) => element != null && element.textContent && matches(element.textContent, text));

  return {
    pass: hasDescription,
    message: () => {
      return matcherHint(`${this.isNot ? '.not' : ''}.toHaveDescriptionText`, getFieldName(formField), String(text));
    },
  };
}

Here is an example:

expect(getByLabelText('Account name')).toHaveDescriptionText('This field is required');

Would that be useful to add it into testing-library/jest-dom? i could provide a PR.

Regards

@eps1lon
Copy link
Member

eps1lon commented Apr 22, 2020

@ghostd Thanks for offering help. Eventually dom-accessibility-api should get a computeAccessibleDescription following the accname spec. So any effort should go into that package which has proper infra to test that the accessible description is correct.

@ghostd
Copy link

ghostd commented Apr 22, 2020

Thanks for the pointers. I didn't know this specification.

qijixin2 pushed a commit to qijixin2/dom-testing-library that referenced this issue Aug 26, 2022
**What**: Add queries for role dom attribute

<!-- Why are these changes necessary? -->

**Why**: As evidenced by [testing-library/jest-dom#55](testing-library/jest-dom#55) having a role attribute query would be helpful when attempting to get elements that don't fit the other queries.

My example was a dialog who did not have any other matching queries and due to the associated library. I was able to add data-testid but that added the attribute too low in the generated dom not to the container root as expected, this is a problem with the library not dom-testing-library.

<!-- How were these changes implemented? -->

**How**: I added new query selectors for the node attribute.

<!-- Have you done all of these things?  -->

**Checklist**:

<!-- add "N/A" to the end of each line that's irrelevant to your changes -->

<!-- to check an item, place an "x" in the box like so: "- [x] Documentation" -->

- [x] Documentation
- [x] Tests
- [x] Ready to be merged <!-- In your opinion, is this ready to be merged as soon as it's reviewed? -->
- [x] Added myself to contributors table <!-- this is optional, see the contributing guidelines for instructions -->

<!-- feel free to add additional comments -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed needs discussion We need to discuss this to come up with a good solution question Further information is requested
Projects
None yet
Development

No branches or pull requests

5 participants