testing-library / dom-testing-library

🐙 Simple and complete DOM testing utilities that encourage good testing practices.
https://testing-library.com/dom
MIT License
3.27k stars 467 forks source link

Add `level` filter for `treeitem` #980

Open lindapaiste opened 3 years ago

lindapaiste commented 3 years ago

Summary

@testing-library/jest-dom explicitly disallows the use of the level property in getByRole for all roles except heading. This needs to be broadened as W3 also supports this property on the treeitem, listitem, and row roles.

Relevant code or config:

test("Can select by role treeitem with level", () => {
  render(<Hello />);
  expect(
    screen.getByRole("treeitem", { name: "project-3B.docx", level: 3 })
  ).toBeInTheDocument();
  expect(
    screen.getByRole("treeitem", { name: "project-3B.docx", level: 1 })
  ).not.toBeInTheDocument();
});

What you did:

I am trying to select an element with role="treeitem" based on its level. In my actual app I have the same names appearing as both a parent and a child.

What happened:

The test cannot be executed due to an error:

Role "treeitem" cannot have "level" property.

at queryAllByRole (node_modules/@testing-library/dom/dist/queries/role.js:72:13)

at node_modules/@testing-library/dom/dist/query-helpers.js:87:17

at node_modules/@testing-library/dom/dist/query-helpers.js:62:17

at getByRole (node_modules/@testing-library/dom/dist/query-helpers.js:111:19)

at Object. (src/components/bubbles/BubblesScreen.test.tsx:24:28)

Reproduction:

Created a CodeSandbox

Problem description:

The aria-level attribute and implicit levels are supported by W3 for multiple roles, as seen here:

However @testing-library/dom does a validation check which only allows for level to be used with the heading role.

The problematic code appears on lines 67-72 of /src/queries/roles.js:

if (level !== undefined) { 
   // guard against using `level` option with any role other than `heading`
    if (role !== 'heading') {
      throw new Error(`Role "${role}" cannot have "level" property.`)
    }
}

Suggested solution:

The code section above needs to be modified to prevent throwing the error.

The actual computation of the level property happens here. The current code will work on treeitem elements which use the explicit aria-level attribute. The implicit level is based on the current item's position in the tree, so additional work needs to be done to compute the implicit level. heading levels can be computed just from the tagName but treeitem levels cannot. It would require some sort of querying of the parents and the tree.

eps1lon commented 3 years ago

Thanks for the feedback.

We're waiting on a new release of aria-query for this.

Though I strongly advise against using level for treeitems since you'd have to explicitly declare these attributes (which may be redundant). We currently don't support computing the actual level of a treeitem within a tree.

MatanBobi commented 2 years ago

@eps1lon am I missing something? it looks like this one is supported in Aria 1.1 and the change on our end seems quite straightforwards since there's no computation needed, just adding the role to this if statement

Can you please elaborate what are we waiting for from aria-query? If we're not waiting anymore, I'd be more than happy to fix this one.

eps1lon commented 2 years ago

Not sure anymore why this was an issue.

But we definitely would need to add support for computing implicit level of a treeitem

thescripted commented 1 month ago

@eps1lon Am also hitting this issue as well (when applying level to row element). I noticed that your draft #1088 will solve for implicit treeview arialevels, but that will not solve for row elements.

Let me know if you're willing to accept PRs (or if this should be a separate issue). I can help add support so that row elements can also be filtered by level attributes.