Skip to content
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

ByLabelText does not work with IE 11 #723

Closed
eps1lon opened this issue Jul 29, 2020 · 7 comments · Fixed by #726
Closed

ByLabelText does not work with IE 11 #723

eps1lon opened this issue Jul 29, 2020 · 7 comments · Fixed by #726
Labels
bug Something isn't working help wanted Extra attention is needed released

Comments

@eps1lon
Copy link
Member

eps1lon commented Jul 29, 2020

  • @testing-library/dom version: 7.21.6
  • Testing Framework and version: any
  • DOM Environment: IE11 or Firefox 52

Relevant code or config:

<label for="rating-test-2">2 Stars</label>
<input id="rating-test-test" type="radio" />

What you did:

getByLabelText('2 Stars')

What happened:

  Found a label with the text of: 2 Stars, however the element associated with this label (<input />) is non-labellable [https://html.spec.whatwg.org/multipage/forms.html#category-label]. If you really need to label a <input />, you can use aria-label or aria-labelledby instead.

Reproduction:

55e72b2 (#21944)

Problem description:

HTMLInputElement.labels is not supported in IE 11 or prior to Firefox 56

Suggested solution:

Use CSS selector. It's even used later in the code and it's not clear why we use two different approaches.

@eps1lon eps1lon added the bug Something isn't working label Jul 29, 2020
@kentcdodds
Copy link
Member

Dang it. 😅

@kentcdodds kentcdodds added the help wanted Extra attention is needed label Jul 29, 2020
@eps1lon
Copy link
Member Author

eps1lon commented Jul 29, 2020

Dang it.

dom-accessibility-api didn't support native labels either in these environments. Polyfilling this is actually quite involved: https://github.com/eps1lon/dom-accessibility-api/pull/352/files

Though that fix also targets Edge 17. If you just target Edge 18 and IE 11 you can leverage HTMLLabelElement.control which simplifies things by a lot.

@kentcdodds
Copy link
Member

That actually doesn't look all that bad. Thanks for sharing! We could just put that code at the bottom of the label query module or in a helpers file and if we can't access things by .label we could use that as a fallback.

Anyone wanna work on this? It's possible this could also be useful as another module and both our libraries could consume I guess.

@BatuhanW
Copy link

BatuhanW commented Aug 2, 2020

I'd like to work on this one, but unfortunately, I'm not totally clear about how this one should be fixed. Some help is appreciated!

@maxnewlands
Copy link
Contributor

I'd like to work on this one, but unfortunately, I'm not totally clear about how this one should be fixed. Some help is appreciated!

@BatuhanW Apologies, I didn't mean to step on your toes with my PR! I'd started this a few days ago, so just thought I'd go for it. Happy to discuss if you'd find it helpful 🙂

@kentcdodds
Copy link
Member

@BatuhanW, I'm sure there will be another opportunity to contribute in the future :) Watch the repo and you can be notified when there's an opportunity :) We'd love to have your contribution!

kentcdodds pushed a commit that referenced this issue Aug 5, 2020
* fix(ByLabelText): support IE

Polyfill HTMLInputElement.labels to support IE and other legacy browsers. Closes #723.

* fix: ensure mocks are restored

* test: remove unnecessary test cases

Co-authored-by: Maxwell Newlands <maxwelln@spotify.com>
@kentcdodds
Copy link
Member

🎉 This issue has been resolved in version 7.21.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants