Skip to content
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(classNames): move classNames to outer wrapper #9491

Merged
merged 15 commits into from
Sep 10, 2021

Conversation

tw15egan
Copy link
Collaborator

@tw15egan tw15egan commented Aug 17, 2021

Refs #9465
Refs #9502
Closes #5853

Uses the feature-flags package to move the className prop to the outer wrapper.

Changelog

New

  • Moved the className prop to the outermost wrapper so that all elements inside are targetable via css

Changed

  • If the className prop was being added inside an element, it now returns null when the V11 feature flag is enabled. Instead, it is placed on the outer wrapper

Testing / Reviewing

Test the following components, which each have a test story that will be removed before merging:

  • Checkbox
  • Combobox / Dropdown / Multiselect
  • DatePicker
  • NumberInput
  • RadioButtonGroup
  • Slider
  • TimePicker

Ensure className is placed on the outer wrapper if wrapped in the FeatureFlag element. className should remain in the same spot otherwise

@netlify
Copy link

netlify bot commented Aug 17, 2021

❌ Deploy Preview for carbon-react-next failed.

🔨 Explore the source changes: 1e40e6b

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-react-next/deploys/613b3bf8e393060007a55192

@tw15egan
Copy link
Collaborator Author

cc @andreancardona

@netlify
Copy link

netlify bot commented Aug 17, 2021

✔️ Deploy Preview for carbon-elements ready!

🔨 Explore the source changes: 1e40e6b

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/613b3bf854eb7c000736b0e2

😎 Browse the preview: https://deploy-preview-9491--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented Aug 17, 2021

✔️ Deploy Preview for carbon-components-react ready!
Built without sensitive environment variables

🔨 Explore the source changes: 1e40e6b

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/613b3bf83903b0000841d099

😎 Browse the preview: https://deploy-preview-9491--carbon-components-react.netlify.app

@tw15egan tw15egan force-pushed the className-updates branch 2 times, most recently from 43fa8f1 to 706409f Compare August 18, 2021 11:14
@tw15egan tw15egan marked this pull request as ready for review August 18, 2021 11:57
@tw15egan tw15egan requested review from a team as code owners August 18, 2021 11:57
Copy link
Contributor

@jnm2377 jnm2377 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested the component stories and they're working as expected! 🎉 🎉 🔥

@tw15egan
Copy link
Collaborator Author

bump @joshblack when you have a chance 👍🏻

Copy link
Contributor

@joshblack joshblack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So sorry about the delay! Looks great 👀

@kodiakhq kodiakhq bot merged commit ee69d34 into carbon-design-system:main Sep 10, 2021
This was referenced Sep 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[carbon-react] Input className prop inconsistencies
3 participants