Skip to content

Add level option for *ByRole('heading') #743

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.

Sign up for GitHub

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
Closed
winterlamon opened this issue Aug 18, 2020 · 9 comments
Closed

Add level option for *ByRole('heading') #743

winterlamon opened this issue Aug 18, 2020 · 9 comments
Assignees
@winterlamon
Labels
enhancement New feature or request

Comments

@winterlamon
Copy link
Contributor

Describe the feature you'd like:

Add new queries for *ByHeadingLevel that allow the specification of the heading level by which to query, e.g. getAllByHeadingLevel('h2').

I know that suggestions about adding queries by tag have been rejected because they go against the user-centric testing principles, but I think this particular case is user-centric, especially for those using assistive technologies.

Suggested implementation:

getByHeadingLevel, queryByHeadingLevel, getAllByHeadingLevel, queryAllByHeadingLevel, findByHeadingLevel, findAllByHeadingLevel

getByHeadingLevel(
  container: HTMLElement, // or skip this if using `screen`
  level: 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6'
  // I don't think this would need to use to standard options like `exact` (with the way it's typed)
  // or `normalizer`, but please correct me if I'm wrong
): HTMLElement

Describe alternatives you've considered:

While *ByRole('heading') can grab the headings on the DOM, there is currently no way to specify the heading levels. And while a custom query could be made, or simply using document.querySelector(), this particular query seems like it should be a built-in as it is one of the ways that people using assistive technologies navigate web pages.

Screen Shot 2020-08-18 at 11 59 33 AM

Teachability, Documentation, Adoption, Migration Strategy:

Because the syntax is similar to other queries, I imagine the teachability and adoption should take minimal effort (beyond what's needed to learn any of the other queries).

Possible Documentation

ByHeadingLevel

getByHeadingLevel, queryByHeadingLevel, getAllByHeadingLevel, queryAllByHeadingLevel, findByHeadingLevel, findAllByHeadingLevel

getByHeadingLevel(
  container: HTMLElement, // if you're using `screen`, then skip this argument
  level: HeadingLevel ('h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6')
  // I don't think this would need to use to standard options like `exact` (with the way it's typed)
  // or `normalizer`, but please correct me if I'm wrong
): HTMLElement

Queries headings by their level and returns the element -- either the HTML heading element (<h1> - <h6>) or the element with role="heading" and aria-level with matching level.

Although ByHeadingLevel queries elements with role="heading" and the aria-level attribute present, it is strongly encouraged to use the semantic HTML heading elements (<h1> - <h6>).

Note that the ByHeadingLevel query only supports heading levels h1-h6. While, it is possible to use aria-level with a heading level greater than 6, it is discouraged due to inconsistent browser and assistive technology support.

For example in

<body>
  <main>
    <h1>All About Tuesday</h1>

    <h2>What I Did Today</h2>
    <p>I didn't get as much done today as I would have liked.</p>

    <h3>Drinking Coffee</h3>
    <p>I drank a lot of iced coffee. It was a Kona blend with oat milk.</p>
    <p>I'm really bad at coming up with examples.</p>

    <h2>What I Didn't Accomplish Today</h2>
    <p>Everything else.</p>
  </main>
</body>

getByHeadingLabel('h3') would return <h3>Drinking Coffee</h3>.
getAllByHeadingLabel('h2') would return [<h2>What I Did Today</h2>, <h2>What I Didn't Accomplish Today</h2>].

(...plus some reference links thrown in...)
@eps1lon
Copy link
Member

eps1lon commented Aug 19, 2020

I'd prefer ByRole('heading', { level: n }). It could query by aria-level and would be in line with the current pressed, selected, and expanded filters in ByRole.

Maybe aria-query already has some built-in validation for allowed values of aria-level.

@winterlamon
Copy link
Contributor Author

winterlamon commented Aug 19, 2020 •
edited
Loading

I'd prefer ByRole('heading', { level: n }). It could query by aria-level and would be in line with the current pressed, selected, and expanded filters in ByRole.

Maybe aria-query already has some built-in validation for allowed values of aria-level.

That works for me. I wasn't sure about about having options available that don't apply to specifically to headings, but I see that ByRole does that already and just throws errors when the option doesn't apply to a given role.

I think aria-query only types aria-level as an integer; the sky's the limit! I'm happy to keep the number unspecified in the implementation, but I think it would still be a good idea to encourage using 1-6 in the documentation.

@alexkrolick
Copy link
Collaborator

I was just thinking it would be good to have this feature! 🚀

ByRole('heading', { level: n }) +1 to this API

@kentcdodds
Copy link
Member

I'm on board with that suggestion as well 👍

@kentcdodds
Copy link
Member

We should probably have some validation at runtime to make sure that this option is only used with the role of heading.

@winterlamon
Copy link
Contributor Author

Awesome. I'm happy to take a shot at a PR for it (as long as no one wants it like tomorrow).

@eps1lon
Copy link
Member

eps1lon commented Aug 20, 2020 •
edited
Loading

We should probably have some validation at runtime to make sure that this option is only used with the role of heading.

I think we already do similar checks for the existing filters. Anybody that wants to work on this might want to check out #540, #692 or #729.

Though I think we shouldn't throw with "aria-X is not supported" but rather with "the element cannot have X". They don't always come from aria attributes and, in the end, ARIA is just an implementation details 😉

@nickserv nickserv added the enhancement New feature or request label Aug 21, 2020
@weyert
Copy link
Contributor

weyert commented Aug 22, 2020

Loving this idea!

@kentcdodds kentcdodds changed the title Add *ByHeadingLevel queries Add level option for *ByRole('heading') Aug 22, 2020
@winterlamon winterlamon mentioned this issue Aug 27, 2020
Merged
4 tasks
@winterlamon
Copy link
Contributor Author

Closed by #757

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@winterlamon winterlamon

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@weyert @nickserv @kentcdodds @alexkrolick @eps1lon @winterlamon