Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BREAKING][#666] Default full to attached nodes only; add legacy-full #764

Merged
merged 1 commit into from
Aug 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/dull-wombats-approve.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'tabbable': major
---

🚨 BREAKING: Default `displayCheck` 'full' option no longer treats detached nodes as visible. Use the new 'legacy-full' option to restore old (incorrect) behavior only if you must. Ideally, make sure tabbable only runs once all nodes of interest have been attached to the document.
13 changes: 9 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ These options apply to all APIs.

### displayCheck option

Type: `full` | `non-zero-area` | `none` . Default: `full`.
Type: `full` | `full-with-hidden` | `non-zero-area` | `none` . Default: `full`.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the new name was legacy-full?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's legacy-full. Thanks for catching that. Will fix it right away.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1d15204


Configures how to check if an element is displayed.

Expand All @@ -145,9 +145,14 @@ For this reason Tabbable offers the ability of an alternative way to check if an

The `displayCheck` configuration accepts the following options:

- `full`: (default) Most reliably resembling browser behavior, this option checks that an element is displayed, which requires all of his ancestors are displayed as well (notice that this doesn't exclude `visibility: hidden` or elements with zero size). This option will cause layout reflow, however. If that is a concern, consider the `none` option.
- ⚠️ If the container given to `tabbable()` or `focusable()`, or the node given to `isTabbable()` or `isFocusable()`, is not attached to the window's main `document`, the display check will default to __"none"__ (see below for details) because the APIs used to determine a node's display are not supported unless it is attached (i.e. the browser does not calculate its display unless it is attached). This has effectively been tabbable's behavior for a _very_ long time, and you may never have encountered an issue if the nodes with which you used tabbable were always displayed anyway (i.e. the "none" mode assumption was coincidentally correct).
- You may encounter the above situation if, for example, you render to a node via React, and this node is [not attached](https://github.com/facebook/react/issues/9117#issuecomment-284228870) to the document (or perhaps, due to timing, it is not _yet_ attached at the time you use tabbable's APIs).
- `full`: (default) Most reliably resembling browser behavior, this option checks that an element is displayed, which requires it to be attached to the DOM, and for all of his ancestors to be displayed (notice this doesn't exclude `visibility: hidden` or elements with zero size). This option will cause layout reflow, however. If that is a concern, consider the `none` option.
- ⚠️ If the container given to `tabbable()` or `focusable()`, or the node given to `isTabbable()` or `isFocusable()`, is not attached to the window's main `document`, the node will be considered hidden and neither tabbable nor focusable. This behavior is new as of `v6.0.0`.
- If your code relies on the legacy behavior where detached nodes were considered visible, and you are unable to fix your code to use tabbable once the node is attached, use the `legacy-full` option.
- `legacy-full`: Same as `full` but restores the __legacy behavior__ of treating detached nodes as visible. This means that if a node is detached, it's then treated as though the display check was set to `none` (see below for details).
- ❗️ Since detached nodes are not treated as tabbable/focusable by browsers, using this option is __not recommended__ as it knowingly diverges from browser behavior.
- ⚠️ This option may be removed in the future. Tabbable will not maintain it at the expense of new features or if having it makes the code disproportionately more complex. It only exists to make the upgrade path to the correct behavior (i.e. the `full` option) as long and smooth as reasonably possible.
- The APIs used to determine a node's display are not supported unless its attached (i.e. the browser does not calculate its display unless it is attached). This has effectively been tabbable's behavior for a _very_ long time (up until the `v6.0.0` release), and you may never have encountered an issue if the nodes with which you used tabbable were always displayed anyway (i.e. the `none` mode assumption was coincidentally correct).
- You may encounter the above situation if, for example, you render to a node via React, and this node is [not attached](https://github.com/facebook/react/issues/9117#issuecomment-284228870) to the document (or perhaps, due to timing, it is not _yet_ attached at the time you use tabbable's APIs on it).
- `non-zero-area`: This option checks display under the assumption that elements that are not displayed have zero area (width AND height equals zero). While not keeping true to browser behavior, this option may enhance accessibility, as zero-size elements with focusable content are considered a strong accessibility anti-pattern.
- Like the `full` option, this option also causes layout reflow, and should have basically the same performance. Consider the `none` option if reflow is a concern.
- ⚠️ As with the `full` option, there is a nuance in behavior depending on whether tabbable APIs are executed on attached vs detached nodes using this mode: Attached nodes that are actually displayed will be deemed visible. Detached nodes, _even though displayed_ will always be deemed __hidden__ because detached nodes always have a zero area as the browser does not calculate is dimensions.
Expand Down
2 changes: 1 addition & 1 deletion index.d.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
type FocusableElement = HTMLElement | SVGElement;

export type CheckOptions = {
displayCheck?: 'full' | 'non-zero-area' | 'none';
displayCheck?: 'full' | 'legacy-full' | 'non-zero-area' | 'none';
getShadowRoot?: boolean | ((node: FocusableElement) => ShadowRoot | boolean | undefined);
};

Expand Down
78 changes: 53 additions & 25 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,44 @@ const isNonTabbableRadio = function (node) {
return isRadio(node) && !isTabbableRadio(node);
};

// determines if a node is ultimately attached to the window's document
const isNodeAttached = function (node) {
// The root node is the shadow root if the node is in a shadow DOM; some document otherwise
// (but NOT _the_ document; see second 'If' comment below for more).
// If rootNode is shadow root, it'll have a host, which is the element to which the shadow
// is attached, and the one we need to check if it's in the document or not (because the
// shadow, and all nodes it contains, is never considered in the document since shadows
// behave like self-contained DOMs; but if the shadow's HOST, which is part of the document,
// is hidden, or is not in the document itself but is detached, it will affect the shadow's
// visibility, including all the nodes it contains). The host could be any normal node,
// or a custom element (i.e. web component). Either way, that's the one that is considered
// part of the document, not the shadow root, nor any of its children (i.e. the node being
// tested).
// To further complicate things, we have to look all the way up until we find a shadow HOST
// that is attached (or find none) because the node might be in nested shadows...
// If rootNode is not a shadow root, it won't have a host, and so rootNode should be the
// document (per the docs) and while it's a Document-type object, that document does not
// appear to be the same as the node's `ownerDocument` for some reason, so it's safer
// to ignore the rootNode at this point, and use `node.ownerDocument`. Otherwise,
// using `rootNode.contains(node)` will _always_ be true we'll get false-positives when
// node is actually detached.
let nodeRootHost = getRootNode(node).host;
let attached = !!(
nodeRootHost?.ownerDocument.contains(nodeRootHost) ||
node.ownerDocument.contains(node)
);

while (!attached && nodeRootHost) {
// since it's not attached and we have a root host, the node MUST be in a nested shadow DOM,
// which means we need to get the host's host and check if that parent host is contained
// in (i.e. attached to) the document
nodeRootHost = getRootNode(nodeRootHost).host;
attached = !!nodeRootHost?.ownerDocument.contains(nodeRootHost);
}

return attached;
};

const isZeroArea = function (node) {
const { width, height } = node.getBoundingClientRect();
return width === 0 && height === 0;
Expand All @@ -271,29 +309,11 @@ const isHidden = function (node, { displayCheck, getShadowRoot }) {
return true;
}

// The root node is the shadow root if the node is in a shadow DOM; some document otherwise
// (but NOT _the_ document; see second 'If' comment below for more).
// If rootNode is shadow root, it'll have a host, which is the element to which the shadow
// is attached, and the one we need to check if it's in the document or not (because the
// shadow, and all nodes it contains, is never considered in the document since shadows
// behave like self-contained DOMs; but if the shadow's HOST, which is part of the document,
// is hidden, or is not in the document itself but is detached, it will affect the shadow's
// visibility, including all the nodes it contains). The host could be any normal node,
// or a custom element (i.e. web component). Either way, that's the one that is considered
// part of the document, not the shadow root, nor any of its children (i.e. the node being
// tested).
// If rootNode is not a shadow root, it won't have a host, and so rootNode should be the
// document (per the docs) and while it's a Document-type object, that document does not
// appear to be the same as the node's `ownerDocument` for some reason, so it's safer
// to ignore the rootNode at this point, and use `node.ownerDocument`. Otherwise,
// using `rootNode.contains(node)` will _always_ be true we'll get false-positives when
// node is actually detached.
const nodeRootHost = getRootNode(node).host;
const nodeIsAttached =
nodeRootHost?.ownerDocument.contains(nodeRootHost) ||
node.ownerDocument.contains(node);

if (!displayCheck || displayCheck === 'full') {
if (
!displayCheck ||
displayCheck === 'full' ||
displayCheck === 'legacy-full'
) {
if (typeof getShadowRoot === 'function') {
// figure out if we should consider the node to be in an undisclosed shadow and use the
// 'non-zero-area' fallback
Expand Down Expand Up @@ -333,7 +353,7 @@ const isHidden = function (node, { displayCheck, getShadowRoot }) {
// NOTE: We must consider case where node is inside a shadow DOM and given directly to
// `isTabbable()` or `isFocusable()` -- regardless of `getShadowRoot` option setting.

if (nodeIsAttached) {
if (isNodeAttached(node)) {
// this works wherever the node is: if there's at least one client rect, it's
// somehow displayed; it also covers the CSS 'display: contents' case where the
// node itself is hidden in place of its contents; and there's no need to search
Expand All @@ -354,6 +374,13 @@ const isHidden = function (node, { displayCheck, getShadowRoot }) {
// APIs on nodes in detached containers has actually implicitly used tabbable in what
// was later (as of v5.2.0 on Apr 9, 2021) called `displayCheck="none"` mode -- essentially
// considering __everything__ to be visible because of the innability to determine styles.
//
// v6.0.0: As of this major release, the default 'full' option __no longer treats detached
// nodes as visible with the 'none' fallback.__
if (displayCheck !== 'legacy-full') {
return true; // hidden
}
// else, fallback to 'none' mode and consider the node visible
} else if (displayCheck === 'non-zero-area') {
// NOTE: Even though this tests that the node's client rect is non-zero to determine
// whether it's displayed, and that a detached node will __always__ have a zero-area
Expand All @@ -363,7 +390,8 @@ const isHidden = function (node, { displayCheck, getShadowRoot }) {
return isZeroArea(node);
}

// visible, as far as we can tell, or per current `displayCheck` mode
// visible, as far as we can tell, or per current `displayCheck=none` mode, we assume
// it's visible
return false;
};

Expand Down
143 changes: 74 additions & 69 deletions test/e2e/focusable.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,79 +21,84 @@ describe('focusable', () => {
});

describe('example fixtures', () => {
[true, false].forEach((inDocument) => {
it(`correctly identifies focusable elements in the "basic" example ${
inDocument ? '(container IN doc)' : '(container NOT in doc)'
}`, () => {
let expectedFocusableIds;

if (inDocument) {
expectedFocusableIds = [
'contenteditable-true',
'contenteditable-nesting',
'contenteditable-negative-tabindex',
'contenteditable-NaN-tabindex',
'input',
'input-readonly',
'select',
'select-readonly',
'href-anchor',
'tabindex-hrefless-anchor',
'textarea',
'textarea-readonly',
'button',
'tabindex-div',
'negative-select',
'hiddenParentVisible-button',
'displaycontents-child',
'audio-control',
'audio-control-NaN-tabindex',
'video-control',
'video-control-NaN-tabindex',
];
} else {
expectedFocusableIds = [
'contenteditable-true',
'contenteditable-nesting',
'contenteditable-negative-tabindex',
'contenteditable-NaN-tabindex',
'input',
'input-readonly',
'select',
'select-readonly',
'href-anchor',
'tabindex-hrefless-anchor',
'textarea',
'textarea-readonly',
'button',
'tabindex-div',
'negative-select',
'displaynone-textarea',
'visibilityhidden-button',
'hiddenParent-button',
'hiddenParentVisible-button',
'displaycontents',
'displaycontents-child',
'displaycontents-child-displaynone',
'audio-control',
'audio-control-NaN-tabindex',
'video-control',
'video-control-NaN-tabindex',
];
}
[undefined, 'full', 'legacy-full'].forEach((displayCheck) => {
[true, false].forEach((inDocument) => {
it(`correctly identifies focusable elements in the "basic" example ${
inDocument ? '(container IN doc' : '(container NOT in doc'
}, displayCheck=${displayCheck || '<default>'})`, () => {
let expectedFocusableIds;

if (inDocument) {
expectedFocusableIds = [
'contenteditable-true',
'contenteditable-nesting',
'contenteditable-negative-tabindex',
'contenteditable-NaN-tabindex',
'input',
'input-readonly',
'select',
'select-readonly',
'href-anchor',
'tabindex-hrefless-anchor',
'textarea',
'textarea-readonly',
'button',
'tabindex-div',
'negative-select',
'hiddenParentVisible-button',
'displaycontents-child',
'audio-control',
'audio-control-NaN-tabindex',
'video-control',
'video-control-NaN-tabindex',
];
} else if (displayCheck === 'legacy-full') {
expectedFocusableIds = [
'contenteditable-true',
'contenteditable-nesting',
'contenteditable-negative-tabindex',
'contenteditable-NaN-tabindex',
'input',
'input-readonly',
'select',
'select-readonly',
'href-anchor',
'tabindex-hrefless-anchor',
'textarea',
'textarea-readonly',
'button',
'tabindex-div',
'negative-select',
'displaynone-textarea',
'visibilityhidden-button',
'hiddenParent-button',
'hiddenParentVisible-button',
'displaycontents',
'displaycontents-child',
'displaycontents-child-displaynone',
'audio-control',
'audio-control-NaN-tabindex',
'video-control',
'video-control-NaN-tabindex',
];
} else {
// should find nothing because the container will be detached
expectedFocusableIds = [];
}

const container = document.createElement('div');
container.innerHTML = fixtures.basic;
const container = document.createElement('div');
container.innerHTML = fixtures.basic;

if (inDocument) {
document.body.append(container);
}
if (inDocument) {
document.body.append(container);
}

const focusableElements = focusable(container);
const focusableElements = focusable(container, { displayCheck });

expect(getIdsFromElementsArray(focusableElements)).to.eql(
expectedFocusableIds
);
expect(getIdsFromElementsArray(focusableElements)).to.eql(
expectedFocusableIds
);
});
});
});

Expand Down
Loading