Recent danger button update breaks react testing library getRyRole

[finken2]

In the most recent carbon react build (7.32.1), a span was added inside <Button> when it's a danger button. From the storybook:

<button tabindex="0" class="bx--btn bx--btn--danger" type="button" aria-describedby="danger-description-2">
<span id="danger-description-2" class="bx--visually-hidden">danger</span>
Button</button>

The proper way to use react-testing-library is to use getByRole('button', {name: 'blah'}); With this update, danger buttons are no longer found.
https://testing-library.com/docs/queries/byrole

One possible fix is adding a aria-label to the Button tag, but there's other possibilities.

[finken2]

One temporary workaround is to use a regex for the name property. This isn't optimal though, b/c it requires the user to know the inner structure of the button and as we've seen, that can change.
getByRole('button', { name: /Remove$/ })
In this case, i need the $ to ensure i don't match other buttons that start with Remove. It's also saying that we expect the name to be the last value in the button contents, which it currently is, but... yeah, still better to fix this.

[joshblack]

Hi @finken2! 👋 I believe #8222 added in the dangerDescription prop with the fallback of danger to close #8220.

I believe the original test could be updated to use the regex as you mentioned, or to include danger or provide a custom dangerDescription for the text to resolve correctly.

Let me know if I'm misunderstanding how RTL resolves what the text of something is, it's been a while since I've taken a look at how that works 👀

[finken2]

@joshblack RTL allows you to specify "name" when using getByRole, which should go through the right assistive technology flow of looking at labeledby, aria-label, etc, eventually landing on text node in the button. Since we don't have any of the other labeling, it ends up looking in the body and doesn't match.
Here's that logic: https://www.w3.org/TR/accname-1.1/#mapping_additional_nd_te

I agree, we can work around it with the regex, but that's really error-prone and not how it should work. I'd rather add aria-label to the Button if we can.

[joshblack]

@finken2 that's awesome, based on that it seems like doing getByRole('button', { name: 'danger Remove' }) should do the trick, and if the text needs to be configured then one can use the dangerDescription prop and include the value of that in the name.

When aria-label, aria-labelledby, etc come up I think we would run into the same problem with determining the label because the dangerDescription prop will be appended based on the intent on #8220.

[finken2]

@joshblack yep, that does work. Good point about the appending w/ aria-label too. Once we put that "danger" description in there, we're probably stuck with something unintuitive for the test.
using "danger " seems reasonable, definitely better than the regex workaround.

Thanks for talking through it.