-
Notifications
You must be signed in to change notification settings - Fork 842
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
[Emotion] Convert EuiSplitPanel #7172
Conversation
- styles are fairly basic, so converting them all in one go + no theme needed + remove `flex-direction` overrides in favor of two keys with JS logic + reorder imports + remove modifier classes
92e8da8
to
b094674
Compare
- wasn't being imported anywhere, so wasn't being used
Preview documentation changes for this PR: https://eui.elastic.co/pr_7172/ |
💚 Build Succeeded
History
|
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.
This is good to go! This seems like a fairly easy Emotion conversion without tons of caveats (for once 😅). Double checked the Storybook examples and all props, including responsive ones, toggle as expected.
EUI `88.2.0` ➡️ `88.3.0` ## [`88.3.0`](https://github.com/elastic/eui/tree/v88.3.0) - `EuiGlobalToastList` now shows a "Clear all" button by default once above a certain number of toasts (defaults to 3). This threshold is configurable with the `showClearAllButtonAt` prop ([#7111](elastic/eui#7111)) - Added an optional `onClearAllToasts` callback to `EuiGlobalToastList` ([#7111](elastic/eui#7111)) - Added the `value`, `onChange`, and `onCancel` props that allow `EuiInlineEdit` to be used as a controlled component ([#7157](elastic/eui#7157)) - Added `grabOmnidirectional`, `transitionLeftIn`, `transitionLeftOut`, `transitionTopIn`, and `transitionTopOut` icon glyphs. ([#7168](elastic/eui#7168)) **Bug fixes** - Fixed `EuiInlineEdit` components to correctly spread `...rest` attributes to the parent wrapper ([#7157](elastic/eui#7157)) - Fixed `EuiListGroupItem` to correctly render the `extraAction` button when `showToolTip` is also passed ([#7159](elastic/eui#7159)) **Dependency updates** - Updated `@hello-pangea/dnd` to v16.3.0 ([#7125](elastic/eui#7125)) - Updated `@types/lodash` to v4.14.198 ([#7126](elastic/eui#7126)) **Accessibility** - `EuiAccordion` now correctly respects reduced motion settings ([#7161](elastic/eui#7161)) - `EuiAccordion` now shows a focus outline to keyboard users around its revealed children on open ([#7161](elastic/eui#7161)) **CSS-in-JS conversions** - Converted `EuiSplitPanel` to Emotion ([#7172](elastic/eui#7172))⚠️ As a quick heads up, serverless tests appear to have been extremely flake/failure-prone the last couple weeks, particularly Cypress tests. We've evaluated the listed failures and fixed ones that were related to changes in this PR, and we're relatively confident the remaining failures are not related to changes from EUI. Please let us know if you think this is not the case. --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Cee Chen <constance.chen@elastic.co> Co-authored-by: Jon <jon@elastic.co>
⚠️ NOTE: This PR is a copy of #166292 (which was reverted due to failing Storybook builds). This is the same exact PR but with Storybook building fixed. --- EUI `88.2.0` ➡️ `88.3.0` ## [`88.3.0`](https://github.com/elastic/eui/tree/v88.3.0) - `EuiGlobalToastList` now shows a "Clear all" button by default once above a certain number of toasts (defaults to 3). This threshold is configurable with the `showClearAllButtonAt` prop ([#7111](elastic/eui#7111)) - Added an optional `onClearAllToasts` callback to `EuiGlobalToastList` ([#7111](elastic/eui#7111)) - Added the `value`, `onChange`, and `onCancel` props that allow `EuiInlineEdit` to be used as a controlled component ([#7157](elastic/eui#7157)) - Added `grabOmnidirectional`, `transitionLeftIn`, `transitionLeftOut`, `transitionTopIn`, and `transitionTopOut` icon glyphs. ([#7168](elastic/eui#7168)) **Bug fixes** - Fixed `EuiInlineEdit` components to correctly spread `...rest` attributes to the parent wrapper ([#7157](elastic/eui#7157)) - Fixed `EuiListGroupItem` to correctly render the `extraAction` button when `showToolTip` is also passed ([#7159](elastic/eui#7159)) **Dependency updates** - Updated `@hello-pangea/dnd` to v16.3.0 ([#7125](elastic/eui#7125)) - Updated `@types/lodash` to v4.14.198 ([#7126](elastic/eui#7126)) **Accessibility** - `EuiAccordion` now correctly respects reduced motion settings ([#7161](elastic/eui#7161)) - `EuiAccordion` now shows a focus outline to keyboard users around its revealed children on open ([#7161](elastic/eui#7161)) **CSS-in-JS conversions** - Converted `EuiSplitPanel` to Emotion ([#7172](elastic/eui#7172)) --------- Co-authored-by: Bree Hall <briannajdhall@gmail.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Jon <jon@elastic.co>
Summary
closes #6406
Really quick and simple conversion of
EuiSplitPanel
to Emotion. There's no theme tokens being used so their styles can just be a static object.This PR also removes an unused Sass file that apparently did not get deleted in the original
EuiPanel
conversion. There's likely moreEuiPanel
cleanup work to be done (e.g. removing modifier classes, removing global mixins/variables), but that should likely happen after all current Sass components using panel variables are converted to Emotion.QA
gh pr checkout 7172 && yarn storybook
General checklist
Emotion checklist
Kibana usage
**/target, **/*.snap, **/*.storyshot
for less noise) foreui{Component}
(case sensitive) to find:{euiComponent}-
(case sensitive) to check for usage of modifier classes - No modifier usageeuiBadge
class on a div instead of simply using theEuiBadge
component) - NoneGeneral
className(s)
read as expected in snapshots and browsers[ ] Checked component playgroundNo playground, but added StorybooksUnit tests
shouldRenderCustomStyles()
test was added and passes with parent component and any nestedchildProps
(e.g.tooltipProps
)Sass/Emotion conversion process
[ ] Converted all global Sass vars/mixins to JS (e.g.$euiSize
toeuiTheme.size.base
)[ ] Removed or converted component-specific Sass vars/mixins to exported JS versions[ ] Listed var/mixin removals in changelog[ ] Ranyarn compile-scss
to update var/mixin JSON files[ ] Simplifiedcalc()
tomathWithUnits
if possible (if mixing different unit types, this may not be possible)[ ] Added an@warn
deprecation message within theglobal_styling/mixins/{component}.scss
file[ ] All SCSS overrides have been removed from the Amsterdam themeNo overridesCSS tech debt
[ ] Wrapped all animations or transitions ineuiCanAnimate
[ ] Usedgap
property to add margin between items if using flex-inline
and-block
logical properties (check inline styles as well as CSS)DOM Cleanup
euiComponent
,euiComponent__child
)Extras/nice-to-have
[ ] Documentation pass[ ] Check for issues in the backlog that could be a quick fix for that component[ ] Optional component/code cleanup: consider splitting up the component into multiple children if it's overly verbose or difficult to reason about