Skip to content
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

Merged
merged 2 commits into from
Nov 21, 2022

Conversation

cee-chen
Copy link
Member

@cee-chen cee-chen commented Nov 21, 2022

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

  • Checked in both light and dark modes
  • Checked in mobile
  • Checked in Chrome, Safari, Edge, and Firefox
  • A changelog entry exists and is marked appropriately

@@ -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}
Copy link
Contributor

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?

Copy link
Member Author

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

Copy link
Member Author

@cee-chen cee-chen Nov 21, 2022

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!

Copy link
Contributor

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.

Copy link
Member Author

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.

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6422/

Copy link
Contributor

@1Copenut 1Copenut left a 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.

@cee-chen cee-chen merged commit abb925a into elastic:main Nov 21, 2022
@cee-chen cee-chen deleted the flyout-anim-fix branch November 21, 2022 19:59
cee-chen pushed a commit to elastic/kibana that referenced this pull request Nov 23, 2022
## 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))
1Copenut added a commit to elastic/kibana that referenced this pull request Dec 2, 2022
`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>
edmarmoretti pushed a commit to edmarmoretti/eui that referenced this pull request May 3, 2023
… flyouts (elastic#6422)

* [EuiFlyout][EuiCollapsibleNav] Fix broken slide in animation for left flyouts

* changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Collapsable Sidebar with smooth transition (Page Template)
4 participants