-
-
Notifications
You must be signed in to change notification settings - Fork 9.1k
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
Addon-a11y: Fix type of context passed to axe.run
#16129
Conversation
Nx Cloud ReportCI ran the following commands for commit cbc2388. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this branch
Sent with 💌 from NxCloud. |
By the way, what's this code supposed to do? storybook/addons/a11y/src/a11yRunner.ts Line 20 in 4750c6b
Is If this is only related to this addon, should it be documented? |
Not that I could find: https://github.com/storybookjs/storybook/search?q=story-root 🤷♂️ |
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.
This change LGTM, though we may wish to simplify what may be un-needed code in a later effort (see: #16129 (comment)).
Thanks, @kaelig!
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 filed a follow-up issue to remove #story-root
, since we couldn't figure out what that is or what it's supposed to do. Merging this now! Thanks @kaelig !!! 🙏
axe.run
axe.run
What I did
According to axe's API reference, the context passed to
axe.run
can be aNodeList
, but the addon-a11y is passing anHTMLCollection
.This PR ensures we're passing a NodeList and shouldn't impact functionality.
Related: dequelabs/axe-core#3161
How to test
If your answer is yes to any of these, please make sure to include it in your PR.