-
Notifications
You must be signed in to change notification settings - Fork 784
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
fix(aria-required-children): do not fail for children with aria-hidden #3949
Conversation
@@ -40,7 +40,7 @@ export { default as isNode } from './is-node'; | |||
export { default as isOffscreen } from './is-offscreen'; | |||
export { default as isOpaque } from './is-opaque'; | |||
export { default as isSkipLink } from './is-skip-link'; | |||
export { default as isVisibleToScreenReaders } from './is-visible-for-screenreader'; | |||
export { default as isVisibleToScreenReaders } from './is-visible-to-screenreader'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that the file names were still using for
instead of to
so changed those to match the name of the function while I was there.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
@@ -15,6 +20,10 @@ function getOwnedRoles(virtualNode, required) { | |||
const ownedElements = getOwnedVirtual(virtualNode); | |||
for (let i = 0; i < ownedElements.length; i++) { | |||
const ownedElement = ownedElements[i]; | |||
if (ownedElement.props.nodeType !== 1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needed to ignore non-element nodes as isVisibletoScreenreaders
calls into isHiddenForEveryone
which tries to get the computed style for display
, which fails for non-element nodes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. If you haven't already, can you look if this same problem may need to be fixed on the list and definition-list rules? Separate issue if so.
<ul>
<div aria-hidden="true">foo</div>
<li>bar</li>
</ul> passes the <dl>
<span aria-hidden="true">foo</span>
<dt>bar</dt>
<dd>baz</dd>
</dl> passes the |
Has this fix been released? I upgraded axe-core to 4.7.2 and I still the issue. Please let me know. |
@ilantom This should have been fixed in 4.7.1 I believe. Could you provide a code sample of it failing? It was validated for the original issue #3850 (comment). |
Is your site public that I could take a look at it? Running axe-core on the microsoft page itself still does not show a violation for |
Closes: #3850