Update popover position when children contents change by chandlerprall · Pull Request #966 · elastic/eui

chandlerprall · 2018-07-03T16:51:29Z

features / bug fixes

`EuiMutationObserver`

Adds a component that wraps components with the mutation observer API.

`EuiPortal`

A new optional prop called insert that allows the portal to be inserted in arbitrary locations in the DOM.

`EuiPopover`

Dynamic content

Popover repositions with dynamic content, such as an animated context menu height
fixes #958 fixes #955

Container restriction

An optional container prop allows restricting the popover boundaries
supports canvas euiification

`EuiWrappingPopover`

New component that allows non-React DOM elements to be used as popover anchors
supports Kibana navigation euiification

todo

Details

solve popover jumping around when position is top
EuiMutationObserver docs, tests
popover with external anchor + docs

browser testing

Details

IE11

popover
accordion
portal
context menu

Edge

popover
accordion
portal
context menu

Chrome

popover
accordion
portal
context menu

FF

popover
accordion
portal
context menu

Safari

popover
accordion
portal
context menu

cchaos · 2018-07-10T21:00:10Z

Not sure if this is in your purview, but I just saw this happen in Kibana where if the contents of the popover itself changes, the position doesn't get updated.

chandlerprall · 2018-07-10T22:53:47Z

@cchaos I've updated this PR to fix that positioning issue for popovers, the tooltip uses separate logic so I've created #996 to track

cchaos · 2018-07-11T21:36:15Z

Is this ready? If so, want to remove the "WIP" from the title?

chandlerprall · 2018-07-11T21:56:21Z

@cchaos title updated, thanks!

cchaos

Seems to work well! I checked in IE11 too for good measure. Had a couple comments, mainly around documentation.

cchaos · 2018-07-11T21:43:47Z

+    }],
+    text: (
+      <p>
+        <EuiCode>MutationObserver</EuiCode> is a wrapper around the Mutation Observer API.


Can you link to the API and describe more about what it is for?

cchaos · 2018-07-11T21:51:03Z

+    }],
+    text: (
+      <p>
+        There is an optional <EuiCode>insert</EuiCode> prop that can specify the portal&quot;s


Since the props list won't properly list out what the insert object accepts, can you explain it here.

"s => '

I've added the props tab with a descriptive note, and expanded the writeup.

cchaos · 2018-07-11T21:52:31Z

+    };
+  }
+
+  setButtonRef = node => this.buttonRef = node


missing semicolon

cchaos · 2018-07-11T22:01:28Z

+
+import PopoverContainer from './popover_container';
+const popoverContainerSource = require('!!raw-loader!./popover_htmlelement_anchor');
+const popoverContainerHtml = renderToHtml(PopoverHTMLElementAnchor);


Pointing to wrong sources here.

great catch, thanks!

cchaos · 2018-07-11T22:10:51Z

+      <div>
+        <p>
+          <EuiCode>EuiWrappingPopover</EuiCode> is an extra popover component that allows
+          any existing DOM element to be passed as the <EuiCode>button</EuiCode> prop.


I don't really understand the use-case for this one, but I'm guessing someone needed it?

Yeah, Nathan needs it for the top nav changes.

chandlerprall · 2018-07-12T18:01:21Z

@cchaos pushed changes from your feedback

cchaos

Great, thx. Just one grammar issue.

cchaos · 2018-07-12T19:10:16Z

+        </p>
+        <p>
+          <EuiCode>insert</EuiCode> is an object with two key/values: <EuiCode>sibling</EuiCode>
+          and <EuiCode>position</EuiCode>. sibling is either the React node or HTMLElement


Capitalize sibling

sibling should remain lowercase as the property name itself is lowercase. Verified with @gchaps, who reworded it to read better.

…height transition

…e active

…rver

…ver anchors

cchaos · 2018-07-16T17:58:46Z

@@ -189,7 +236,7 @@ export class EuiPopover extends Component {
      }


