-
Notifications
You must be signed in to change notification settings - Fork 861
[flyout system/fix] Breaking: Prevent default configuration flyouts from interacting with the Flyout System #9124
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
Conversation
| const { session, as, onClose, onActive, ...rest } = | ||
| usePropsWithComponentDefaults('EuiFlyout', props); | ||
| const hasActiveSession = useHasActiveSession(); | ||
| const hasActiveSession = useRef(useHasActiveSession()); |
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.
neat!
|
I love how clean the solution is! The changes work great 👏🏻 I found that opening a child flyout when a non-managed flyout is also open can look strange because of the Screen.Recording.2025-10-17.at.16.23.11.mov |
where inherit is the default value
| * - `never`: Opts out of session management and always functions as a standard flyout. | ||
| * @default 'inherit' | ||
| */ | ||
| session?: 'start' | 'inherit' | 'never'; |
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.
nit: now that we're using string literals, could we have these defined in manager/const.ts and manager/types.ts for consistency with other values like LEVEL_MAIN?
|
Everything seems to be working just fine with your latest changes @tsullivan! Code changes look good as well |
💚 Build SucceededHistory
|
💚 Build Succeeded
History
|
This PR adds breaking API change to the flyout system (feature branch).
Basically, this is the same logic as before, but we're changing the type for the session flag to prevent confusion.
As a fix that comes along with this change, a flyout will no longer get "bumped" into automatically/accidentally becoming a child if it is open with the default configuration when a new main flyout gets opened. This was a bug and was causing confusion all around.