-
-
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
Addon-a11y: Move stories into addon #19114
Conversation
As a11y stories are no longer present, I had to target something else for the same purpose.
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 feel like there are a lot of stories deleted here that we need to figure out how to reimplement somehow. Obviously a11y is tricky as the stories depend on HTML structure, is this blocked by creating an Html
component?
code/examples/official-storybook/stories/addon-a11y/base-button.stories.js
Show resolved
Hide resolved
code/examples/official-storybook/stories/addon-a11y/base-button.stories.js
Show resolved
Hide resolved
code/examples/official-storybook/stories/addon-a11y/highlight.stories.js
Show resolved
Hide resolved
@tmeasday I argued to @shilman that we should test the things that matter, and are storybook implemented. I think there's no point in testing axe internals. But yes, I do think this is blocked for now, and likely should not be merged, or if we do, we should create follow-up tasks to ensure we get the proper coverage again. |
# Conflicts: # code/renderers/react/template/components/index.js # code/renderers/svelte/template/components/index.js # code/renderers/vue3/template/components/index.js
@tmeasday quick question for you about the sandbox script. I can only see 3: Controls, Actions & Interactions.. |
@ndelangen I think the issue is you added |
Addendum to the above, the |
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 is pretty close, I just wonder if it might be better to move some of the parameter-based stories to the other file. @ndelangen I'll leave it to you to make the final call!
export const Targetted = { | ||
parameters: { | ||
a11y: { | ||
element: 'button', | ||
}, | ||
}, | ||
}; |
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.
Is it obvious this is doing something? Would it be better to do these kind of things in the HTML-based story file?
All right, i think I've improved things a bit, will merge when green! |
code/addons/a11y/template/stories