-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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(#16232): use a11y params > element properly #17989
Conversation
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 06eeb2f. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this branch ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
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.
Thanks for this, @jsomsanith! I left a comment that's worth discussing, but it generally looks good!
Co-authored-by: Kyle Gach <[email protected]>
@kylegach thanks for the review |
Co-authored-by: Kyle Gach <[email protected]>
- Updates story's parameters to match changes in previous commit (9feb188) - Minor wording/style changes
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.
Thanks for this improvement, @jsomsanith! 👏
I pushed a commit with a fix and some minor wording/style updates. I'll merge as soon as CI does its thing.
I don't think the CI failures are related to this PR, so I'm going to merge. |
Thank for the review :) |
Issue: #16232
What I did
The a11y param > element was not used properly. It is supposed to be an element id too serve as scan root, but the code takes it as the element itself.
Furthermore, if no element is passed in params, it queries a
#story-root
element first that comes from nowhere (no ref found in the repo) before taking#root
.I removed this unknown
#story-root
and take theelement
param properly as element id.How to test
If your answer is yes to any of these, please make sure to include it in your PR.
Added a story about a11 parameters in official-storybook example