@chandlerprall I just realized I messed up this arrowBuffer value, would you mind setting it to 10 instead of 0?

nreese · 2018-07-18T16:16:32Z

This works just as I expected and allows for anchoring popovers to angular elements in Kibana. Below is a screen shot of a popover anchored to a button element rendered by angular. It works as expected. Thanks

chandlerprall · 2018-07-18T16:53:51Z

jenkins test this

chandlerprall force-pushed the bug/955-popover-children-resize branch from 58a5025 to 903374c Compare July 9, 2018 19:51

This was referenced Jul 9, 2018

Accordion: Mutation observer is not removed #980

Closed

Accordion: Race condition in Mutation observer #981

Closed

chandlerprall force-pushed the bug/955-popover-children-resize branch from 01088fe to 36f7e9f Compare July 11, 2018 17:44

chandlerprall requested review from cchaos and nreese July 11, 2018 19:21

chandlerprall force-pushed the bug/955-popover-children-resize branch from 5e3ee29 to aa4f99c Compare July 11, 2018 19:25

chandlerprall changed the title ~~WIP: update popover position when children contents change~~ Update popover position when children contents change Jul 11, 2018

cchaos suggested changes Jul 11, 2018

View reviewed changes

cchaos approved these changes Jul 12, 2018

View reviewed changes

chandlerprall force-pushed the bug/955-popover-children-resize branch from 1078ac9 to 1db2d3d Compare July 12, 2018 22:15

chandlerprall added 14 commits July 13, 2018 09:15

create EuiMutationObserver util; implement it on accordion and popover

3652e69

updated EuiContextMenu to trigger popover's mutation observer during …

29b80b8

…height transition

another approach to the popover conundrum

7b6a23c

change popover to call requestAnimationFrame while a transition may b…

a842dcd

…e active

refactor EuiMutationObserver to support multiple children

0add8cd

Added full MutationObserver polyfill, added tests for EuiMutationObse…

948acdc

…rver

Allow EuiPortal to be inserted at specific locations in the DOM

12b01cc

Added documentation for EuiMutationObserver

0649bed

EuiPopover now watches for text changes too

7545fc2

Add EuiWrappingPopover to allow non-React elements to be used as popo…

08b86bc

…ver anchors

changelog and some cleanup

f736f8a

Add container prop to EuiPopover

ce8a6d2

more changelog

3193dc0

PR feedback

3c7415b

re-worded portal docs

66cf140

chandlerprall force-pushed the bug/955-popover-children-resize branch from 1db2d3d to 66cf140 Compare July 13, 2018 15:16

Allow EuiPopover to be created with isOpen=true

e6bdd42

cchaos reviewed Jul 16, 2018

View reviewed changes

Update EuiPopover's arrow positioning

3db245c

cchaos mentioned this pull request Jul 18, 2018

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

Closed

Merge branch 'master' into bug/955-popover-children-resize

4b0bb65

chandlerprall merged commit c3759f7 into elastic:master Jul 18, 2018

snide mentioned this pull request Oct 1, 2018

[BUG] Index management manage menu creates scroll on main window elastic/kibana#23647

Closed

		@@ -189,7 +236,7 @@ export class EuiPopover extends Component {
		}

Conversation

chandlerprall commented Jul 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

features / bug fixes

EuiMutationObserver

EuiPortal

EuiPopover

Dynamic content

Container restriction

EuiWrappingPopover

todo

browser testing

Uh oh!

cchaos commented Jul 10, 2018

Uh oh!

chandlerprall commented Jul 10, 2018

Uh oh!

cchaos commented Jul 11, 2018

Uh oh!

chandlerprall commented Jul 11, 2018

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chandlerprall commented Jul 12, 2018

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nreese commented Jul 18, 2018

Uh oh!

chandlerprall commented Jul 18, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chandlerprall commented Jul 3, 2018 •

edited

Loading

`EuiMutationObserver`

`EuiPortal`

`EuiPopover`

`EuiWrappingPopover`