Skip to content

fix: *ByA11yState default value false value for busy, disabled & selected state - #1166

Merged
mdjastrzebski merged 8 commits into
mainfrom
fix/by-a11y-state-default-value
Oct 21, 2022
Merged

fix: *ByA11yState default value false value for busy, disabled & selected state#1166
mdjastrzebski merged 8 commits into
mainfrom
fix/by-a11y-state-default-value

Conversation

@mdjastrzebski

Copy link
Copy Markdown
Member

Summary

Resolves #1165.

Based on empirical research results: https://github.com/callstack/react-native-testing-library/wiki/Accessibility:-State

Test plan

Added additional test for matching behaviour.

@AugustinLF

AugustinLF commented Oct 10, 2022

Copy link
Copy Markdown
Collaborator

Might be worth sharing an implementation with #1161, no?

@mdjastrzebski mdjastrzebski added bug Something isn't working a11y labels Oct 15, 2022
@mdjastrzebski
mdjastrzebski marked this pull request as ready for review October 15, 2022 22:03
@mdjastrzebski

Copy link
Copy Markdown
Member Author

@AugustinLF Added exported matchAccessibilityState that might be shared with *ByRole implementation.

Comment thread src/queries/a11yState.ts Outdated
Comment thread src/queries/a11yState.ts Outdated
@mdjastrzebski

Copy link
Copy Markdown
Member Author

@AugustinLF would you mind if I would use the exported matchAccessibilityState predicate from *ByA11yState in *ByRole to have common logic? Seems like *ByA11yState and *ByRole share the same behaviour in terms of a11y state matching.

@mdjastrzebski
mdjastrzebski force-pushed the fix/by-a11y-state-default-value branch from 11fa4a0 to 864528c Compare October 20, 2022 09:53
@mdjastrzebski

Copy link
Copy Markdown
Member Author

@pierrezimmermannbam I've addressed your comment about default state object.

@mdjastrzebski

Copy link
Copy Markdown
Member Author

@AugustinLF I've used matchAccessibilityState from *ByA11yState to reduce duplicated state matching in *ByRole query.

@AugustinLF

Copy link
Copy Markdown
Collaborator

would you mind if I would use the exported matchAccessibilityState predicate from *ByA11yState in *ByRole to have common logic? Seems like *ByA11yState and *ByRole share the same behaviour in terms of a11y state matching.

@mdjastrzebski Yep good for me, makes sense

Comment thread src/queries/role.ts Outdated
@AugustinLF

Copy link
Copy Markdown
Collaborator

Nice work!

Comment thread src/helpers/matchers/accessibilityState.ts Outdated

@pierrezimmermannbam pierrezimmermannbam left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@mdjastrzebski I added just an additional suggestion of refactor, otherwise changes look good to me, well done !

@mdjastrzebski
mdjastrzebski merged commit bd5ddd7 into main Oct 21, 2022
@mdjastrzebski
mdjastrzebski deleted the fix/by-a11y-state-default-value branch October 21, 2022 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a11y bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: *ByA11yState query ignores default state values of false

3 participants