-
Notifications
You must be signed in to change notification settings - Fork 861
feat: Add a new boolean hasChildBackground prop to the Flyout to use for child flyouts only
#9056
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
feat: Add a new boolean hasChildBackground prop to the Flyout to use for child flyouts only
#9056
Conversation
| id="flyout-manager-playground-child" | ||
| size={childSize} | ||
| backgroundStyle={childBackgroundStyle} | ||
| childBgShaded={childBackgroundStyle === 'shaded'} |
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.
Screen.Recording.2025-10-08.at.16.21.36.mov
backgroundStyle prop to the Flyout (default/shaded)childBgShaded prop to the Flyout to use for child flyouts only
|
This PR can also clean up the previous implementation, which relied on consumers directly rendering |
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.
Left a suggestion
Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com>
| * When the flyout is used as a child in a managed flyout session, setting `true` gives the shaded background style. | ||
| * @default false | ||
| */ | ||
| childBackgroundShaded?: boolean; |
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.
@paulinashakirova we'll also need to update all the places that used the old childBgShaded to use this name
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.
To align more with existing naming conventions for booleans, this should rather be something like: hasChildBackground.
Imho, we should drop the "shaded" part. The color system has a "shaded" background color (which is not used here), so the naming might be confusing.
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.
Thank you for your suggestion!
Where should I describe what type of the background "child" background is?
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.
Where should I describe what type of the background "child" background is?
@paulinashakirova Sorry, I don't quite understand - What do you mean exactly?
The type boolean stays. In terms of naming, we don't need to have the color name "shaded" in the prop, that would make the API volatile if we want to change the color we'd have to change the prop as well.
The JSDoc describes the appliance already for devs 🙂
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.
Gotcha!
Yes, I misunderstood you - of course JSDoc will mention shaded. And if we want to change the color we will update the comment.
childBgShaded prop to the Flyout to use for child flyouts onlychildBackgroundShaded prop to the Flyout to use for child flyouts only
| ### `src/components/flyout/manager/flyout_child.tsx` | ||
| Renders child flyouts within a session: | ||
| - **Positioning**: Automatically positions relative to main flyout width | ||
| - **Styling**: Supports `backgroundStyle` prop for default/shaded backgrounds |
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.
In this file, let's change line 97-98 to:
### `src/components/flyout/manager/flyout.styles.ts`
Managed flyout styling for the flyout management system.
97 is ok, 98 should have the mention background styles removed
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.
It's nearly there, just a few small issues with comments that need revising :)
💚 Build SucceededHistory
|
💚 Build Succeeded
History
|
tkajtoch
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.
I love the cleanup you did! I believe there's one remaining unresolved comment that I'd like to see done before we merge this PR: https://github.com/elastic/eui/pull/9056/files/6e1ea9904d65de24391364f2d4d8b24db96d66bc#r2422128137
Other than that I have no objections
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 the hard work!!
… use for child flyouts only (#9056) Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com>
childBackgroundShaded prop to the Flyout to use for child flyouts onlyhasChildBackground prop to the Flyout to use for child flyouts only
## 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
This PR resolves [Flyout System] Add “pass through props” for EuiChildFlyout on EuiFlyout if session=true issue.
Why are we making this change?
In the implementation of the new Developer API for flyout system (https://github.com/elastic/kibana-team/issues/1874), a capability became broken, which is to provide the
backgroundStyleprop toEuiFlyoutChild. This happened becauseEuiFlyoutChildis no longer intended to be consumed directly.For this reason, we are introducing a new
hasChildBackgroundprop that can set shaded background on a child flyout ifsession=true.Screenshots
Impact to users
This is not a breaking change. This new prop has a default value that keeps current UI untouched.
General checklist
Browser QA
Docs site QA
@defaultif default values are missing) and playground togglesCode quality checklist
Release checklist