-
Notifications
You must be signed in to change notification settings - Fork 861
[Flyout System] Support resizing flyouts #8999
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
[Flyout System] Support resizing flyouts #8999
Conversation
…lyout resizable when `resizable=true`
…ble` to prevent extra styles being added
| `; | ||
|
|
||
| exports[`EuiFlyout props size fill is rendered 1`] = ` | ||
| [ |
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 an updated snapshot that was failing on feat/flyout-system due to last-minute changes on the last PR merged to the feature branch. The changes match other "is rendered" variants
| @@ -0,0 +1,180 @@ | |||
| /* | |||
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 primarily a restructure of the event handlers previously defined in flyout_resizable.tsx. I did not change the logic here
💚 Build SucceededHistory
cc @tkajtoch |
💚 Build Succeeded
History
cc @tkajtoch |
| size?: EuiFlyoutSize | CSSProperties['width']; | ||
| /** | ||
| * Sets the minimum width of the panel. | ||
| * Especially useful when set with `resizable = true`. |
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.
It will be good to have this for fill mode calculations as well. In the side-by-side layout when a flyout has fill mode, we need to refer to this in our calculation for the breakpoint when the layout is switched to stacked. Right now the code is using non-dynamic s value as the minimum width.
|
This is great! I spot a few issues, which you probably are aware of
IMO, we should use a calculation for the width to make sure the flyout never becomes wider than 90% of the viewport:
|
|
I think this looks good as this is targeted to a feature branch and it achieves 90% of what is needed. The pending feedback I gave can be addressed in a later PR |
tsullivan
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.
LGTM
Thanks for fixing the broken snapshot and consolidating the code!
acstll
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.
Small things I found in a QA round:
Confirm
EuiFlyoutResizableworks the same as on prod
- type
pushis not working as expected in the docs - the story's prop table is not picking up the comments (it does in prod)
Confirm the
mainFlyoutResizableandchildFlyoutResizabletoggles work in the managed flyout story
- child flyout animates over or above ("z-index") the main flyout when opening
Everything else working as expected. No comments on the code side 👍
I can see it's working but crashes when toggling between push and overlay in the docs. That applies to both this PR and Do you see any other issues? |
perfect, thanks for the clarification!
I agree
nope
|
acstll
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.
🟢 LGTM
@acstll It seems that the EuiFlyoutResizable story already has some props type overrides in place to possibly fix an underlying issue related to how deeply nested the type is - there's multiple Props are rendered correctly in the docs; I think it's okay to find a way to fix this issue in this specific story later on. |
## Summary This EUI upgrade brings the new Flyout System. While all of the changes we made are opt-in, we did have to update the DOM nesting in `EuiFlyout` and `EuiFlyoutResizable`. This change includes making overlay masks a sibling of flyouts, rather than wrapping the flyouts as children, which required internal changes to the mask's z-index value. Following @tsullivan request, please run through your UIs and make sure the flyouts render as expected, especially in the areas where you override EuiFlyout styles. ## Dependency updates - `@elastic/eui`: `v110.0.0` ⏩ `v111.0.0` - `@elastic/eui-theme-borealis`: `v5.1.0` ⏩ `v5.2.0` --- ## Changes - Removed `z-index` overrides from various places across Kibana. The updated EuiFlyout logic calculates `z-index` values dynamically based on the order of opening flyouts, making the manual overrides unnecessary. - Updated types of refs passed to `EuiFlyout` and `EuiFlyoutResizable` ## Package updates ### `@elastic/eui` v111.0.0 - Added an opt-in EuiFlyout session management for creating flyout compositions and journeys effortlessly. Session management handles side-by-side flyout rendering based on parent-child grouping, simple flyout transitions with history, state sharing, and more. ([#9202](elastic/eui#9202)) - EuiFlyout session management is an optional feature that can be enabled by adding `session="start"` to EuiFlyout. Check out the [documentation](https://eui.elastic.co/docs/components/containers/flyout/session-management) to learn more. - Added a new `hasChildBackground` boolean prop (defaults to false) to `EuiFlyout` ([#9056](elastic/eui#9056)) - Updated `EuiFlyout` with new `onActive` callback and enable stack managed history controls. ([#9003](elastic/eui#9003)) - Updated `EuiFlyoutMenu` with new prop `historyItems` and refactored props for back button. ([#9003](elastic/eui#9003)) - Added a new optional `resizable` (boolean) prop to `EuiFlyout`. Resizability can now be controlled dynamically without the need to use `EuiFlyoutResizable`. ([#8999](elastic/eui#8999)) - Flyout system menu bar: require tile, support custom actions ([#8897](elastic/eui#8897)) - Added a new `EuiFlyoutMenu` component that provides a standardized top menu bar for flyouts. ([#8851](elastic/eui#8851)) **Breaking changes** - Changed the way EuiFlyout renders overlay masks to decouple the overlay mask from the flyout itself. Now, the overlay mask is a separate portalled element. ([#9202](elastic/eui#9202)) - This change does not modify the functionality or behavior of flyout overlays but might affect some custom usages when your application relies on the specific element nesting within EuiFlyout. ### `@elastic/eui-theme-borealis` v5.2.0 - Updated parameters used for `euiAnimSlightResistance` for a smoother animation ([#9202](elastic/eui#9202)) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>


Summary
Resolves #8875.
This PR is targeting the
feat/flyout-systemfeature branch.This PR adds support for making flyouts resizable by setting
resizable=true, allowing managed flyouts to be resized when resizing is explicitly enabled.The solution builds upon the original
EuiFlyoutResizableimplementation. The resizing logic is moved to an internaluseEuiFlyoutResizablehook and integrated directly intoEuiFlyoutComponent(EuiFlyoutonmain). This allows for dynamic control of whether a flyout is resizable while keeping the API backwards compatible. With this change,EuiFlyoutResizablebecomes a small wrapper that setsresizable=trueonEuiFlyout.Tests remain unchanged since the functionality is generally the same. We may consider restructuring them before merging the feature branch into main and releasing it.
Why are we making this change?
Because the Flyout System project needs to support resizable main and child flyouts as per requirements. See #8875 and the Flyout System meta issue for more information.
Impact to users
No negative impact to users expected.
QA
EuiFlyoutResizableworks the same as on prodmainFlyoutResizableandchildFlyoutResizabletoggles work in the managed flyout storyGeneral checklist
@defaultif default values are missing) and playground toggles