-
Notifications
You must be signed in to change notification settings - Fork 144
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
add ByA11yState
to ALL_QUERIES_METHODS
const
#956
base: main
Are you sure you want to change the base?
Conversation
- `prefer-screen-queries` was failing for destructured a11y queries Signed-off-by: Viren Mohindra <[email protected]>
Hi @virenpepper! Could you give us more context on this query? Where is it coming from? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #956 +/- ##
==========================================
+ Coverage 96.23% 96.33% +0.09%
==========================================
Files 44 46 +2
Lines 2419 2481 +62
Branches 1000 1028 +28
==========================================
+ Hits 2328 2390 +62
Misses 91 91 ☔ View full report in Codecov by Sentry. |
describe('test', () => {
it('prefer-screen-queries does not throw an error here', async () => {
const { findByA11yState } = render(<TestComponent/>);
expect(queryAllByA11yState({ disabled: true }).length).toBe(1);
const test = await findByA11yState({
selected: true,
});
});
}); but a couple things to callout,
so the tl;dr is maybe this PR can be closed? we are also facing issues where the const { rerender } = render(<TestComponent />); unsure how to proceed here, as of now we hacked the local library to catch these gaps [1] https://callstack.github.io/react-native-testing-library/docs/api/queries#by-accessibility-state
|
@virenpepper As you correctly stated: this plugin (currently) doesn't support I'll let @Belco90 decide what to do with this, but imo we should not add this and add full support in 1 go instead |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @virenpepper! Sorry it took me so long to react to this.
I've seen there are several PRs for adding support for React Native in this plugin (this PR, #958, others in the past). Instead of having separated PRs for adding support here and there, we should unify the changes to support RN entirely in one go, similar to what was done in #973.
So we need to create an umbrella issue to gather what's missing in this plugin to fully support RN, then move the current work there, including the RN preset.
Checks
Changes
ALL_QUERIES_METHODS
so we can successfully target those queries while pattern matchingContext
prefer-screen-queries
was failing for destructured a11y queries