[USWDS] COMBOBOX | Fix textContent combobox#4287
Conversation
mejiaj
left a comment
There was a problem hiding this comment.
Looks good, seems to resolve the text issue.
thisisdano
left a comment
There was a problem hiding this comment.
This look good to me. Thanks!
@aduth any comments?
|
Looks good to me 👍 |
| // eslint-disable-next-line no-plusplus | ||
| for (let i = 0; i < list.children.length; i++) { | ||
| assert.strictEqual( | ||
| list.children[i].children.innerHTML, | ||
| undefined, | ||
| "should not contain child HTML" | ||
| ); | ||
| } |
There was a problem hiding this comment.
children.innerHTML will always be undefined, because children is the array-like collection. You may need a nested loop to check each list item's children. As an alternative to checking innerHTML, you could check that each child is a text node.
| // eslint-disable-next-line no-plusplus | |
| for (let i = 0; i < list.children.length; i++) { | |
| assert.strictEqual( | |
| list.children[i].children.innerHTML, | |
| undefined, | |
| "should not contain child HTML" | |
| ); | |
| } | |
| Array.from(list.children).forEach((listItem) => { | |
| Array.from(listItem.children).forEach((childNode) => { | |
| assert.strictEqual(childNode, Node.TEXT_NODE); | |
| }); | |
| }); |
There was a problem hiding this comment.
TIL!! this change worked for us locally - committing the suggestion
There was a problem hiding this comment.
Hm, something's not quite right with this actually. That assertion should be checking childNode.nodeType. In debugging, it looks like the assertion's not being called, because Array.from(listItem.children) is empty, even though that's valid in the real-world 🤷 Might need to revert back to plain-ol' for loop?
There was a problem hiding this comment.
oof, this thing is trickier than I expected, taking another gander
There was a problem hiding this comment.
Ah, I see the issue. children doesn't include the text nodes (source). Works after switching to childNodes.
Array.from(list.children).forEach((listItem) => {
Array.from(listItem.childNodes).forEach((childNode) => {
assert.strictEqual(childNode.nodeType, Node.TEXT_NODE);
});
});
Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
Manually creates
liand includesoption.textas the saferli.textContentincludes a new test to check for potentially rendered
innerHtmlof list child elements.Before you hit Submit, make sure you’ve done whichever of these applies to you:
npm testand make sure the tests for the files you have changed have passed.