Skip to content
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

[Dashboard] Add panel flyout opens 2 flyouts #64789

Closed
cchaos opened this issue Apr 29, 2020 · 7 comments · Fixed by #65861 or #76931
Closed

[Dashboard] Add panel flyout opens 2 flyouts #64789

cchaos opened this issue Apr 29, 2020 · 7 comments · Fixed by #65861 or #76931
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:Dashboard Dashboard related features impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. v7.8.0

Comments

@cchaos
Copy link
Contributor

cchaos commented Apr 29, 2020

Kibana version: At least master, but most likely also 7.7 and possibly even earlier

Describe the bug:

There's an EuiFlyout inception that is happening when opening the "Add panel" flyout from Dashboard.

Steps to reproduce:

  1. Open or create a new Dashboard
  2. Click the "New" button in the menu
  3. Watch as you see two levels of flyouts appear during the animation.

Once they're fully open it's hard to tell until you search the rendered dom.

Expected behavior:

One flyout please. The two flyouts can cause the animation to be sluggish and stutter. It will also cause problems with the EUI upgrade that pushes the panel below the header. Nested flyouts will double that amount.

image

Screenshots (if relevant):

image
image

cc @stacey-gammon

@cchaos cchaos added bug Fixes for quality problems that affect the customer experience Feature:Dashboard Dashboard related features Team:AppArch impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. v7.8.0 labels Apr 29, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

@Dosant
Copy link
Contributor

Dosant commented May 8, 2020

I'll pick it up because had similar issue with drilldowns flyout

@cchaos
Copy link
Contributor Author

cchaos commented Aug 26, 2020

This issue seems to be back. It happens when using the "Replace panel" function in dashboards. Here is a video you can scrub through to see the problem.

@Dosant
Copy link
Contributor

Dosant commented Sep 8, 2020

Thanks @cchaos,

@cchaos
Copy link
Contributor Author

cchaos commented Sep 8, 2020

I think it's hard for EUI to know what is "intended" for this kind of behavior. For instance, it is possibly valid to have two flyouts where one flyout triggers another. So I'd suggest the validator be part of the overlays service. I'd even recommend, if you can trying to get more plugins to use this service because it then provides a single place to be able to adjust for any other overlays as well. Not sure the best way of doing this except for trying to provide a warning message somewhere when using EuiFlyout directly. (Like maybe a custom lint message)

@pgayvallet
Copy link
Contributor

since this is so easy mistake to make when using overlays.openFlyout, what do you think about adding a runtime check that would through in development in case is passed as a child in a component tree

I'm not sure that is really doable, due to the fact that the API is agnostic and just accepts a MountPoint

open(mount: MountPoint, options?: OverlayFlyoutOpenOptions): OverlayRef;

@Dosant
Copy link
Contributor

Dosant commented Sep 8, 2020

@pgayvallet, bummer... alternatively we probably could actually check DOM inside that method?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Dashboard Dashboard related features impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. v7.8.0
Projects
None yet
4 participants