[Emotion] Pre-emptively fix/regression test form components with tricky className/css locations
#6254
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This adds an unused Emotion className, but I wanted to ensure 1. The css merging approach is in place and not forgotten when converted to EUI (since ...rest) is not passed to the className wrapper 2. the test is in place and not forgetten when it's converted to Emotion
See previous commit message for EuiCheckbox
see previous EuiCheckbox commit message for rationale
2 tasks
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6254/ |
2 tasks
Looks complimentary. Probably just a single merge conflict that will be easy to sort out. |
|
Thanks for the confirmation Greg! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Related PRs: #6239, #6248, #6253, #6255
Several of our form components (note: with nested childProps, I didn't look for any others) have somewhat tricky DOM structure in that the wrapper their
classNames go on is not the same as the location that...restis spread to. This means that to accuratelyshouldRenderCustomStylestest these components, I had to create a fake Emotion CSS className to merge with a custom/passedcssprop.I thought this worth doing now rather than later (i.e. during the actual Emotion conversion of the components) to ensure that:
cssarray merge approach is in place and not forgotten when converted to Emotionalso CC @miukimiu and @thompsongl since IIRC y'all are working on EuiDualRange currently - not sure if this conflicts with your work or compliments it.
Checklist
[ ] A changelog entry exists and is marked appropriatelyN/A, in theory not a bugfix