fix(commons/dom): change isFocusable to use unfocusableBecauseHidden #654
Conversation
0ddfell0w
force-pushed
the
invisible-focusable
branch
2 times, most recently
from
60fe2c8 to
783920e
Compare
method instead of isVisible Adds functionality for detecting when hidden elements are still focusable or in focus order. fixes dequelabs#647
0ddfell0w
force-pushed
the
invisible-focusable
branch
from
783920e to
28de37b
Compare
|
Ok, this commit is totally confusing
It seems like this same change could be achieved by calling |
|
Sure, that's fine too |
|
@0ddfell0w @WilcoFiers I do wonder whether this new method might introduce some performance improvements though - thoughts on this versus "duplicate code"? |
|
#655 for your suggestion |
| * @param {Element} el | ||
| * @return {Boolean} Whether the element is unfocusable due to being hidden | ||
| */ | ||
| function unfocusableBecauseHidden (el) { |
There was a problem hiding this comment.
If we keep this for performance - what about parent-child relationships? This function is a bit naive - look at isVisible for all the conditions that must be handled. Also add tests to handle all those conditions.
There was a problem hiding this comment.
Also, is it possible to do a faster implementation like https://github.com/jquery/jquery/blob/2d4f53416e5f74fa98e0c1d66b6f3c285a12f0ce/src/css/hiddenVisibleSelectors.js#L8
change isFocusable to use unfocusableBecauseHidden function instead of isVisible
Adds functionality for detecting when hidden elements are still
focusable or in focus order.
fixes #647
Demo showing that off-screen and clipped elements remain in the focus order, where display:none and visibility:hidden elements do not (they are totally unfocusable)