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

feat: toHaveAccessibilityState() #124

Merged
merged 14 commits into from Nov 10, 2022
Merged

Conversation

mdjastrzebski
Copy link
Collaborator

@mdjastrzebski mdjastrzebski commented Nov 3, 2022

What:

Matcher for matching accessiblity state, that takes into account implied state.

Why:

Current toHaveProp is performing strict matching of accessibilityState prop, while matching based only on some state entries is useful. Additionally toHaveProp is not aware of implicit state entries.

How:

Calculate implied accessibilityState using the same rules as React Native Testing Library and match it only agains requested a11y state entries.

Checklist:

  • Documentation added to the docs
  • Typescript definitions updated
  • Tests
  • Ready to be merged

@pierrezimmermannbam, @AugustinLF, @MattAgn pls take a look

@codecov
Copy link

codecov bot commented Nov 3, 2022

Codecov Report

Merging #124 (cea85b1) into main (fae097a) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #124   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            9        10    +1     
  Lines          175       193   +18     
  Branches        57        60    +3     
=========================================
+ Hits           175       193   +18     
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/extend-expect.ts 100.00% <ø> (ø)
src/to-have-accessibility-state.ts 100.00% <100.00%> (ø)

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

@mdjastrzebski mdjastrzebski requested review from MichaelDeBoey, thymikee and brunohkbx and removed request for MichaelDeBoey November 4, 2022 08:35
@mdjastrzebski mdjastrzebski changed the title feat: to have accessibility state feat: toHaveAccessibilityState() Nov 8, 2022
README.md Show resolved Hide resolved
Comment on lines +461 to +494
expect(screen.getByTestId('view')).not.toHaveAccessibilityState({ checked: false });
expect(screen.getByTestId('view')).not.toHaveAccessibilityState({ expanded: false });
Copy link

@MattAgn MattAgn Nov 9, 2022

Choose a reason for hiding this comment

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

these 2 last examples confused me for a bit because we also have

expect(screen.getByTestId('view')).not.toHaveAccessibilityState({ checked: true });
expect(screen.getByTestId('view')).not.toHaveAccessibilityState({ expanded: true });

did you check for false here to contrast with the other example below?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for pointing this out. I will add additional docs to explain it. Basically for checked & expanded having false value is considered something different than not having value. While for disabled, selected and busy not having value equal to having false value.

Part of value proposition of this matcher is to encapsulate that behaviour.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've rewritten the docs, pls check if under current docs that makes more sense.

Copy link

Choose a reason for hiding this comment

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

Much clearer to me :)

src/__tests__/utils.ts Outdated Show resolved Hide resolved
src/utils.ts Outdated Show resolved Hide resolved
src/utils.ts Outdated Show resolved Hide resolved
@mdjastrzebski
Copy link
Collaborator Author

@MattAgn thanks for review

@mdjastrzebski
Copy link
Collaborator Author

@MattAgn Could you take another look, I've addressed all your comments.

@mdjastrzebski mdjastrzebski merged commit f050c41 into main Nov 10, 2022
@github-actions
Copy link

🎉 This PR is included in version 5.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@mdjastrzebski mdjastrzebski deleted the feat/to-have-accessibility-state branch November 10, 2022 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants