From 3545740cbfdb5d80957fda53e028e913371e956b Mon Sep 17 00:00:00 2001 From: Chandler Date: Mon, 25 Jun 2018 11:01:59 -0600 Subject: [PATCH] Update EuiComboBox to use tthe new popover service (#946) * Update EuiComboBox to use new popover service * Refactor internal combobox updateListPosition method * changelog * Account for horizontal scroll position --- CHANGELOG.md | 1 + src/components/combo_box/combo_box.js | 40 +++++++++---------- .../combo_box_options_list.js | 2 +- src/services/popover/popover_positioning.js | 15 ++++--- .../popover/popover_positioning.test.js | 26 ++++++++++++ 5 files changed, 57 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39f9248dbb3..4e7fd325c2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ **Bug fixes** - `EuiTooltip` re-positions content correctly after the window is resized ([#936](https://github.com/elastic/eui/pull/936)) +- `EuiComboBox` list is positioned correctly in IE ([#946](https://github.com/elastic/eui/pull/946)) ## [`0.0.55`](https://github.com/elastic/eui/tree/v0.0.55) diff --git a/src/components/combo_box/combo_box.js b/src/components/combo_box/combo_box.js index 7daf510d1b7..ce2153cbe2e 100644 --- a/src/components/combo_box/combo_box.js +++ b/src/components/combo_box/combo_box.js @@ -11,7 +11,7 @@ import PropTypes from 'prop-types'; import classNames from 'classnames'; import tabbable from 'tabbable'; -import { comboBoxKeyCodes, calculatePopoverPosition } from '../../services'; +import { comboBoxKeyCodes, findPopoverPosition } from '../../services'; import { BACKSPACE, TAB, ESCAPE } from '../../services/key_codes'; import { EuiPortal } from '../portal'; import { EuiComboBoxInput } from './combo_box_input'; @@ -59,7 +59,7 @@ export class EuiComboBox extends Component { this.state = { matchingOptions: getMatchingOptions(options, selectedOptions, initialSearchValue, props.async), - listBounds: undefined, + listElement: undefined, searchValue: initialSearchValue, isListOpen: false, listPosition: 'bottom', @@ -87,7 +87,9 @@ export class EuiComboBox extends Component { }); }; - updateListPosition = (listBounds = this.state.listBounds) => { + updateListPosition = ( + listElement = this.state.listElement + ) => { if (!this._isMounted) { return; } @@ -96,33 +98,29 @@ export class EuiComboBox extends Component { return; } - if (!listBounds) { + if (!listElement) { return; } const comboBoxBounds = this.comboBox.getBoundingClientRect(); - listBounds = { - bottom: listBounds.bottom, - height: listBounds.height, - left: comboBoxBounds.left, - right: comboBoxBounds.right, - top: listBounds.top, - width: comboBoxBounds.width, - x: listBounds.x, - y: listBounds.y, - }; - - const { position, left, top } = calculatePopoverPosition(comboBoxBounds, listBounds, 'bottom', 0, ['bottom', 'top']); + const { position, top } = findPopoverPosition({ + anchor: this.comboBox, + popover: listElement, + position: 'bottom', + allowCrossAxis: false + }); - this.optionsList.style.top = `${top + window.scrollY}px`; - this.optionsList.style.left = `${left}px`; + this.optionsList.style.top = `${top}px`; + // listElement doesn't have its width set until after updating the position + // which means the popover service won't know about the correct width + // however, we already know where to position the element + this.optionsList.style.left = `${comboBoxBounds.left + window.pageXOffset}px`; this.optionsList.style.width = `${comboBoxBounds.width}px`; - // Cache for future calls. Assign values directly instead of destructuring because listBounds is - // a DOMRect, not a JS object. + // Cache for future calls. this.setState({ - listBounds, + listElement, width: comboBoxBounds.width, listPosition: position, }); diff --git a/src/components/combo_box/combo_box_options_list/combo_box_options_list.js b/src/components/combo_box/combo_box_options_list/combo_box_options_list.js index fc1bd11bd1b..0e0d079201e 100644 --- a/src/components/combo_box/combo_box_options_list/combo_box_options_list.js +++ b/src/components/combo_box/combo_box_options_list/combo_box_options_list.js @@ -51,7 +51,7 @@ export class EuiComboBoxOptionsList extends Component { updatePosition = () => { // Wait a beat for the DOM to update, since we depend on DOM elements' bounds. requestAnimationFrame(() => { - this.props.updatePosition(this.list.getBoundingClientRect()); + this.props.updatePosition(this.list); }); }; diff --git a/src/services/popover/popover_positioning.js b/src/services/popover/popover_positioning.js index bc0eea27cbe..7b033176202 100644 --- a/src/services/popover/popover_positioning.js +++ b/src/services/popover/popover_positioning.js @@ -36,13 +36,14 @@ const positionSubstitutes = { * @param position {string} Position the user wants. 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 + * @param [allowCrossAxis=true] {boolean} Whether to allow the popover to be positioned on the cross-axis * @param [container] {HTMLElement|React.Component} Element the popover must be constrained to fit within * @param [arrowConfig] {{arrowWidth: number, arrowBuffer: number}} If present, describes the size & constraints for an arrow element, and the function return value will include an `arrow` param with position details * * @returns {{top: number, left: number, position: string, fit: number, arrow?: {left: number, top: number}}|null} absolute page coordinates for the popover, * and the placements's relation to the anchor; if there's no room this returns null */ -export function findPopoverPosition({ anchor, popover, position, buffer = 16, offset = 0, container, arrowConfig }) { +export function findPopoverPosition({ anchor, popover, position, buffer = 16, offset = 0, allowCrossAxis = true, container, arrowConfig }) { container = findDOMNode(container); // resolve any React abstractions // find the screen-relative bounding boxes of the anchor, popover, and container @@ -84,11 +85,15 @@ export function findPopoverPosition({ anchor, popover, position, buffer = 16, of */ const iterationPositions = [ - position, // Try the user-desired position first. - positionComplements[position], // Try the complementary position. - positionSubstitutes[position], // Switch to the cross axis. - positionComplements[positionSubstitutes[position]], // Try the complementary position on the cross axis. + position, // Try the user-desired position first. + positionComplements[position], // Try 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. + ); + } const { bestPosition, diff --git a/src/services/popover/popover_positioning.test.js b/src/services/popover/popover_positioning.test.js index 7c57ac26be7..7f05bdb4e5e 100644 --- a/src/services/popover/popover_positioning.test.js +++ b/src/services/popover/popover_positioning.test.js @@ -470,5 +470,31 @@ describe('popover_positioning', () => { }); }); }); + + describe('disable positioning on the cross-axis', () => { + it('forces the popover to stay on the primary axis', () => { + const anchor = document.createElement('div'); + anchor.getBoundingClientRect = () => makeBB(450, 150, 550, 50); + + const popover = document.createElement('div'); + popover.getBoundingClientRect = () => makeBB(0, 30, 100, 0); + + const container = document.createElement('div'); + container.getBoundingClientRect = () => makeBB(400, 1024, 600, 0); + + expect(findPopoverPosition({ + position: 'top', + anchor, + popover, + container, + allowCrossAxis: false + })).toEqual({ + fit: 0.34, + position: 'top', + top: 350, + left: 85 + }); + }); + }); }); });