fix: Let findUp work on shadow root children #447
Conversation
# Conflicts: # test/testutils.js
WilcoFiers
force-pushed
the
sd/find-up-bug
branch
from
b423673
to
02daad1
Compare
| @@ -9,33 +9,24 @@ | |||
| * @return {HTMLElement|null} Either the matching HTMLElement or `null` if there was no match | |||
| */ | |||
| dom.findUp = function (element, target) { | |||
| 'use strict'; | |||
There was a problem hiding this comment.
These changes don't seem to actually change anything in terms of functionality - is this purely a performance fix?
There was a problem hiding this comment.
ok, so the difference here is that if the starting node is the highest node in the shadow DOM, then the old code did not find a matching host node - whereas this code does?
There was a problem hiding this comment.
The problem is that if the child is a directly on the shadow root, it would start looking from the host (outside the shadow), even though the matches array still only contained elements inside the shadow tree.
Followup thought, should this use element.matches() instead of doing querySelectAll() and see if the element is included? I imagine that'd be faster.
test/commons/dom/find-up.js
Outdated
| }); | ||
|
|
||
| (shadowSupport.v1 ? it : xit)('should work on shadow root children', function () { | ||
| fixture.innerHTML = '<div role="list"><div id="something"></div></div>'; |
There was a problem hiding this comment.
shouldn't this attach to the div with role="list"?
Should help resolve #422.