Skip to content

Add support for ByRole with name - #1127

Merged
mdjastrzebski merged 3 commits into
callstack:mainfrom
AugustinLF:feat/by-role-with-name
Sep 22, 2022
Merged

Add support for ByRole with name#1127
mdjastrzebski merged 3 commits into
callstack:mainfrom
AugustinLF:feat/by-role-with-name

Conversation

@AugustinLF

@AugustinLFAugustinLF commented Sep 18, 2022

Copy link
Copy Markdown
Collaborator

Summary

Folllow up of #875. I started from scratch given the amount of conflicts.

Closes#827

If merged as is, don't forget to mention @kiranjd as part of the release.

Kudo to @MattAgn's work in #977 which made that a breeze. Look at the diff between this PR and the previously opened one, the code reorg was def the right call.

mdjastrzebski
mdjastrzebski requested changes Sep 19, 2022
View reviewed changes

@mdjastrzebskimdjastrzebski left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Awesome work @AugustinLF!

The code is indeed much simpler after @MattAgn's #977.

I've added some minor suggests and tweaks to address before we merge this.

Comment threadsrc/queries/__tests__/role.test.tsx Outdated
Comment threadwebsite/docs/Queries.md Outdated
Comment threadtypings/index.flow.js Outdated
Comment threadtypings/index.flow.js Outdated
Comment threadtypings/index.flow.js Outdated
Comment threadsrc/queries/role.ts Outdated
Comment threadsrc/queries/__tests__/role.test.tsx Outdated
Comment threadsrc/queries/__tests__/role.test.tsx
Comment threadsrc/queries/__tests__/role.test.tsx
Comment threadsrc/queries/role.ts Outdated
@AugustinLFAugustinLF mentioned this pull request Sep 20, 2022
Closed
mdjastrzebski
mdjastrzebski reviewed Sep 22, 2022
View reviewed changes
Comment threadsrc/queries/role.ts Outdated
mdjastrzebski
mdjastrzebski requested changes Sep 22, 2022
View reviewed changes

@mdjastrzebskimdjastrzebski left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Requested one small minor readability change.

@AugustinLF

AugustinLF commented Sep 22, 2022

Copy link
Copy Markdown
CollaboratorAuthor

@mdjastrzebski we should be good!

mdjastrzebski
mdjastrzebski approved these changes Sep 22, 2022
View reviewed changes

@mdjastrzebskimdjastrzebski left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great job @AugustinLF 🚀 Also big thank you 🙏🏻 to @kiranjd for submitting the initial implementation of this PR as #875.

@mdjastrzebski
mdjastrzebski merged commit c9ab3cf into callstack:mainSep 22, 2022
@mdjastrzebskimdjastrzebski mentioned this pull request Sep 22, 2022
Closed
@AugustinLF
AugustinLF deleted the feat/by-role-with-name branch September 22, 2022 12:43
@mdjastrzebskimdjastrzebski mentioned this pull request Sep 22, 2022
Closed
@mdjastrzebski

mdjastrzebski commented Sep 26, 2022

Copy link
Copy Markdown
Member

🎉 This PR is included in version 11.2.0 🎉

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mdjastrzebskimdjastrzebskimdjastrzebski approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

getByRole should accept a second argument to refine query as in react-testing-library

2 participants

@AugustinLF@mdjastrzebski