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.

Sign up for GitHub

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

Jump to bottom

[EuiFlyout] Allow push flyouts to have slide in animations #7239

Merged
merged 6 commits into from
Oct 4, 2023
Merged

[EuiFlyout] Allow push flyouts to have slide in animations #7239

merged 6 commits into from
Oct 4, 2023

Conversation

cee-chen
Copy link
Member

@cee-chen cee-chen commented Sep 30, 2023 •
edited by 1Copenut
Loading

Summary

closes #7205

screencap

QA

General checklist

  • Browser QA
    • Checked in Chrome, Safari, Edge, and Firefox
  • Docs site QA
  • Code quality checklist
  • Release checklist
    • A changelog entry exists and is marked appropriately.
  • Designer checklist - skipped - IIRC Figma doesn't have animations?
    - [ ] Updated the Figma library counterpart

@cee-chen cee-chen mentioned this pull request Sep 30, 2023
Closed
@cee-chen
[misc cleanup] Fix incorrect push styles
f2d966c 
- push flyouts that turn into overlay flyouts on smaller screens aren't using the correct styles on prod :(
@cee-chen
Add pushAnimation prop
3bacc4f 
- which allow consumers to configure push flyouts with slide in animations
@cee-chen
Update/add tests
949ae98
@cee-chen
Update downstream snapshots
09f92cc
@cee-chen
changelog
f23dbbf
@cee-chen
[docs] Add storybook for testing
36bc48b 
NOTE: I strongly prefer storybook over the docs site for testing the animation, as the docs site causes a ton of page jumping that makes it hard to click the button easily
@cee-chen cee-chen marked this pull request as ready for review October 4, 2023 03:06
@cee-chen cee-chen requested a review from a team as a code owner October 4, 2023 03:06
@cee-chen
Copy link
Member Author

cee-chen commented Oct 4, 2023

@ThomThomson FYI, this is coming soon!

