Support textareas with children #344
Conversation
| // The children of a textarea are part of `textContent` as well. We | ||
| // need to remove them from the string so we can match it afterwards. | ||
| label.querySelectorAll('textarea').forEach(textarea => { | ||
| textToMatch = textToMatch.replace(textarea.value, '') |
There was a problem hiding this comment.
Hmm... What if the label is the same as the text value at that moment?
There was a problem hiding this comment.
I think we should be able to use label.childNodes[0].textContent here?
There was a problem hiding this comment.
@alexkrolick replace only replaces the first occurence:
'abcabc'.replace('abc', '') // "abc"Therefore this should be fine.
@timdeschryver Sorry, I can't follow? Generally I think it's good to avoid childNodes so we support arbitrary nesting for both the textarea and the label. There could be some divs and spans wrapped around for example.
There was a problem hiding this comment.
This is a little odd, but I think I'm ok with this change. I can't think of situations where it would lead to people breaking our goals with testing and it doesn't add anything to the API. It also doesn't really violate the principle of least surprise, so I think I'm good with it. I'd love a 👍 from another maintainer though.
|
Thanks for the review! Is there another maintainer who wants to have a look at this? Would be great to get this fixed, so I can remove a workaround in my code base 🙂. Thanks! |
|
@all-contributors please add @amannn for code and tests |
|
I've put up a pull request to add @amannn! 🎉 |
|
🎉 This PR is included in version 6.2.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
amannn
changed the title
What:
Fixes this bug: #343 (comment)
Checklist:
Documentation addedTypescript definitions updated