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

feat: deprecate and replace dom.isVisible, utils.isHidden, and dom.isHiddenWithCss #3351

Merged
merged 39 commits into from
Sep 21, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
6693b99
chore: refactor isVisible
straker Jan 14, 2022
68f30a2
comment
straker Jan 14, 2022
c3f9f39
fix firefox
straker Jan 14, 2022
c87d1e8
Update lib/commons/dom/is-visible.js
straker Jan 17, 2022
7a9dcef
update per suggestions
straker Jan 18, 2022
3d668f0
Update lib/commons/dom/is-visible.js
straker Jan 21, 2022
0e8892a
Update lib/commons/dom/is-visible.js
straker Jan 21, 2022
2134df8
Merge branch 'refs/heads/develop' into refactor-visible
straker Jul 25, 2022
cd7871b
fixes
straker Jul 28, 2022
db3006d
revert changes
straker Aug 25, 2022
d5f74c8
is-hidden-for-everyone
straker Aug 26, 2022
b9ad968
functions done
straker Aug 31, 2022
e500486
revert files
straker Aug 31, 2022
3c34939
fix lint
straker Aug 31, 2022
a830961
finish refactor
straker Sep 1, 2022
d99849b
missed
straker Sep 1, 2022
ca9b4fd
merge develop
straker Sep 1, 2022
ca5467c
fixes
straker Sep 1, 2022
33ce4d7
revert files
straker Sep 1, 2022
64fc544
Merge branch 'develop' into refactor-visible
straker Sep 6, 2022
9edbbfe
finish tests
straker Sep 6, 2022
df4ebe5
fix polyfill
straker Sep 6, 2022
d663d86
fix test
straker Sep 6, 2022
5d4aca6
Update lib/commons/dom/is-modal-open.js
straker Sep 8, 2022
c3371e8
suggestions
straker Sep 8, 2022
04e8c2a
Update lib/commons/dom/visibility-functions.js
straker Sep 8, 2022
d63581f
suggestions
straker Sep 8, 2022
fe8cdff
merge develop
straker Sep 8, 2022
432cacc
merge develop
straker Sep 8, 2022
86b9051
suggestions
straker Sep 8, 2022
f8dadac
memoize correctly
straker Sep 9, 2022
f514f5f
memoize for real this time
straker Sep 9, 2022
b825fbb
uni tests
straker Sep 19, 2022
41328e6
guard
straker Sep 19, 2022
3c5899c
reset eslintrc
straker Sep 19, 2022
c59d8b6
Merge branch 'develop' into refactor-visible
straker Sep 19, 2022
fdddacf
merge
straker Sep 19, 2022
992613d
changes
straker Sep 20, 2022
fff04f1
fix lint
straker Sep 20, 2022
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
6 changes: 4 additions & 2 deletions lib/checks/navigation/heading-order-evaluate.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,10 @@ function headingOrderEvaluate() {
// @see https://github.com/dequelabs/axe-core/issues/728
const selector = 'h1, h2, h3, h4, h5, h6, [role=heading], iframe, frame';
// TODO: es-modules_tree
const vNodes = querySelectorAllFilter(axe._tree[0], selector, vNode =>
isVisibleForScreenreader(vNode.actualNode)
const vNodes = querySelectorAllFilter(
axe._tree[0],
selector,
isVisibleForScreenreader
);

headingOrder = vNodes.map(vNode => {
Expand Down
13 changes: 7 additions & 6 deletions lib/commons/dom/has-lang-text.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { hasChildTextNodes } from './has-content-virtual';
import isVisualContent from './is-visual-content';
import isVisibleForScreenreader from './is-visible-for-screenreader';
import isHiddenForEveryone from './is-hidden-for-everyone';

/**
* Check that a node has text, or an accessible name which language is defined by the
Expand All @@ -19,9 +19,10 @@ export default function hasLangText(virtualNode) {
// See: https://github.com/dequelabs/axe-core/issues/3281
return !!axe.commons.text.accessibleTextVirtual(virtualNode);
}
return virtualNode.children.some(child => (
!child.attr('lang') && // non-empty lang
hasLangText(child) && // has text
isVisibleForScreenreader(child) // Not hidden for AT
));
return virtualNode.children.some(
child =>
!child.attr('lang') && // non-empty lang
hasLangText(child) && // has text
!isHiddenForEveryone(child) // Not hidden for anyone
);
}
55 changes: 33 additions & 22 deletions lib/commons/dom/is-hidden-for-everyone.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,44 +5,55 @@ import {
nativelyHidden,
displayHidden,
visibilityHidden
} from './visibility-functions';
} from './visibility-methods';

const hiddenMethods = [displayHidden, visibilityHidden];

/**
* Determine if an element is hidden from screenreaders and visual users
* @method isHiddenForEveryone
Copy link
Contributor

Choose a reason for hiding this comment

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

I remember why we called this isHiddenWithCss. It's because for example an off screen element with aria-hidden is hidden for everyone, but this method is going to return false on it.

I'm not sure what to suggest for this, other than that maybe we need another name for this still. This one can be potentially just as misleading as what we have today.

* @memberof axe.commons.dom
* @param {VirtualNode} vNode The Virtual Node
* @param {Boolean} [ancestors=true] If the ancestor tree should be used
* @param {Boolean} recursed
* @param {Object} [options]
* @param {Boolean} [options.skipAncestors] If the ancestor tree should be not be used
* @param {Boolean} [options.isAncestor] If this function is being called on an ancestor for the target node
* @return {Boolean} The element's visibility state
*/
export default function isHiddenForEveryone(vNode, ancestors = true, recursed) {
export default function isHiddenForEveryone(
vNode,
{ skipAncestors, isAncestor } = {}
) {
vNode = vNode instanceof AbstractVirtualNode ? vNode : getNodeFromTree(vNode);

return ancestors
? isHiddenAncestors(vNode, recursed)
: isHiddenSelf(vNode, recursed);
if (skipAncestors) {
return isHiddenSelf(vNode, { isAncestor });
}

return isHiddenAncestors(vNode, { isAncestor });
}

/**
* Check the element for visibility state
*/
const isHiddenSelf = memoize(function isHiddenSelfMemoized(vNode, recursed) {
const isHiddenSelf = memoize(function isHiddenSelfMemoized(
vNode,
{ isAncestor }
) {
if (nativelyHidden(vNode)) {
return true;
}

if (vNode.actualNode) {
const hiddenMethods = [displayHidden, visibilityHidden];
if (!vNode.actualNode) {
return false;
}

if (hiddenMethods.some(method => method(vNode, recursed))) {
return true;
}
if (hiddenMethods.some(method => method(vNode, { isAncestor }))) {
return true;
}

// detached node
if (!vNode.actualNode.isConnected) {
return true;
}
// detached node
if (!vNode.actualNode.isConnected) {
return true;
}

return false;
Expand All @@ -53,15 +64,15 @@ const isHiddenSelf = memoize(function isHiddenSelfMemoized(vNode, recursed) {
*/
const isHiddenAncestors = memoize(function isHiddenAncestorsMemoized(
vNode,
recursed
{ isAncestor }
) {
if (isHiddenSelf(vNode, recursed)) {
if (isHiddenSelf(vNode, { isAncestor })) {
return true;
}

if (vNode.parent) {
return isHiddenAncestors(vNode.parent, true);
if (!vNode.parent) {
return false;
}

return false;
return isHiddenAncestors(vNode.parent, { isAncestor: true });
});
2 changes: 1 addition & 1 deletion lib/commons/dom/is-modal-open.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ function isModalOpen(options) {
// TODO: es-module-_tree
axe._tree[0],
'dialog, [role=dialog], [aria-modal=true]',
vNode => isVisibleOnScreen(vNode)
isVisibleOnScreen
);

if (definiteModals.length) {
Expand Down
10 changes: 7 additions & 3 deletions lib/commons/dom/is-offscreen.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import getComposedParent from './get-composed-parent';
import getElementCoordinates from './get-element-coordinates';
import getViewportSize from './get-viewport-size';
import AbstractVirtualNode from '../../core/base/virtual-node/abstract-virtual-node';

function noParentScrolled(element, offset) {
element = getComposedParent(element);
Expand All @@ -22,14 +23,17 @@ function noParentScrolled(element, offset) {
* @memberof axe.commons.dom
* @instance
* @param {Element} element
* @param {Object} [options]
* @param {Boolean} [options.isAncestor] If this function is being called on an ancestor of the target node
* @return {Boolean}
*/
function isOffscreen(element, recursed) {
if (recursed) {
function isOffscreen(element, { isAncestor } = {}) {
if (isAncestor) {
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why would this be false? I don't quite get the logic of this.

Copy link
Contributor Author

@straker straker Sep 8, 2022

Choose a reason for hiding this comment

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

It failed a test without it due to this logic and comment https://github.com/dequelabs/axe-core/blob/develop/lib/commons/dom/is-visible.js#L211-L216

// visibility is only accurate on the first element and
// position does not matter if it was already calculated

}

element = element.actualNode ? element.actualNode : element
element =
element instanceof AbstractVirtualNode ? element.actualNode : element;
straker marked this conversation as resolved.
Show resolved Hide resolved

let leftBoundary;
const docElement = document.documentElement;
Expand Down
23 changes: 15 additions & 8 deletions lib/commons/dom/is-visible-for-screenreader.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import AbstractVirtualNode from '../../core/base/virtual-node/abstract-virtual-n
import { getNodeFromTree } from '../../core/utils';
import memoize from '../../core/utils/memoize';
import isHiddenForEveryone from './is-hidden-for-everyone';
import { ariaHidden, isAreaVisible } from './visibility-functions';
import { ariaHidden, isAreaHidden } from './visibility-methods';

/**
* Determine if an element is visible to a screen reader
Expand All @@ -13,30 +13,37 @@ import { ariaHidden, isAreaVisible } from './visibility-functions';
*/
export default function isVisibleForScreenreader(vNode) {
vNode = vNode instanceof AbstractVirtualNode ? vNode : getNodeFromTree(vNode);
return isVisibleForScreenreaderVirtual(vNode);
return isVisibleForScreenreaderVirtual(vNode, {});
}

/**
* Check the element and ancestors
*/
const isVisibleForScreenreaderVirtual = memoize(
function isVisibleForScreenreaderMemoized(vNode, recursed) {
function isVisibleForScreenreaderMemoized(vNode, options) {
// cannot use memoize with a default parameter if we transpile
// the code. the transpiler removes the parameter from the function
// signature so the function is only memoized with the one
// parameter
options = options ?? {};
const isAncestor = options.isAncestor;

if (ariaHidden(vNode)) {
return false;
}

if (vNode.actualNode && vNode.props.nodeName === 'area') {
return isAreaVisible(vNode, isVisibleForScreenreaderVirtual);
return !isAreaHidden(vNode, isVisibleForScreenreaderVirtual);
}

if (isHiddenForEveryone(vNode, false, recursed)) {
if (isHiddenForEveryone(vNode, { skipAncestors: true, isAncestor })) {
return false;
}

if (vNode.parent) {
return isVisibleForScreenreaderVirtual(vNode.parent, true);
if (!vNode.parent) {
return true;
}

return true;
return isVisibleForScreenreaderVirtual(vNode.parent, { isAncestor: true });
}
);
48 changes: 28 additions & 20 deletions lib/commons/dom/is-visible-on-screen.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,18 @@ import {
scrollClipped,
overflowHidden,
clipHidden,
isAreaVisible
} from './visibility-functions';
isAreaHidden
} from './visibility-methods';
import isOffscreen from './is-offscreen';

const hiddenMethods = [
opacityHidden,
scrollClipped,
overflowHidden,
clipHidden,
isOffscreen
];

/**
* Determine if an element is visible on screen
* @method isVisibleOnScreen
Expand All @@ -25,33 +33,33 @@ export default function isVisibleOnScreen(vNode) {

const isVisibleOnScreenVirtual = memoize(function isVisibleOnScreenMemoized(
vNode,
recursed
options
) {
// cannot use memoize with a default parameter if we transpile
// the code. the transpiler removes the parameter from the function
// signature so the function is only memoized with the one
// parameter
options = options ?? {};
const isAncestor = options.isAncestor;

if (vNode.actualNode && vNode.props.nodeName === 'area') {
return isAreaVisible(vNode, isVisibleOnScreenVirtual);
return !isAreaHidden(vNode, isVisibleOnScreenVirtual);
}

if (isHiddenForEveryone(vNode, false, recursed)) {
if (isHiddenForEveryone(vNode, { skipAncestors: true, isAncestor })) {
return false;
}

if (vNode.actualNode) {
const hiddenMethods = [
opacityHidden,
scrollClipped,
overflowHidden,
clipHidden,
isOffscreen
];

if (hiddenMethods.some(method => method(vNode, recursed))) {
return false;
}
if (
vNode.actualNode &&
hiddenMethods.some(method => method(vNode, { isAncestor }))
) {
return false;
}

if (vNode.parent) {
return isVisibleOnScreenVirtual(vNode.parent, true);
if (!vNode.parent) {
return true;
}

return true;
return isVisibleOnScreenVirtual(vNode.parent, { isAncestor: true });
});
Loading