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

ByRole: Constrain API further #1202

Closed
eps1lon opened this issue Jan 9, 2023 · 4 comments
Closed

ByRole: Constrain API further #1202

eps1lon opened this issue Jan 9, 2023 · 4 comments
Labels
BREAKING CHANGE This change will require a major version bump

Comments

@eps1lon
Copy link
Member

eps1lon commented Jan 9, 2023

Describe the feature you'd like:

As part of https://gist.github.com/bvaughn/d3c8b8842faf2ac2439bb11773a19cec I'm revisiting a lot of selector APIs from different libraries.

The API of ByRole stood out with regard to role selection and its options.

Suggested implementation:

  1. Constraint role to string
  2. match fallback roles (e.g. byRole('none') === byRole('presentation') in <div role="none presentation" /> Leaving for now. Not so clear what's the best default here
  3. Remove exact option
  4. Remove normalizer option

Describe alternatives you've considered:

Document concrete use cases (motivated by real-world tests). If you have some, please let me know.

Teachability, Documentation, Adoption, Migration Strategy:

We can document fallback role matching in a footnote. I expect that most queries will not be concerned with fallback roles.

Constraining the API makes teaching this query easier.

@eps1lon eps1lon added the BREAKING CHANGE This change will require a major version bump label Jan 9, 2023
@eps1lon
Copy link
Member Author

eps1lon commented Feb 6, 2023

Not dropping the queryFallbacks for now. Not clear to me if we should restrict that and what the best default would be.

@eps1lon eps1lon mentioned this issue Feb 6, 2023
4 tasks
@MatanBobi
Copy link
Member

@eps1lon can we close this issue? I think that all of the points were covered as part of #1211.

@eps1lon
Copy link
Member Author

eps1lon commented Feb 18, 2023

Closed in #1211
Released in @testing-library/dom@9.0.0

@eps1lon eps1lon closed this as completed Feb 18, 2023
@charliematters
Copy link

charliematters commented Apr 6, 2023

Sorry to dig up a closed issue, but we've just upgraded to the version with this fix in, and this query no longer works. Maybe we're abusing it and there's a better way, but I can't find any suggestions from the docs? We use this to wait for a modal with a given string to appear

 const modal = await screen.findByRole(
    (content, element) => {
      return (
        content === 'dialog' &&
        within(element as HTMLElement).queryByText(title) !== null
      )
    }

Just in case anyone else has something like this, we've reverted to the lower-level:

const modal = await waitFor(() => {
    const foundModal = screen
      .getAllByRole('dialog')
      .find((dialog) => within(dialog).queryByText(title) !== null)

    if (!foundModal) {
      throw new Error(`Cannot find Modal with "${title}"`)
    }

    return foundModal
  })

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BREAKING CHANGE This change will require a major version bump
Projects
None yet
Development

No branches or pull requests

3 participants