Skip to content

Commit

Permalink
Update EuiComboBox to use tthe new popover service (#946)
Browse files Browse the repository at this point in the history
* Update EuiComboBox to use new popover service

* Refactor internal combobox updateListPosition method

* changelog

* Account for horizontal scroll position
  • Loading branch information
chandlerprall authored Jun 25, 2018
1 parent adab5c5 commit 3545740
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 27 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
40 changes: 19 additions & 21 deletions src/components/combo_box/combo_box.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -87,7 +87,9 @@ export class EuiComboBox extends Component {
});
};

updateListPosition = (listBounds = this.state.listBounds) => {
updateListPosition = (
listElement = this.state.listElement
) => {
if (!this._isMounted) {
return;
}
Expand All @@ -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,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
};

Expand Down
15 changes: 10 additions & 5 deletions src/services/popover/popover_positioning.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
26 changes: 26 additions & 0 deletions src/services/popover/popover_positioning.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
});
});
});
});
});

0 comments on commit 3545740

Please sign in to comment.