[EuiFlyoutResizable] Add resizable flyout

[cee-chen]

Summary

closes #287

Areas I'm looking for feedback on:

• API - does adding a new <EuiFlyoutResizable /> component (as opposed to, e.g. <EuiFlyout resizable={true} />) make sense? I leaned this way because it felt the cleanest in terms of implementation (as opposed to adding Yet More Code to the existing logic-heavy flyout file), but definitely would like to hear feedback on this.

• UI/UX - do we need more visual indicators or UI hints that the edge is resizable? Or does the existing appearance feel sufficient?

• Documentation - especially with our new focus on patterns, I would really like feedback on the copy I wrote. I cribbed text from the above issue on why using resizable flyouts might be a good idea, but I think we should also consider adding some copy on when not to use resizable flyouts.

QA

• https://eui.elastic.co/pr_7439/#/layout/flyout#resizable-flyouts

General checklist

• Browser QA
  • Checked in both light and dark modes
  • 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
    • Jest tests skipped as this functionality requires a lot more mocking in jsdom (widths etc), and is significantly less painful in Cypress/E2E. Code coverage is at 100% with Cypress 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
  • Updated the Figma library counterpart
    • Not sure if this should be added. Will check at the next Figma meeting

[tkajtoch]

I love how straightforward this solution is!

I was afraid adding 4 event listeners to window on mouse down and removing on mouse up might cause a small performance hit, but it seems everything's just fine 💯

[andreadelrio]

Areas I'm looking for feedback on:

• UI/UX - do we need more visual indicators or UI hints that the edge is resizable? Or does the existing appearance feel sufficient?

I think the visual indicators you currently have are adequate. To add more context, what we've been doing lately in Discover is manually hiding the icons in EuiResizableContainer (menuDown, menuLeft, menuRight, menuUp). The reasoning behind that is that we've found that those icons then to be overkill and can crowd the UI a bit. If you look at references such as Figma, Slack and Github they follow the pattern you're following here: they change the cursor and make the line a bit thicker and/or show it in blue.

[JasonStoltz]

Just curious. For the API, how did you decide to go with a new component variant as opposed to a resizable prop?

[cee-chen]

@JasonStoltz that's mentioned in the PR description! It's actually an area I was looking for feedback on:

API - does adding a new <EuiFlyoutResizable /> component (as opposed to, e.g. <EuiFlyout resizable={true} />) make sense? I leaned this way because it felt the cleanest in terms of implementation (as opposed to adding Yet More Code to the existing logic-heavy flyout file), but definitely would like to hear feedback on this.

After having some time to sit on it, I'm like a 7.5/10 sold on the API choice - I think the code cleanliness outweighs the benefits, and having it as a separate component also I think adds a small extra friction that makes it so consumers won't reach for resizable flyouts without thinking at least a little more on it first (which I think we do want).

WDYT? Am I way off on that?

[JasonStoltz]

@cee-chen Ugh, I am so sorry for not reading that PR description 🤦.

If we set the underlying implementation aside, the "resizable" prop feels like the right interface.

Is it possible to keep the logic for this cleanly separated like you have it, but maintain a prop-based API?

Something like:

export const EUIFlyout = ({ resizable }) => {
  if (resizable) {
    return EUIFlyoutResizable;
  }
  
  return EUIFlyoutBase;
}

[JasonStoltz]

@kyrspl @boriskirov @mdefazio @cee-chen Is there any additional guidance you think we should add that goes along with this new option? Are there any considerations or recommendations we can make around when to make a flyout resizable or not?

If you'll recall, we had a lengthy conversation here, I'm hoping to try to gather some of that into our docs.

Here is the copy currently in this PR:

You can use EuiFlyoutResizable to render a flyout that users can drag with their mouse or use arrow keys to resize. This may be useful for scenarios where the space the user needs can be unpredictable, if content is dynamic. Resizable flyouts allow users to adjust content to better fit their individual screens and workflows.

We strongly recommend setting reasonable numerical minWidth and maxWidth props based on the flyout content and page content that you can predict.

Things we discussed in that thread that I'm wondering about:

• Push flyouts vs non-push flyouts -- is this optimal for one or the other?
• Responsiveness - do we need to encourage folks using this option to test their layouts with resizing at various resolutions to ensure their layouts continue to work?
• Cee: You mentioned a number of technical considerations here, should folks using this be aware of any performance impllications
• If a user needs more space, should they simply set a larger flyout width, or make it resizable? Do we have a preference as a UX team which they use?

[cee-chen]

Is it possible to keep the logic for this cleanly separated like you have it, but maintain a prop-based API?

Personally, I don't think so. Adding another EuiFlyoutBase component would create a decent amount of shenanigans primarily around refs and types (due to the as prop which lets consumers render a custom DOM element rendered, which then affects ref typing). I could also see it leading to an annoying amount of Enzyme snapshot churn due to the added wrapper. I just personally don't see the benefit worth time spent, I guess?

Cee: You mentioned a number of technical considerations #7290 (comment), should folks using this be aware of any performance impllications

Ha, past me was so smart, good thing I totally forgot everything I wrote there! Most of the bullet points are addressed but I totally forgot the perf/debouncer bit. That would be good to add pre-emptively to be honest. After some spiking, it's actually a bit of a pain 🥲 I'd be tempted to hang tight on this to see what performance is like before adding any performance optimizations.

One other option we can do here is to flag the component as a Beta one (probably should have done that anyway) and continue to build up the docs and perf behavior in the interim, if Kibana needs the component sooner rather than later. IMO, beta functionality is a fairly solid reason to make this a separate component as well.

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: 0fa3594

History

• 💚 Build #1200 succeeded 579065a
• 💔 Build #1199 failed 3da2ad0

[kibanamachine]

Preview staging links for this PR:

• Docs site: https://eui.elastic.co/pr_7439/
• Storybook: https://eui.elastic.co/pr_7439/storybook

[cee-chen]

Hey y'all! Since this is a feature requested in Kibana, I'm going to go ahead and merge this in as a beta component so it gets into today's EUI release. That being said, I would still really love some answers (especially around documentation and recommendations) - any other comments that get added to this thread I'll be sure to address in a follow-up PR.

[kyrspl]

Thanks for working on this @cee-chen

Guidelines
Guidelines will be discussed in the Patterns group as we're ramping up a conversation on modality.

Edge case
While playing with the component, I noticed a scenario where the resizable flyout can be added in addition to an existing pushed flyout (see vid). What should be the expected behavior here?

Screen.Recording.2024-01-10.at.11.00.01.mov

Visual indicators
I'm torn regarding the visual indicators on resizing possibilities.

On one hand I do understand the argument of UI clutter. Conversely, removing affordances increases the mental effort to (1) know this option exists in the first place, and (2) move the cursor in the right spot to make it work
I'm also thinking here in terms of accessibility.

Here's a relevant article that describes the problem in more detail.