[Flyout System] Make session logic truly opt-in

[tsullivan]

Summary

Makes the Flyout System truly opt-in by changing the session value default from inherit to never.

Module	Change
EuiFlyout	Changed: session prop The default has been changed from inherit to never

Closes #9158.

Why are we making this change?

The default of inherit was problematic. It meant that a flyout with no session value added would automatically become a child if a session flyout is active. This would be a problem in Kibana, where all flyouts currently have no session value. Work would be needed to address the problem by adding session="start" on them to avoid possible bugs, which means the default logic planned would amount to a breaking change.

Impact to users

BREAKING CHANGE - the default of session has been changed to never.

The default value of EuiFlyout session prop is changing from inherit to never. This makes the flyout session management system truly opt-in: a flyout that has not been modified and has no session prop set will not become a child flyout of an active main flyout and will continue to act as a regular flyout. Flyouts will only become a child of when they have the session value set to inherit.

• Not changed: if a flyout has an undefined value for session and is nested within a main flyout, it behaves as a child flyout.
• Not changed: if a flyout has session="inherit" and there is no main flyout, it will behave as a regular flyout.

To continue the previous behavior of a flyout rendered as a child, you can explicitly set the session prop to inherit, or you can nest the flyout into a main flyout.

This breaking change only effects the code in the feature branch. There are no EUI releases that have the previous behavior.

QA

Remove or strikethrough items that do not apply to your PR.

General checklist

• Browser QA
  • [ ] Checked in both light and dark modes
  • [ ] Checked in both MacOS and Windows high contrast modes
    • (emulate forced colors if you do not have access to a Windows machine.)
  • [ ] Checked in mobile
  • [ ] Checked in Chrome, Safari, Edge, and Firefox
  • [ ] Checked for accessibility including keyboard-only and screenreader modes
• Docs site QA
  • Added documentation
  • Props have proper autodocs (using @default if default values are missing) and playground toggles
  • Checked Code Sandbox works for any docs examples
• Code quality checklist
  • Added or updated jest and cypress tests
  • [ ] Updated visual regression tests
• Release checklist
  • A changelog entry exists and is marked appropriately
  • If applicable, added the breaking change issue label (and filled out the breaking change checklist)
  • [ ] If the changes unblock an issue in a different repo, smoke tested carefully (see Testing EUI features in Kibana ahead of time)
• Designer checklist
  • [ ] If applicable, file an issue to update EUI's Figma library with any corresponding UI changes. (This is an internal repo, if you are external to Elastic, ask a maintainer to submit this request)

[tkajtoch]

I reviewed and tested this PR and found that the current behavior of the session prop for child flyouts might be confusing. We agreed with Tim to update the logic to default to session="inherit" when EuiFlyout is rendered inside of a main flyout.

[tsullivan]

As it currently is in this PR, a flyout could be structured within a managed flyout but not be a child - it would even then need session="inherit". This is not very intuitive. The routing logic we want is:

main flyout:
  - has `session="start"`
child flyout:
  - has `session="inherit"` OR
  - is structured inside of a "main"
normal flyout:
  - has `session="never"` OR
  - has no explicit session prop and is not nested inside of a "main"

cc @tkajtoch

[tkajtoch]

I'm gonna test it shortly. Thank you @tsullivan!

[tsullivan]

Blocked on #9178

[tsullivan]

The modifications mentioned here have been implemented.

@tkajtoch @weronikaolejniczak this is ready for another look

[weronikaolejniczak]

@tsullivan I'll take a look at this today, one doubt I have is - we should merge it into the feature branch, not main, right?

[tsullivan]

I'll take a look at this today, one doubt I have is - we should merge it into the feature branch, not main, right?

@weronikaolejniczak thanks! You are correct. This PR is targeted to the feat/flyout-system feature branch.

[tsullivan]

I've removed the breaking change label on this PR because the change only effects the code in the feat/flyout-system feature branch. There are no EUI releases that have the previous behavior, so it is not a true breaking change.

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: d7c4b6e
• Documentation website
• Storybook

History

• 💚 Build #1922 succeeded 2b2ea52
• 💚 Build #1879 succeeded cf826f9
• 💚 Build #1821 succeeded db2c4ab
• 💚 Build #1767 succeeded 44c5fdb

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: d7c4b6e

History

• 💚 Build #5209 succeeded 2b2ea52
• 💚 Build #5175 succeeded cf826f9
• 💚 Build #5134 succeeded db2c4ab
• 💚 Build #5089 succeeded 44c5fdb

[tkajtoch]

Thank you for updating the session inheritance logic! LGTM