eui
eui copied to clipboard
Add EuiFlyoutChild for Grouped Flyouts
EuiFlyoutChild: A Secondary "Child" Flyout with Dynamic Responsive Stacking
This PR introduces the EuiFlyoutChild component, a new addition to the EUI flyout system, designed to allow for side-by-side (grouped) flyout panels.
https://github.com/user-attachments/assets/1f311949-bab1-41d1-a5ad-1bc7ace390da
Feature Overview
The EuiFlyoutChild component enables a secondary flyout panel to be opened adjacent to a primary EuiFlyout.
Key Behaviors:
- Side-by-Side Layout: On wider viewports, the child flyout appears to the left edge of its parent
EuiFlyout. - Stacked Layout: On narrower viewports, the child flyout stacks on top of its parent, occupying the full width of the parent.
- Dynamic Stacking Breakpoint: The transition point between side-by-side and stacked layouts is dynamically calculated based on the combined widths of the parent and child flyouts.
- The stacking breakpoint is the sum of the parent flyout's effective width and the child flyout's effective width.
- For example:
- If Parent is size 'm' and Child is size 's', the breakpoint is
theme.breakpoint.m + theme.breakpoint.s. - If Parent is size 's' and Child is size 's', the breakpoint is
theme.breakpoint.s + theme.breakpoint.s.
- If Parent is size 'm' and Child is size 's', the breakpoint is
- Size Restrictions:
EuiFlyoutChildsupports sizes 's' (small) and 'm' (medium).- If the parent
EuiFlyoutis size 'm', theEuiFlyoutChildcan only be size 's'. - If the parent
EuiFlyoutis size 's', theEuiFlyoutChildcan be size 's' or 'm'.
Implementation Details
- Component Structure (
eui_flyout_child.tsx):- Manages its own responsive logic by listening to window resize events to get the current viewport width (
window.innerWidth). - Calculates the
stackingBreakpointValue(numeric pixel value) by summing the theme-defined pixel widths corresponding to the parent's and child's sizes. - Determines
isAboveStackingBreakpoint(boolean) by comparingwindow.innerWidthtostackingBreakpointValue. This state variable controls whether to apply side-by-side or stacked positioning styles.
- Manages its own responsive logic by listening to window resize events to get the current viewport width (
- Context Usage:
- Uses
EuiFlyoutContextto get the size of the parentEuiFlyout.
- Uses
- Validation:
- Ensures
EuiFlyoutChildis used within anEuiFlyout. - Requires an
EuiFlyoutBodyas a child. - Enforces the size restriction (parent 'm' cannot have child 'm').
- Ensures
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
@defaultif 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)
- 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)
I'm in the process of reviewing this work
@tkajtoch I've spent a little bit of time trying to make animation effects work for the child flyout. The requirements I was attempting are:
- When animating, the child flyout should slide in and out of view from "under" the parent flyout. However, once the child is finished animating it will need a z-index than the parent flyout, because of our responsive "stacked layout" rules.
- We want the child flyout to animate when opening and closing, but not when it gets a layout change due to responsive rules (maybe, depending on feedback).
Unfortunately, this is a bit beyond my ability. As this PR should be for an MVP, I hope we can hold off on the animation requirements until a future PR, when someone with more CSS expertise can attempt it.
cc @ryankeairns @JasonStoltz
We can definitely help animating child flyout transitions @tsullivan! Is having them animated a requirement, though?
We can definitely help animating child flyout transitions @tsullivan! Is having them animated a requirement, though?
@tkajtoch I'm not hearing that it's a "hard" requirement. I'm hearing that designers want to get their hands on the MVP and evaluate / give feedback whether animation is needed, and to what extent.
From @alexmarhaba:
small nitpick: the animation when the flyout disappears on resize feels a bit strange, as if it's being pushed out of the screen. Is there a a way to make it simply disappear? (no sliding animation)
when opening a child flyout, it's nice to have it slide out of the parent flyout to highlight the "parent / child" relationship, but I'd defer to @ryankeairns
From @ryankeairns:
+1 to animating the child. ... that can be a bit of a rabbit hole to get 'just right' so a rough animation will do initially. Then, we can refine it once we have something to click around on / test.
From @petrklapka:
I like the animation on resize, it "informs" me what is happening. I'll only +0.5 an animation on the child opening though. There's a nice "crispness" to it opening instantly, a perception of performance if you will, that might get lost if it is animated. I'd recommend a "fast" animation of it opening. You know that scene in Star Wars when the door closes crazy fast? Like that!
From @ryankeairns:
Agreed. Almost unnoticeable; prioritize 'snappiness'
Thanks Tim!
Regarding animation
- I'm curious if we tried/should apply the same flyout animation to the child as already exists for the parent?
- Personally, I am ok with coming back to animation of the child if that's the consensus from the group
One visual 'bug' In my initial review, I noticed there is a gradient that appears over the child footer:
The close button is layering behind the child flyout panel on the small breakpoint; probably need to crank up the z-index
Ok, so we got this out of the gate by simplifying the size options... now, some follow up questions:
- ~As it stands, is the parent allowed to be any size / is the only the child technically limited to
s, m?~ Update: tested and now see that the presence of a child enforces the parent size to be eithersorm - We know, in the end, the child will need to be rather large to contain a 'tool' like the Analyzer graph... how do you suppose we approach or enhance for this option? For example, we may do something like add a
filloption for the child size (e.g.s, m, fill) - Also 'in the end', we will need to support custom/specific sizes for the main flyout... which means whatever the sizes provided, we will need to handle and adapt
So that we are all on the same page, should we handle these requirements in a subsequent PR(s)? And, if so, perhaps it is best to keep this new functionality undocumented until these (and potentially other) enhancements are made?
https://github.com/elastic/eui/pull/8771#issuecomment-3002033086
Regarding animation I'm curious if we tried/should apply the same flyout animation to the child as already exists for the parent?
@ryankeairns: Yes, I've spent a few hours on this. There are unique aspects of the child flyout that make it a bit more challenging:
-
When animating open/closed, the child flyout should appear to slide "under" the parent flyout. However, once the animation finishes, the child flyout should conceptually "stacked above" the parent flyout, in order to implement the responsive rules of using a "stacked layout" that puts the child flyout on top of the parent when the screen size is too small to show both.
-
The child and parent flyouts have different "stacking contexts" due to their positioning styles, which makes the z-index logic more complicated. This is the main thing I struggle with.
-
We haven't settled on the desired behavior of whether the child will animate when responsively switching the layout between "stacked" and "side-by-side." This is more of a discussion item that is still pending.
I've captured the ongoing discussion here: https://github.com/elastic/eui/pull/8771#issuecomment-3001458256
One visual 'bug' In my initial review, I noticed there is a gradient that appears over the child footer:
Wow, great catch! Fixed in f02a3b93c
https://github.com/elastic/eui/pull/8771#issuecomment-3002049342
The close button is layering behind the child flyout panel on the small breakpoint; probably need to crank up the z-index
@ryankeairns: I'm not able to reproduce this, or maybe I don't understand the issue. Let's sync up about it.
https://github.com/elastic/eui/pull/8771#issuecomment-3002058735
We know, in the end, the child will need to be rather large to contain a 'tool' like the Analyzer graph... how do you suppose we approach or enhance for this option? For example, we may do something like add a fill option for the child size (e.g. s, m, fill)
@ryankeairns: Fill is doable, which I've verified from accidentally creating that effect in mid-development :D. I think this
is mostly an issue of using a clip-path style that makes this happen.
Also 'in the end', we will need to support custom/specific sizes for the main flyout... which means whatever the sizes provided, we will need to handle and adapt
Interesting! I missed this from docs that came out from planning this feature. Is this only the case for the main flyout?
So that we are all on the same page, should we handle these requirements in a subsequent PR(s)? And, if so, perhaps it is best to keep this new functionality undocumented until these (and potentially other) enhancements are made?
To me, this both feel like follow-up items, which seem within my technical range, and I'm happy to continue working in this repo to achieve whatever tasks we need. I agree the new functionality could be undocumented (not in the EUI docs site) until suitable progress has been made.
Thanks for the replies.
One visual 'bug' In my initial review, I noticed there is a gradient that appears over the child footer:
Wow, great catch! Fixed in https://github.com/elastic/eui/commit/f02a3b93cc1a2c2e961d1da053b7aa364f44bf72
Confirmed, lgtm!
The close button is layering behind the child flyout panel on the small breakpoint; probably need to crank up the z-index
@ryankeairns: I'm not able to reproduce this, or maybe I don't understand the issue. Let's sync up about it.
Dug a little deeper. This seems to have to do with the max-inline-size for the child flyout.
On small viewports (i.e. under small breakpoint), the max inline size should be set to 90vw, same as we do with other flyouts. Right now, this is not the case so the child is a bit too wide resulting in the right edge going out of view (including the close button)
Before: without 90vw max
After: with 90vw max
Regarding the follow-up work on supporting additional sizes:
- Adding support for a larger child flyout is a hard requirement given the size of tools (e.g. Analyzer; see image below). My proposal to start with supporting only
s, mwas to simplify our starting point... which you seem to have nailed :) Without a larger (e.g. custom child size) orfilloption, Security won't be able to use the child flyout. - Supporting custom parent flyout sizes is debatable, and I could see us moving ahead without it initially given the support for
s, m. It appears, to me, that this would cover the use cases - for the parent slot - that we have seen to date. I could be wrong.
Note: they also support resizing which is another future enhancement consideration.
To clarify, I am not suggesting we hold this PR up. It tackles a healthy chunk of the task at hand, and it may serve other, less-complex use cases for parent-child flyouts, as-is.
My intent with this feedback is to clarify that there are additional requirement(s) needed before this can be adopted for Security use cases. Nothing surprising, just sharing context.
cc:/ @alexmarhaba
Dug a little deeper. This seems to have to do with the max-inline-size for the child flyout.
@ryankeairns thanks for the detailed explanation! This has been fixed in f12ba501c
Tested locally, the fix lgtm. Thanks!
:green_heart: Build Succeeded
- Buildkite Build
- Commit: f660f6b33b067f29fd2b84d07397b8797baa6160
- Documentation website
History
- :green_heart: Build #638 succeeded 180089c67ae89240bf3f81d825a4ffe34d8ca477
- :green_heart: Build #627 succeeded f12ba501c42e16ec074be1bfc719b6cc039ab93d
- :green_heart: Build #626 succeeded 89d79d491e946099429effd096bb4c1817d35355
- :green_heart: Build #625 succeeded ec1a72c02e3528e19b8a63682e17198a6561f19e
- :green_heart: Build #624 succeeded f02a3b93cc1a2c2e961d1da053b7aa364f44bf72
- :green_heart: Build #622 succeeded 42f8bf7dda650889c05aa95e7844cbce338dff19
cc @tsullivan
:green_heart: Build Succeeded
- Buildkite Build
- Commit: f660f6b33b067f29fd2b84d07397b8797baa6160
History
- :green_heart: Build #4277 succeeded 180089c67ae89240bf3f81d825a4ffe34d8ca477
- :green_heart: Build #4269 succeeded f12ba501c42e16ec074be1bfc719b6cc039ab93d
- :green_heart: Build #4268 succeeded 89d79d491e946099429effd096bb4c1817d35355
- :green_heart: Build #4267 succeeded ec1a72c02e3528e19b8a63682e17198a6561f19e
- :green_heart: Build #4266 succeeded f02a3b93cc1a2c2e961d1da053b7aa364f44bf72
- :green_heart: Build #4264 succeeded 42f8bf7dda650889c05aa95e7844cbce338dff19
cc @tsullivan