[flyouts] Developer API + sessions

[clintandrewhall]

Summary

In this iteration of flyout management, a single session prop is used to control whether a flyout is rendered as a new session, or as a standard flyout.

Demo

There is a Storybook demonstration of flyout management using a contrived "shopping cart" example.

Flyouts

Shopping cart flyout

This flyout always starts a new session, session={true}.

Review order flyout

This flyout always starts a new session, session={true}.
It is rendered on it's own from the page root, but also from within the Shopping Cart flyout. This demonstrates how a new session can be started from anywhere, but how it's session interacts with others.

Item details flyout

This flyout is a regular flyout.
It is rendered on its own, but also from within the Shopping Cart flyout.
If rendered on its own, and no session is active, it is rendered as a regular flyout.
If rendered on its own, and a session is active, it is rendered as a new main flyout as a new session.
If rendered from within the Shopping Cart flyout, it will be rendered as a child flyout.

Parent + child management

Standard flyout becomes main if a session is active

Parent + child stacking behavior

Details

The determination of whether a managed flyout is rendered as a main or child is based:

1. on the presence of an active session, and
2. if the flyout is rendered from within a managed flyout.

This approach means the relationship between the main and child flyout, as well as the history of which main flyouts have been opened, are implicitly derived from the React structure.

So from a DX perspective, no one need wonder if they should create a MainFlyout or a ChildFlyout, or check what may already be open... the way its structured and the session prop handle it all.

Notes

• The EuiFlyout component was moved to flyout.component.tsx.
• Logic for determining which flyout to render was thus placed in flyout.tsx.

[elasticmachine]

💚 Build Succeeded

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

History

• 💚 Build #981 succeeded 921c51d
• 💔 Build #975 failed bfe70f4

[weronikaolejniczak]

@clintandrewhall I'll stop at this because my head is exploding 🤯 You've all done amazing job here! 🎉 💚 Flyouts are working smoothly, the API is simple and clear, and I love the code quality - no magic numbers, clear separation of concerns, no code smells.

There are a bunch of comments I left, I made sure to say whether they are blocking or non-blocking in my mind (the majority are non-blocking) and leave GH code suggestions to make the fixes quick and easy. Feel free to omit / postpone as many of my suggestions as you'd like. And if you want, we can discuss some of them in more depth.

[clintandrewhall]

@ThomThomson @tsullivan I did, indeed, remove "automatic" virtualization for this approach. I just felt the DX ease justified the trade-off.

My mitigation would be a container component for intentional virtualization, rather than a pre-optimization:

<EuiFlyout session={true}>
  <EuiFlyoutBody virtualized={true}>
    // Expensive stuff.
  </EuiFlyoutBody>
  {renderChildFlyout ? <EuiFlyout /> : null}
</EuiFlyout>

Or perhaps <EuiFlyoutVirtualized>, or similar... but basically allow us to identify cases where the virtualization was necessary. Does that make sense? Could use a sanity check on the idea... but I'd rather solve a problem that arises, rather than plan to pre-optimize.

[tsullivan]

My mitigation would be a container component for intentional virtualization, rather than a pre-optimization
...

Or perhaps , or similar... but basically allow us to identify cases where the virtualization was necessary.

@clintandrewhall @ThomThomson it sounds like this is an optimization that doesn't necessarily need to be made in this PR. The idea wouldn't be a major pivot from the direction we're starting here, correct?

but I'd rather solve a problem that arises, rather than plan to pre-optimize.

Totally agree

[tsullivan]

ping @tkajtoch

[tkajtoch]

I'm addressing this as part of the animations improvements work. This whole EuiOverlayMask is a bit of a mess and needs reworking.

Let's leave it as is since this goes into a feature branch, and I'll fix it in a follow-up PR for clarity if you have no objections.

[tsullivan]

LGTM. All tasks that are open are going to be handleed in separate PRs in this feature branch. Along with other things, they're being tracked in this document: https://docs.google.com/document/d/1OXx2H997k6zfz_w4jc-CJ1qyJxesi47CW5bEDsZ7OQk/edit?tab=t.0

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: 3071ce5
• Documentation website
• Storybook

History

• 💚 Build #1125 succeeded 3071ce5
• 💚 Build #1124 succeeded a530e45
• 💚 Build #1119 succeeded e31a1e8
• 💚 Build #1118 succeeded e31a1e8
• 💚 Build #1112 succeeded f5feb52
• 💚 Build #1111 succeeded f5feb52

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: 3071ce5

History

• 💚 Build #4625 succeeded a530e45
• 💔 Build #4622 failed e31a1e8
• 💚 Build #4616 succeeded f5feb52
• 💔 Build #4615 failed 34670d5
• 💚 Build #4612 succeeded 330e186
• 💚 Build #4600 succeeded ca1bafd

[weronikaolejniczak]

nit:

I don't mind adding these but I'd put them in a separate "section", right now they seem under "Yarn". Maybe even move to "IDE-specific files".

[weronikaolejniczak]

Recent changes look great! I like that it was extensively covered with tests. I skimmed them and they are alright 😄

🟢 🚀

[tkajtoch]

I'm happy merging this as is since it goes into a feature branch. I reviewed the code and functionality and have no complaints that would be blocking merging of this PR.