Skip to content

Commit

Permalink
Bug/popover realignment (#1197)
Browse files Browse the repository at this point in the history
* Discard/ignore desired popover alignment when positioning on the cross-axis

* Update unit test to check that alignment is discarded on cross-axis tests

* changelog
  • Loading branch information
chandlerprall authored Sep 17, 2018
1 parent a46bdac commit 342ac83
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

**Bug fixes**

- Fixed cross-axis alignment bug when positioning EuiPopover ([#1197](https://github.com/elastic/eui/pull/1197))
- Added background to `readOnly` inputs ([#1188](https://github.com/elastic/eui/pull/1188))
- Fixed some modal default and responsive sizing ([#1188](https://github.com/elastic/eui/pull/1188))
- Fixed z-index issue of `EuiComboBoxOptionsList` especiall inside modals ([#1192](https://github.com/elastic/eui/pull/1192))
Expand Down
6 changes: 4 additions & 2 deletions src/services/popover/popover_positioning.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,16 +99,18 @@ export function findPopoverPosition({
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 {
bestPosition,
} = iterationPositions.reduce(({ bestFit, bestPosition }, iterationPosition) => {
} = iterationPositions.reduce(({ bestFit, bestPosition }, iterationPosition, idx) => {
// If we've already found the ideal fit, use that position.
if (bestFit === 1) {
return { bestFit, bestPosition };
Expand All @@ -117,7 +119,7 @@ export function findPopoverPosition({
// Otherwise, see if we can find a position with a better fit than we've found so far.
const screenCoordinates = getPopoverScreenCoordinates({
position: iterationPosition,
align,
align: iterationAlignments[idx],
anchorBoundingBox,
popoverBoundingBox,
windowBoundingBox,
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 @@ -473,6 +473,32 @@ describe('popover_positioning', () => {
left: 85
});
});

it('ignores any specified alignment', () => {
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',
align: 'down',
anchor,
popover,
container,
offset: 5
})).toEqual({
fit: 1,
position: 'top',
top: 45,
left: 85
});
});
});

describe('placement falls back to second complementary position', () => {
Expand Down

0 comments on commit 342ac83

Please sign in to comment.