-
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(containedlist): move to stable - add vrt, avt, storybook docs #12387
Conversation
✅ Deploy Preview for carbon-components-react ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
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, looking forward to having this component promoted to stable 👍
One question / comment: I noticed your removed the second list from both the "Default" and "Disclosed" story. The intent was to show their difference in spacing ("on page" having a top spacing if adjacent to another list while "disclosed" is flush). Do you think it would be worth to bring these back or does the Playground story cover this enough?
@janhassel Sorry, I totally missed this detail. I just pushed a commit bringing it back so the VRT will cover that styling and prevent regressions there. |
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 🚀
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.
Looks great!
Closes #12319
Final steps to mark ContainedList as stable
Changelog
New
carbon.yml
, added live demo .mdxChanged
/next
Experimental
toComponents
sectionaction
,children
,className
from all stories' controls paneRemoved
unstable_
prefix from ContainedListTesting / Reviewing
unstable_
prefix should be gone from both ContainedList and ContainedListItem