-
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
[EuiFlyout][EuiCollapsibleNav] Fix broken slide in animation for left flyouts #6422
Conversation
@@ -144,7 +144,8 @@ export const euiFlyoutStyles = (euiThemeContext: UseEuiTheme) => { | |||
clip-path: polygon(0 0, 150% 0, 150% 100%, 0 100%); | |||
|
|||
${euiCanAnimate} { | |||
animation: ${euiFlyoutSlideInLeft}; | |||
animation: ${euiFlyoutSlideInLeft} ${euiTheme.animation.normal} |
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.
@constancecchen should we move both animations from this file to src/global_styling/utility/animations.ts
and name it more generic?
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.
Are they used elsewhere other than flyouts? I don't see the point if not
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.
@miukimiu going to go ahead and merge this PR as-is - if we find other instances of left/right sliding that can be DRYed out with EuiFlyout I'd be good with opening another follow-up PR!
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.
The idea is to start having animations easily to consume. So that in the future we can provide some animations for custom components implementations, like in https://animate.style/ (but with our animations, and also we need to provide guidelines).
For example, the new onboarding project has a custom flyout with a non-EUI animation. Could they be reusing this animation?
But I guess I have to put more thought into this.
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.
Ahh gotcha! I definitely don't have as high-level of an overview as you do but my thought is that it might be easier to wait until we have more components converted before making any decisions. I'd also add that a utility class (.eui-animSlideLeft
) rather than a JS mixin feels like it would be more useful for consumers, so I feel like this is something we'd want to tackle with all animations at once + a new documentation page.
Preview documentation changes for this PR: https://eui.elastic.co/pr_6422/ |
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.
LGTM. Tested the flyout in Safari and Firefox with VO running. Both displayed smooth entry and exit, announced properly.
## Summary `eui@67.1.9` ⏩ `eui@67.1.10` This backport fixes EuiFlyout bugs affecting Observability. It is meant to go directly into 8.6. ## [`67.1.10`](https://github.com/elastic/eui/tree/v67.1.10) - Added the `euiMaxBreakpoint` and `euiMinBreakpoint` CSS-in-JS utilities for creating min/max-width media queries ([#6431](elastic/eui#6431)) **Bug fixes** - Fixed missing slide-in animation on `EuiCollapsibleNav`s and left-side `EuiFlyout`s ([#6422](elastic/eui#6422)) - Fixed multiple component media queries for consumers with custom theme breakpoints ([#6431](elastic/eui#6431))
`eui@70.2.4` ⏩ `eui@70.4.0` - "Fixed EuiButtonGroup firing onChange twice" required changing some tests from `click` to `change` ___ ## [`70.4.0`](https://github.com/elastic/eui/tree/v70.4.0) - Updated `EuiTourStep.footerAction` type to accept `ReactNode[]` ([#6384](elastic/eui#6384)) - Vertically aligned all footer content so that `euiTourStepIndicator` is always centered ([#6384](elastic/eui#6384)) - Added `filterInCircle` glyph to `EuiIcon` ([#6385](elastic/eui#6385)) - Added `color` prop to `EuiBeacon` ([#6420](elastic/eui#6420)) - Added the `euiMaxBreakpoint` and `euiMinBreakpoint` CSS-in-JS utilities for creating min/max-width media queries ([#6431](elastic/eui#6431)) **Bug fixes** - Restores the previous match operator behaviour when the query value is split into multiple terms after analysis. ([#6409](elastic/eui#6409)) - Fixed missing slide-in animation on `EuiCollapsibleNav`s and left-side `EuiFlyout`s ([#6422](elastic/eui#6422)) - Fix bug in `EuiCard` where footer were not aligned to the bottom of the card ([#6424](elastic/eui#6424)) - Fixed multiple component media queries for consumers with custom theme breakpoints ([#6431](elastic/eui#6431)) ## [`70.3.0`](https://github.com/elastic/eui/tree/v70.3.0) - `EuiSearchBar` now automatically wraps special characters not used by query syntax in quotes ([#6356](elastic/eui#6356)) - Added `alignment` prop to `EuiBetaBadge` ([#6361](elastic/eui#6361)) - `EuiButton` now accepts `minWidth={false}` ([#6373](elastic/eui#6373)) **Bug fixes** - Fixed `EuiPageTemplate` not correctly passing the `component` prop to the inner main content wrapper. ([#6352](elastic/eui#6352)) - `EuiSkipLink` now correctly calls `onClick` even when `fallbackDestination` is invalid ([#6355](elastic/eui#6355)) - Permanently fixed `EuiModal` to not cause scroll-jumping issues on modal open ([#6360](elastic/eui#6360)) - Re-fixed `EuiPageSection` not correctly merging `contentProps.css` ([#6365](elastic/eui#6365)) - Fixed `EuiTab` not defaulting to size `m` ([#6366](elastic/eui#6366)) - Fixed the shadow sizes of `.eui-yScrollWithShadows` and `.eui-xScrollWithShadows` ([#6374](elastic/eui#6374)) - Fixed bug in `EuiCard` where the inner content in vertical cards was not growing 100% in width ([#6377](elastic/eui#6377)) - Fixed incorrect margins in `EuiSuperDatePicker` caused by `EuiFlex` CSS gap change ([#6380](elastic/eui#6380)) - Fixed visual bug in nested `EuiFlexGroup`s, where the parent `EuiFlexGroup` is responsive but a child `EuiFlexGroup` is not ([#6381](elastic/eui#6381)) **CSS-in-JS conversions** - Converted `EuiModal` to Emotion ([#6321](elastic/eui#6321)) **Fixes** - `EuiButton` no longer outputs unnecessary inline styles for `minWidth={0}` or `minWidth={false}` ([#6373](elastic/eui#6373)) - `EuiFacetButton` no longer reports type issues when passing props accepted by `EuiButton` ([#6373](elastic/eui#6373)) Co-authored-by: Constance Chen <constance.chen@elastic.co>
… flyouts (elastic#6422) * [EuiFlyout][EuiCollapsibleNav] Fix broken slide in animation for left flyouts * changelog
Summary
closes #6419
Before:
Broken animation missing transform in from left: https://eui.elastic.co/v70.0.0/#/navigation/collapsible-nav
After:
Original Sass animation: https://eui.elastic.co/v60.0.0/#/navigation/collapsible-nav
Fixed staging link: https://eui.elastic.co/pr_6422/#/navigation/collapsible-nav
QA