As a heads up, I'm leaning towards making the new animation opt-in for now (i.e., you'll have to manually enable it with <EuiFlyout type="push" pushAnimation={true} />. My reasoning behind that is wanting to disrupt existing usages as little as possible. For example, the current EuiCollapsibleNav component is a push flyout under the hood and would need have the prop manually turned off if it was the default (because the nav is present on page load and shouldn't be animated in). There may be other similar usages of push flyouts like that in Kibana.

However, I'm definitely open to hearing thoughts from others if they feel otherwise / if this should be baked in at some point as a default!

@kibanamachine
Copy link

Preview staging links for this PR:

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

@1Copenut 1Copenut self-requested a review October 4, 2023 13:51
@ThomThomson
Copy link
Contributor

This looks great! Opt-in is fine as I can't imagine most usages wanting this behaviour.

1Copenut
1Copenut approved these changes Oct 4, 2023
View reviewed changes
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!

@cee-chen
Copy link
Member Author

cee-chen commented Oct 4, 2023

Thank you all, and for the terrific suggestion @ThomThomson!

@cee-chen cee-chen merged commit 83e3d4e into elastic:main Oct 4, 2023
8 checks passed
@cee-chen cee-chen deleted the flyout-push-animation branch October 4, 2023 16:22
@cee-chen cee-chen mentioned this pull request Oct 9, 2023
Merged
1Copenut pushed a commit to elastic/kibana that referenced this pull request Oct 11, 2023
@cee-chen
Upgrade EUI to v89.0.0 (#168396)
7708814 
`v88.5.4`⏩`v89.0.0`

---

## [`89.0.0`](https://github.com/elastic/eui/tree/v89.0.0)

- Added new `pushAnimation` prop to push `EuiFlyout`s, which enables a
slide in animation ([#7239](elastic/eui#7239))
- Updated `EuiComboBox` to use `EuiInputPopover` under the hood
([#7246](elastic/eui#7246))
- Added `inputPopoverProps` to `EuiComboBox`, which allows customizing
the underlying popover
([#7246](elastic/eui#7246))
- Added a new beta `EuiTextBlockTruncate` component for multi-line
truncation ([#7250](elastic/eui#7250))
- Updated `EuiBasicTable` and `EuiInMemoryTable` to support multi-line
truncation. This can be set via `truncateText.lines` in the `columns`
prop. ([#7254](elastic/eui#7254))

**Bug fixes**

- Fixed `EuiFlexGroup` and `EuiFlexGrid`'s `m` gutter size
([#7251](elastic/eui#7251))
- Fixed focus trap rerender issues in `EuiFlyout` with memoization
([#7259](elastic/eui#7259))
- Fixed a visual bug with `EuiContextMenu`'s animation between panels
([#7268](elastic/eui#7268))

**Breaking changes**

- EUI's global body font-size now respects the `font.defaultUnits`
token. This means that the global font size will use the `rem` unit by
default, instead of `px`.
([#7182](elastic/eui#7182))
- Removed exported `accessibleClickKeys`, `comboBoxKeys`, and
`cascadingMenuKeys` services. Use the generic `keys` service instead
([#7256](elastic/eui#7256))
- Removed `EuiColorStops` due to low usage
([#7262](elastic/eui#7262))
- Removed `EuiSuggest`. We recommend using `EuiSelectable` or
`EuiComboBox` instead
([#7263](elastic/eui#7263))
- Removed `euiHeaderAffordForFixed` Sass mixin, and `$euiHeaderHeight`
and `$euiHeaderHeightCompensation` Sass variables. Use the CSS variable
`--var(euiFixedHeadersOffset, 0)` instead.
([#7264](elastic/eui#7264))

**Accessibility**

- When using `rem` or `em` font units, EUI now respects, instead of
ignoring, browser default font sizes set by end users.
([#7182](elastic/eui#7182))
dej611 pushed a commit to dej611/kibana that referenced this pull request Oct 17, 2023
@cee-chen @dej611
Upgrade EUI to v89.0.0 (elastic#168396)
887f985 
`v88.5.4`⏩`v89.0.0`

---

## [`89.0.0`](https://github.com/elastic/eui/tree/v89.0.0)

- Added new `pushAnimation` prop to push `EuiFlyout`s, which enables a
slide in animation ([elastic#7239](elastic/eui#7239))
- Updated `EuiComboBox` to use `EuiInputPopover` under the hood
([elastic#7246](elastic/eui#7246))
- Added `inputPopoverProps` to `EuiComboBox`, which allows customizing
the underlying popover
([elastic#7246](elastic/eui#7246))
- Added a new beta `EuiTextBlockTruncate` component for multi-line
truncation ([elastic#7250](elastic/eui#7250))
- Updated `EuiBasicTable` and `EuiInMemoryTable` to support multi-line
truncation. This can be set via `truncateText.lines` in the `columns`
prop. ([elastic#7254](elastic/eui#7254))

**Bug fixes**

- Fixed `EuiFlexGroup` and `EuiFlexGrid`'s `m` gutter size
([elastic#7251](elastic/eui#7251))
- Fixed focus trap rerender issues in `EuiFlyout` with memoization
([elastic#7259](elastic/eui#7259))
- Fixed a visual bug with `EuiContextMenu`'s animation between panels
([elastic#7268](elastic/eui#7268))

**Breaking changes**

- EUI's global body font-size now respects the `font.defaultUnits`
token. This means that the global font size will use the `rem` unit by
default, instead of `px`.
([elastic#7182](elastic/eui#7182))
- Removed exported `accessibleClickKeys`, `comboBoxKeys`, and
`cascadingMenuKeys` services. Use the generic `keys` service instead
([elastic#7256](elastic/eui#7256))
- Removed `EuiColorStops` due to low usage
([elastic#7262](elastic/eui#7262))
- Removed `EuiSuggest`. We recommend using `EuiSelectable` or
`EuiComboBox` instead
([elastic#7263](elastic/eui#7263))
- Removed `euiHeaderAffordForFixed` Sass mixin, and `$euiHeaderHeight`
and `$euiHeaderHeightCompensation` Sass variables. Use the CSS variable
`--var(euiFixedHeadersOffset, 0)` instead.
([elastic#7264](elastic/eui#7264))

**Accessibility**

- When using `rem` or `em` font units, EUI now respects, instead of
ignoring, browser default font sizes set by end users.
([elastic#7182](elastic/eui#7182))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@1Copenut 1Copenut 1Copenut approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[EuiFlyout] Animate push flyouts
5 participants
@cee-chen @kibanamachine @elasticmachine @ThomThomson @1Copenut