-
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
feat(accordion): add size variants to Accordion #7038
feat(accordion): add size variants to Accordion #7038
Conversation
Deploy preview for carbon-elements ready! Built with commit fc94749 |
Deploy preview for carbon-components-react ready! Built without sensitive environment variables with commit fc94749 https://deploy-preview-7038--carbon-components-react.netlify.app |
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 good to me pending visual review
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 looks great and I want to approve. I just have one question, is it possible to call this a "Large" variant vs. an "Extra Large" variant? Since we're starting to add large variants everywhere, we should be consistent. The 64px button variant is only called "Large" and the 48px search I believe might even be default.
Thoughts on this?
@jeanservaas we'll be able to do that on the next major release, but right now we're just aligning it to all the other form input sizes ( |
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.
Related to #5455
Adds in
sm
(32px) andxl
(48px) variants toAccordion
Changelog
New
size
prop added toAccordion
. Adds in two variantssm
andxl
.Changed
Testing / Reviewing
Ensure all size variants look correct