-
Notifications
You must be signed in to change notification settings - Fork 841
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
[CSS-in-JS] Convert classNameMaps
to direct className
keys.
#5889
Conversation
Preview documentation changes for this PR: https://eui.elastic.co/pr_5889/ |
I'm good with this, but just to check, for modifier classes that we find 0 usages of in Kibana, can we prefer to delete the className maps instead of converting them? |
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.
Changes LGTM! Love the cleanup and QA'd all touched components - classNames output looks good!
Yes, yes of course 😄 |
Preview documentation changes for this PR: https://eui.elastic.co/pr_5889/ |
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.
I like it!
Sweet! Thanks! I'll merge this and update the Meta issue with guidance. |
Conversion Idea
Instead of maintaining the manual maps of property value to class name maps, lets switch to applying the modifier piece of the class name directly as they key.
Example:
Becomes:
This removes the idea that we're somehow very specifically using class names by prop value, while still supporting the rendered class name. It also reduces lines of code.
The only time this won't work well is with something like
margin = 'left'
and class name iseuiComponent--marginLeft
. But I think we can at least reduce the classname maps to just signify the modifier instead of repeating the component name like:Checklist
[ ] Checked in both light and dark modes[ ] Checked in mobile[ ] Checked in Chrome, Safari, Edge, and Firefox[ ] Props have proper autodocs and playground toggles[ ] Added documentation[ ] Checked Code Sandbox works for any docs examples[ ] Checked for breaking changes and labeled appropriately[ ] Checked for accessibility including keyboard-only and screenreader modes[ ] Updated the Figma library counterpart[ ] A changelog entry exists and is marked appropriately