-
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
fix(FormGroup): only place className on outer wrapper #9526
Conversation
❌ Deploy Preview for carbon-react-next failed. 🔨 Explore the source changes: ad08cae 🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-react-next/deploys/613b3648648e14000891e768 |
✔️ Deploy Preview for carbon-elements ready! 🔨 Explore the source changes: ad08cae 🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/613b3648256483000870cee7 😎 Browse the preview: https://deploy-preview-9526--carbon-elements.netlify.app |
✔️ Deploy Preview for carbon-components-react ready! 🔨 Explore the source changes: ad08cae 🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/613b36489b94e60007be31d0 😎 Browse the preview: https://deploy-preview-9526--carbon-components-react.netlify.app |
bump @joshblack when you have a chance 👍🏻 |
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.
Seems great! Quick question on the test stories, do we want those showing up in the production storybook or are you removing/disabling them before merge? 👀
So sorry about the delay @tw15egan! Totally missed this 🤦 |
@joshblack will get rid of them before merging 🙂 |
1fc9cf4
to
ad08cae
Compare
Closes #4572
This ensures the
className
prop is only placed on the outer wrapper, and not duplicated. I've placed this behind a feature flag, but we may just want to go ahead and make this change as it would be difficult to style them independently as-isChangelog
New
FormGroup
Changed
className
prop is only placed on the outer wrappingfieldset
elementTesting / Reviewing
Go to the test story and ensure the custom class is only placed on the outer wrapper