feat(ByRole): Improve perf if multiple elements are found #395
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What:
Improves perf >20% reduction if ByRole returns multiple elements. The more elements the bigger the reduction
Why:
Perf concerns were raised about
isInaccessible
How:
An element being inaccessible does not imply every descendant is inaccessible because elements can re-appear by setting
visibility: visible
in avisibility: hidden
subtree. However, they can't overridehidden
oraria-hidden=true
ordisplay: none
. So we cacheisSubtreeInaccessible
return values for each element. For collections of elements (e.g.option
orlistitem
) this reduces getComputedStyle calls fromO(n * depth(n))
toO(n) + O(depth(n))
where n is the number of elements of the given role. It enables a very fast best case for collections where the containing element (e.g. thelistbox
) is hidden. We only have to compute the style once for subsequentlistitems
and can exit early once we hit thelistbox
and the cachesubtreeIsInaccessible
returnstrue
.To avoid passing the computed style too much around I removed
getComputedStyle().visibility
compatibility for jsdom < 15.2. Once jest 25 is released this will be fixed for jest users anyway and CSS was one of the lesser concerns of other collaborators.We use a
WeakMap
for the cache. Those are supported in IE 11: https://caniuse.com/#search=WeakmapChecklist:
[ ] Documentation added to thedocs site
[ ] I've prepared a PR for types targetingDefinitelyTyped
@testing-library/dom
canary mui/material-ui#18021