Skip to content

feat(getByRole): Allow filter by disabled #1231

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

Conversation

kemuridama
Copy link

@kemuridama kemuridama commented May 8, 2023

What:

This PR allows getByRole() filter by disabled attribute.

We often write tests depending on disabled or aria-disabled attribute of elements, but getByRole() method cannot filter by it.

There are different meaning between disabled and aria-disabled attribute slightly. disabled makes a element be unclickable and unfocusable, but a element setting by aria-disabled: true remains clickable and focusable.

https://codepen.io/kemuridama/pen/rNqJKqm

From that differences, I have 2 solutions to filter by disabled attribute:

  • Filter by both disabled and aria-disabled attribute
  • Filter by disabled attribute only

This PR resolves using the former solution, but there are controvertible.

Why:

To write tests depending on disabled or aria-disabled attribute of elements easily.

How:

Same patterns as for expanded, selected etc

Checklist:

@codesandbox-ci
Copy link

codesandbox-ci bot commented May 8, 2023

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 dd179c7:

Sandbox Source
react-testing-library-examples Configuration

@kemuridama kemuridama force-pushed the allow_filter_by_disabled branch from c63de5c to dd179c7 Compare May 8, 2023 05:12
@codecov
Copy link

codecov bot commented May 8, 2023

Codecov Report

Merging #1231 (dd179c7) into main (39a64d4) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main     #1231   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           24        24           
  Lines         1042      1046    +4     
  Branches       351       349    -2     
=========================================
+ Hits          1042      1046    +4     
Flag Coverage Δ
node-14 100.00% <100.00%> (ø)
node-16 100.00% <100.00%> (ø)
node-18 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/queries/role.ts 100.00% <100.00%> (ø)
src/role-helpers.js 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@MatanBobi
Copy link
Member

Hi @kemuridama!
Thanks for opening this.
This was already done by @eps1lon in #1221, I'm closing this :)

@MatanBobi MatanBobi closed this May 8, 2023
@kemuridama kemuridama deleted the allow_filter_by_disabled branch May 9, 2023 15:52
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