From 6063ed3bc50dd5be01e9de50fae47ebfbd37fb2a Mon Sep 17 00:00:00 2001 From: Chandler Date: Wed, 3 Oct 2018 10:37:30 -0600 Subject: [PATCH] Force popover contents to stay in the same position side once open (#1199) * Force popover contents to stay in the same position side once open, unless window resizes * changelog * Make EuiPopover wait until any initial css transitions on its content have finished before locking the position; Fix popover positioning logic to disregard invalid alignments when forcing a position * Don't transition context menu height on initial rendering --- CHANGELOG.md | 23 +---- src/components/context_menu/context_menu.js | 12 +-- src/components/popover/popover.js | 96 ++++++++++++++----- src/services/popover/popover_positioning.js | 34 ++++--- .../popover/popover_positioning.test.js | 26 +++++ 5 files changed, 128 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 035ad04903e..9b240f9a76c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## [`master`](https://github.com/elastic/eui/tree/master) +- Force `EuiPopover` contents to stick to its initial position when the content changes ([#1199](https://github.com/elastic/eui/pull/1199)) + **Bug fixes** - Fix EuiToolTip to show tooltips on disabled elements ([#1222](https://github.com/elastic/eui/pull/1222)) @@ -219,27 +221,6 @@ - `EuiPopover` re-positions with dynamic content (including CSS height/width transitions) ([#966](https://github.com/elastic/eui/pull/966)) -## [`3.0.7`](https://github.com/elastic/eui/tree/v3.0.6) - -**Note: this release is a backport containing changes original made in `3.1.0`** - -- Added `EuiMutationObserver` to expose Mutation Observer API to React components ([#966](https://github.com/elastic/eui/pull/966)) -- Added `EuiWrappingPopover` which allows existing non-React elements to be popover anchors ([#966](https://github.com/elastic/eui/pull/966)) -- `EuiPopover` accepts a `container` prop to further restrict popover placement ([#966](https://github.com/elastic/eui/pull/966)) -- `EuiPortal` can inject content at arbitrary DOM locations, added `portalRef` prop ([#966](https://github.com/elastic/eui/pull/966)) - -**Bug fixes** - -- `EuiPopover` re-positions with dynamic content (including CSS height/width transitions) ([#966](https://github.com/elastic/eui/pull/966)) - -## [`3.0.6`](https://github.com/elastic/eui/tree/v3.0.5) - -**Note: this release is a backport containing changes original made in `4.0.1`** - -**Bug fixes** - -- Fixed an issue in `EuiTooltip` because IE1 didn't support `document.contains()` ([#1190](https://github.com/elastic/eui/pull/1190)) - ## [`3.0.5`](https://github.com/elastic/eui/tree/v3.0.5) **Note: this release is a backport containing changes original made in `3.6.1`** diff --git a/src/components/context_menu/context_menu.js b/src/components/context_menu/context_menu.js index 01f60d11ead..3547a67ec2c 100644 --- a/src/components/context_menu/context_menu.js +++ b/src/components/context_menu/context_menu.js @@ -103,7 +103,7 @@ export class EuiContextMenu extends Component { idToPanelMap: {}, idToPreviousPanelIdMap: {}, idAndItemIndexToPanelIdMap: {}, - idToRenderedItemsMap: {}, + idToRenderedItemsMap: this.mapIdsToRenderedItems(this.props.panels), height: undefined, outgoingPanelId: undefined, @@ -115,13 +115,11 @@ export class EuiContextMenu extends Component { }; } - componentDidMount() { - this.mapIdsToRenderedItems(this.props.panels); - } - componentDidUpdate(prevProps) { if (prevProps.panels !== this.props.panels) { - this.mapIdsToRenderedItems(this.props.panels); + this.setState({ // eslint-disable-line react/no-did-update-set-state + idToRenderedItemsMap: this.mapIdsToRenderedItems(this.props.panels), + }); } } @@ -205,7 +203,7 @@ export class EuiContextMenu extends Component { idToRenderedItemsMap[panel.id] = this.renderItems(panel.items); }); - this.setState({ idToRenderedItemsMap }); + return idToRenderedItemsMap; }; renderItems(items = []) { diff --git a/src/components/popover/popover.js b/src/components/popover/popover.js index 4b91bd030e0..fa168cddbd5 100644 --- a/src/components/popover/popover.js +++ b/src/components/popover/popover.js @@ -83,6 +83,20 @@ function getElementFromInitialFocus(initialFocus) { return initialFocus; } +function getTransitionTimings(element) { + const computedStyle = window.getComputedStyle(element); + + const computedDuration = computedStyle.getPropertyValue('transition-duration'); + let durationMatch = computedDuration.match(GROUP_NUMERIC); + durationMatch = durationMatch ? parseFloat(durationMatch[1]) * 1000 : 0; + + const computedDelay = computedStyle.getPropertyValue('transition-delay'); + let delayMatch = computedDelay.match(GROUP_NUMERIC); + delayMatch = delayMatch ? parseFloat(delayMatch[1]) * 1000 : 0; + + return { durationMatch, delayMatch }; +} + export class EuiPopover extends Component { static getDerivedStateFromProps(nextProps, prevState) { if (prevState.prevProps.isOpen && !nextProps.isOpen) { @@ -122,6 +136,8 @@ export class EuiPopover extends Component { popoverStyles: DEFAULT_POPOVER_STYLES, arrowStyles: {}, arrowPosition: null, + openPosition: null, // once a stable position has been found, keep the contents on that side + isOpenStable: false, // wait for any initial opening transitions to finish before marking as stable }; } @@ -176,7 +192,7 @@ export class EuiPopover extends Component { } if (this.props.repositionOnScroll) { - window.addEventListener('scroll', this.positionPopover); + window.addEventListener('scroll', this.positionPopoverFixed); } this.updateFocus(); @@ -193,14 +209,37 @@ export class EuiPopover extends Component { isOpening: true, }); }); + + // for each child element of `this.panel`, find any transition duration we should wait for before stabilizing + const { durationMatch, delayMatch } = Array.prototype.slice.call(this.panel.children).reduce( + ({ durationMatch, delayMatch }, element) => { + const transitionTimings = getTransitionTimings(element); + + return { + durationMatch: Math.max(durationMatch, transitionTimings.durationMatch), + delayMatch: Math.max(delayMatch, transitionTimings.delayMatch), + }; + }, + { durationMatch: 0, delayMatch: 0 } + ); + + setTimeout( + () => { + this.setState( + { isOpenStable: true }, + this.positionPopoverFixed + ); + }, + (durationMatch + delayMatch) + ); } // update scroll listener if (prevProps.repositionOnScroll !== this.props.repositionOnScroll) { if (this.props.repositionOnScroll) { - window.addEventListener('scroll', this.positionPopover); + window.addEventListener('scroll', this.positionPopoverFixed); } else { - window.removeEventListener('scroll', this.positionPopover); + window.removeEventListener('scroll', this.positionPopoverFixed); } } @@ -219,7 +258,7 @@ export class EuiPopover extends Component { } componentWillUnmount() { - window.removeEventListener('scroll', this.positionPopover); + window.removeEventListener('scroll', this.positionPopoverFixed); clearTimeout(this.closingTransitionTimeout); } @@ -228,16 +267,7 @@ export class EuiPopover extends Component { (waitDuration, record) => { // only check for CSS transition values for ELEMENT nodes if (record.target.nodeType === document.ELEMENT_NODE) { - const computedStyle = window.getComputedStyle(record.target); - - const computedDuration = computedStyle.getPropertyValue('transition-duration'); - let durationMatch = computedDuration.match(GROUP_NUMERIC); - durationMatch = durationMatch ? parseFloat(durationMatch[1]) * 1000 : 0; - - const computedDelay = computedStyle.getPropertyValue('transition-delay'); - let delayMatch = computedDelay.match(GROUP_NUMERIC); - delayMatch = delayMatch ? parseFloat(delayMatch[1]) * 1000 : 0; - + const { durationMatch, delayMatch } = getTransitionTimings(record.target); waitDuration = Math.max(waitDuration, durationMatch + delayMatch); } @@ -245,14 +275,14 @@ export class EuiPopover extends Component { }, 0 ); - this.positionPopover(); + this.positionPopoverFixed(); if (waitDuration > 0) { const startTime = Date.now(); const endTime = startTime + waitDuration; const onFrame = () => { - this.positionPopover(); + this.positionPopoverFixed(); if (endTime > Date.now()) { requestAnimationFrame(onFrame); @@ -263,12 +293,20 @@ export class EuiPopover extends Component { } } - positionPopover = () => { + positionPopover = allowEnforcePosition => { if (this.button == null || this.panel == null) return; - const { top, left, position, arrow } = findPopoverPosition({ + let position = getPopoverPositionFromAnchorPosition(this.props.anchorPosition); + let forcePosition = null; + if (allowEnforcePosition && this.state.isOpenStable && this.state.openPosition != null) { + position = this.state.openPosition; + forcePosition = true; + } + + const { top, left, position: foundPosition, arrow } = findPopoverPosition({ container: this.props.container, - position: getPopoverPositionFromAnchorPosition(this.props.anchorPosition), + position, + forcePosition, align: getPopoverAlignFromAnchorPosition(this.props.anchorPosition), anchor: this.button, popover: this.panel, @@ -292,9 +330,17 @@ export class EuiPopover extends Component { }; const arrowStyles = this.props.hasArrow ? arrow : null; - const arrowPosition = position; + const arrowPosition = foundPosition; + + this.setState({ popoverStyles, arrowStyles, arrowPosition, openPosition: foundPosition }); + } + + positionPopoverFixed = () => { + this.positionPopover(true); + } - this.setState({ popoverStyles, arrowStyles, arrowPosition }); + positionPopoverFluid = () => { + this.positionPopover(false); } panelRef = node => { @@ -306,12 +352,14 @@ export class EuiPopover extends Component { popoverStyles: DEFAULT_POPOVER_STYLES, arrowStyles: {}, arrowPosition: null, + openPosition: null, + isOpenStable: false, }); - window.removeEventListener('resize', this.positionPopover); + window.removeEventListener('resize', this.positionPopoverFluid); } else { // panel is coming into existence - this.positionPopover(); - window.addEventListener('resize', this.positionPopover); + this.positionPopoverFluid(); + window.addEventListener('resize', this.positionPopoverFluid); } }; diff --git a/src/services/popover/popover_positioning.js b/src/services/popover/popover_positioning.js index 9ca983c5a9a..d19b17169f0 100644 --- a/src/services/popover/popover_positioning.js +++ b/src/services/popover/popover_positioning.js @@ -34,6 +34,7 @@ const positionSubstitutes = { * @param anchor {HTMLElement|React.Component} Element to anchor the popover to * @param popover {HTMLElement|React.Component} Element containing the popover content * @param position {string} Position the user wants. One of ["top", "right", "bottom", "left"] + * @param [forcePosition] {boolean} If true, use only the provided `position` value and don't try any other position * @param [align] {string} Cross-axis alignment. One of ["top", "right", "bottom", "left"] * @param [buffer=16] {number} Minimum distance between the popover and the bounding container * @param [offset=0] {number} Distance between the popover and the anchor @@ -49,6 +50,7 @@ export function findPopoverPosition({ popover, align, position, + forcePosition, buffer = 16, offset = 0, allowCrossAxis = true, @@ -95,17 +97,27 @@ export function findPopoverPosition({ * if position = "right" the order is right, left, top, bottom */ - const iterationPositions = [ - position, // Try the user-desired position first. - positionComplements[position], // Try the complementary position. - ]; - const iterationAlignments = [align, align]; // keep user-defined alignment in the original and complementary positions - if (allowCrossAxis) { - iterationPositions.push( - positionSubstitutes[position], // Switch to the cross axis. - positionComplements[positionSubstitutes[position]] // Try the complementary position on the cross axis. - ); - iterationAlignments.push(null, null); // discard desired alignment on cross-axis + const iterationPositions = [position]; // Try the user-desired position first. + const iterationAlignments = [align]; // keep user-defined alignment in the original positions. + + if (forcePosition !== true) { + iterationPositions.push(positionComplements[position]); // Try the complementary position. + iterationAlignments.push(align); // keep user-defined alignment in the complementary position. + + if (allowCrossAxis) { + iterationPositions.push( + positionSubstitutes[position], // Switch to the cross axis. + positionComplements[positionSubstitutes[position]] // Try the complementary position on the cross axis. + ); + iterationAlignments.push(null, null); // discard desired alignment on cross-axis + } + } else { + // position is forced, if it conficts with the alignment then reset align to `null` + // e.g. original placement request for `downLeft` is moved to the `left` side, future calls + // will position and align `left`, and `leftLeft` is not a valid placement + if (position === align || position === positionComplements[align]) { + iterationAlignments[0] = null; + } } const { diff --git a/src/services/popover/popover_positioning.test.js b/src/services/popover/popover_positioning.test.js index 1401a145bb0..6ed6c225b81 100644 --- a/src/services/popover/popover_positioning.test.js +++ b/src/services/popover/popover_positioning.test.js @@ -499,6 +499,32 @@ describe('popover_positioning', () => { left: 85 }); }); + + it('respects forcePosition value', () => { + const anchor = document.createElement('div'); + anchor.getBoundingClientRect = () => makeBB(100, 150, 120, 50); + + const popover = document.createElement('div'); + popover.getBoundingClientRect = () => makeBB(0, 30, 50, 0); + + // give the container limited space on both left and right, forcing to top + const container = document.createElement('div'); + container.getBoundingClientRect = () => makeBB(0, 160, 768, 40); + + expect(findPopoverPosition({ + position: 'right', + forcePosition: true, + anchor, + popover, + container, + offset: 5 + })).toEqual({ + fit: 0, + position: 'right', + top: 85, + left: 155 + }); + }); }); describe('placement falls back to second complementary position', () => {