-
Notifications
You must be signed in to change notification settings - Fork 862
[Emotion] Convert EuiButton
#6150
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
Conversation
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6150/ |
|
@thompsongl just giving you some context. I approved #5989 and it was just waiting for your review. This PR is basically the same PR (#5989) but I squashed the commits because there were some issues when I merged Also, there was an issue that I found in that PR but not a blocker: #6123. I'll tackle this in a new PR. Also, I pushed some minor changes: |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6150/ |
thompsongl
left a comment
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.
Thanks for moving this forward, @miukimiu!
This PR is basically a squash of all the commits from #5989. When I merged
mainand because Caroline is no longer working at Elastic it was necessary to sign the contribution agreement. Because she did all the commits when she was still an Elastic employee I don't see an issue on squash all the commits into one.1. Moved Amsterdam-specific Sass overrides to default
global_styling/folderThis just cleans up some of the unnecessary overrides and Sass files. But mostly leaves the variable / mixins in tact for now.
2. New theme-specific (Amsterdam only for now) button styling functions
disabledas a specificBUTTON_COLORbut allow it to be passed intoeuiButtonColorfor coloringeuiButtonColor()now returns bothbackgroundColorandcolorfor proper text contrasteuiButtonFillColor()creates thefill(solid color, white/black text) styling comboeuiButtonEmptyColor()creates theemptystyle using text-variants for color and transparent hover backgroundsuseEuiButtonColorCSS()now accepts options such as thedisplay: 'base' | 'fill' | 'empty'and returns both keys for color and display.useEuiButtonRadiusCSS()returns theborder-radiusvalues bysizeof buttonuseEuiButtonFocusCSS()is just the translate animation3. Button component updates
EuiButton
ButtonColortoEuiButtonColorandButtonSizetoEuiButtonSizeEuiButtonDisplaycomponent as the render and passing on all the rest of the props to handle the button vs anchor logiccolor,fill, anddisabledand removing associated classes.textcolor fill buttonsThe new tinted colors are just slightly different from their old transparent counterpart. The biggest difference being in the
textversion which will now align better to forms.EuiButtonEmpty
color, anddisabledand removing associated classes.EuiButtonIcon
color,disabled, anddisplayremoving associated classes.EuiButtonGroup & Option
ghost. The combo styles between Emotion and CSS were too complex to maintain, but it felt safe to remove support (based an searching for usages - none) and will guide consumers to use colorMode instead once fully converted.color,disabled, andisSelected(asdisplay='fill')EuiButtonDisplay
Since this is the component that renders the actual element (button vs anchor) I have moved all the logic around the associated props of each from EuiButton to this file so all consumers of this can benefit / doesn't have to worry about the actual DOM element.
EuiButtonDisplayContent
I moved the currently applied
disabledstyles to be handled by EuiButtonDisplay instead while also applying truncation to the text span using the utility class.Other updates
isButtonDisabled()utility for DRY-ing out the logic which compares thedisabled,isDisabled,isLoading, and validity ofhrefto determine finaldisabledstate.data-test-subjprops to empty buttons for testsChecklist
ghostcolors[ ] Checked Code Sandbox works for any docs examples