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

[EuiInputPopover] In fixed container (like flyout) has positioning issues #1023

Closed
Tracked by #4054
snide opened this issue Jul 18, 2018 · 11 comments
Closed
Tracked by #4054

Comments

@snide
Copy link
Contributor

snide commented Jul 18, 2018

We'll want to get this fixed before FF. cc @bmcconaghy so he's aware we're on it.

@cchaos
Copy link
Contributor

cchaos commented Jul 18, 2018

It will probably be fixed if they then use the container prop created in #966

@chandlerprall
Copy link
Contributor

Shouldn't need the container prop, as it tries to always fit the content within the window bounds - which also means adding a container shouldn't effect anything.

@chandlerprall
Copy link
Contributor

This is likely to be fixed by the popover watching for dynamic content in #966, will be merged soon.

@chandlerprall
Copy link
Contributor

Dave verified this is resolved in the latest EUI version.

@nreese nreese reopened this Nov 18, 2018
@nreese
Copy link
Contributor

nreese commented Nov 18, 2018

I am seeing this issue in the latest Kibana and EUI 5.0.0.

The color picker positioning is messed up in a flyout. To reproduce, put a color picker in a flyout. Scroll the flyout before opening the color picker. Open the color picker. Notice that it opens at the old location of the anchor and not the new location.

screen shot 2018-11-18 at 1 42 15 pm

Also, the EuiComboBox menu does not stay with the input when scrolled in a flyout

screen shot 2018-11-18 at 1 45 24 pm

@cchaos
Copy link
Contributor

cchaos commented Nov 19, 2018

We should look into the combo box (and probably check the super select too), but the color picker will never work until we re-write the component to use our own popovers. Right now it's just using an absolutely positioned "pop up" and not our portalled popover since it's using an external library.

@chandlerprall
Copy link
Contributor

For reference, #856 is the WIP custom color picker PR

@chandlerprall chandlerprall removed their assignment Oct 28, 2019
@cjcenizal
Copy link
Contributor

Here's how it looks with combo-boxes in flyouts when the underlying page is scrolled (EUI 16.0.0):

scrolling_fixed_popover

@cchaos
Copy link
Contributor

cchaos commented Mar 16, 2020

The above example is not an EuiComboBox but an EuiSuperSelect. EuiComboBoxes are fixed by auto-closing on scroll. Likely we'll want to do the same for EuiSuperSelect.

@Tocknicsu
Copy link
Contributor

Hello there.

I created a minimum app. https://codesandbox.io/s/tvrux and https://tvrux.csb.app/ is preview page.
Please go to preview page, then click the fullscreen and try those button.

The app has two sections. Each section has two button, one is eui and another is react-bootstrap.
First section is using window scroll and second section is using div scroll.

And all the situation are expected as this comment mentions #1563 (comment)

The popover also does stick to the input if the input is in the normal DOM and the window scrolls. However it does not stick to the input if the input is in a fixed-position element or an element that scrolls independently of the window scroll.

In my app, the functionality of fullscreen is needed. But, window cannot be scroll in fullscreen mode. Therefore, I need to set overflow: auto to body to make body can be scroll. And in this situation, all the eui popover position (including initial position after click) is weird when scroll position of body is not 0, but react-bootstrap popover looks fine.

react-bootstrap using popper library (https://popper.js.org/), and this library has react version (https://popper.js.org/react-popper).

I also trace both library. eui only calc window at https://github.com/elastic/eui/blob/master/src/services/popover/popover_positioning.ts#L228.
popper deal with all parents recursively https://github.com/popperjs/popper-core/blob/master/src/index.js#L89 and https://github.com/popperjs/popper-core/blob/master/src/dom-utils/listScrollParents.js

Finally, I wonder that would you think about using popper library or improve to support recursively calculate parents.

@cchaos cchaos changed the title Popover in fixed container (like flyout) has positioning issues [EuiInputPopover] In fixed container (like flyout) has positioning issues Sep 19, 2020
@chandlerprall
Copy link
Contributor

Another instance (elastic/kibana#108869) with reproduction (https://codesandbox.io/s/damp-wave-mpikw)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants