fix: for react-testing-library to find dialog by findByRole with labe…

[hedaukartik]

…lName as priority in rules on top of findByLabelText

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
dialog	✅ Ready (Inspect)	Visit Preview	May 13, 2022 at 7:28PM (UTC)

[lgtm-com]

This pull request introduces 1 alert when merging 963e7a0 into 061bd82 - view on LGTM.com

new alerts:

• 1 for Unused variable, import, function or class

[codecov]

Codecov Report

Merging #290 (ec1e012) into master (061bd82) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #290   +/-   ##
=======================================
  Coverage   98.05%   98.05%           
=======================================
  Files           6        6           
  Lines         154      154           
  Branches       46       46           
=======================================
  Hits          151      151           
  Misses          2        2           
  Partials        1        1           

Impacted Files	Coverage Δ
src/Dialog/index.tsx	100.00% <ø> (ø)
src/Dialog/Content/index.tsx	95.34% <100.00%> (+0.11%)	⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

[hedaukartik]

@zombieJ
With reference to the issue raise in antd repo- ant-design/ant-design#35392

By this PR we will still be able to query modal by role screen.getByRole('dialog', { name: 'dialog1'}) in https://dialog-ddwo9c8j1-react-component.vercel.app/demo/ant-design

[hedaukartik]

@zombieJ can you please check this breaking change?

[JeffreyATW]

@hedaukartik Shouldn't you be removing the aria-labelledby from https://github.com/react-component/dialog/blob/master/src/Dialog/index.tsx as well?

[dgreene1]

@hedaukartik which screen readers did you test this in?

[JeffreyATW]

Let's say I have a dialog with a title of "Foo".

When I test the dialog prior to the change in #285 using VoiceOver on macOS Safari, it reads "Foo, web dialog".

When I test the dialog after the change in #285, it reads "web dialog".

After this change, it reads "Foo, web dialog" again.

[dgreene1]

Thank you for that information @JeffreyATW. I’ll back you up and ask Zombie to reopen the issue this is attached to.

btw, since it seems unlikely that the maintainers will let us bring RTL into this, can you add a unit test that emulated the get by role (with name) option? That way we can make sure that the regression (that this PR fixes) does not happen again?

I’m not sure what that would look like but I imagine it would be “find the label by ID of the dialog and make sure that it’s the ID of the title.”

[hedaukartik]

@hedaukartik Shouldn't you be removing the aria-labelledby from https://github.com/react-component/dialog/blob/master/src/Dialog/index.tsx as well?

Yes. It seems unnecessary. Removed it now. Thanks for explaining this issue in detail @JeffreyATW

[hedaukartik]

@zombieJ please check

[zombieJ]

+ rc-dialog@8.8.2