-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
test(e2e): add toContainAStory
assertion to visitStory
#16132
test(e2e): add toContainAStory
assertion to visitStory
#16132
Conversation
toContainAStory
assertion to visitStory
toContainAStory
assertion to visitStory
✅ Deploy Preview for v11-carbon-react ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
This fix has actually found 68 errors! It's why ci is failing. I'll work on fixing them |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Looked at this today with @Gururajj77 and noticed that when an Instead of fixing those accessibility errors in this PR, we should skip those tests to focus on getting this PR merged with the So the steps for this PR are now going to be:
|
opened new PR with changes, #16582 |
This adds a new assertion to
visitStory
that checks to make sure an element with the.cds--layout
class is present. The storybook config has a global decorator that applies this class via<Layout>
carbon/packages/react/.storybook/preview.js
Line 344 in f504b2e
This should ensure we avoid a bug where if you give
visitStory
invalid parameters (a wrongid
,component
, etc) the url won't point to a story but storybook still resolves it and gives their error screen:Huge thanks to @matthewgallo for bringing this up!
Changelog
New
toContainAStory
assertion tovisitStory
Testing / Reviewing
yarn playwright test