Skip to content

fix: Various elements and their default roles #406

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

Closed
wants to merge 4 commits into from

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Nov 27, 2019

DO NOT MERGE. new version of aria-query isn't released yet. Instead we're using a version built from aria-query@master.

Opening this already to get some comments on how we address the "with a accessible name" constraint.

What:

Bump aria-query

Why:

Fixes these bugs:

  • <input> being considered a textbox
  • listbox and combobox not being recognized as implicit roles on certain elements

How:

Built aria-query from their master branch via CodeSandbox CI.

Checklist:

  • [ ] Documentation added to the
    docs site
  • [ ] I've prepared a PR for types targeting
    DefinitelyTyped
  • Tests
  • Ready to be merged

@codesandbox-ci
Copy link

codesandbox-ci bot commented Nov 27, 2019

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 598b4de:

Sandbox Source
react-testing-library-examples Configuration

@@ -69,16 +69,54 @@ function getImplicitAriaRoles(currentNode) {
}
}

// <form /> with an accessible name?
if (currentNode.matches('form')) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just restoring the old implementation that did not care about accessible name.

Technically <form> does not have the role="form" because the accessible name is missing (don't ask me why. I haven't tested it yet with AT; just following the spec). We could apply some heuristics to guess if it actually has one but if we want to be strict we would need to run accessible name computation (expensive).

Assuming I finish byRole('form', { name: 'some-name' }) we could leave this quirk and then ignore any element returned getImplicitAriaRoles if the accessible name is empty (and potentially warn if a form is queried without providing a name).

@kentcdodds
Copy link
Member

What's the status here now that aria-query has been updated?

@eps1lon
Copy link
Member Author

eps1lon commented Mar 3, 2020

Seems like I lost track of this as well. This PR is superseded by #446

@eps1lon eps1lon closed this Mar 3, 2020
@eps1lon eps1lon deleted the feat/bump-aria-query branch March 3, 2020 16:20
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

Successfully merging this pull request may close these issues.

2 participants