Skip to content

Commit

Permalink
fix: add required vertical space for combo-box overlay (#5432)
Browse files Browse the repository at this point in the history
  • Loading branch information
tomivirkki committed Feb 3, 2023
1 parent 3911915 commit 160abba
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 16 deletions.
1 change: 1 addition & 0 deletions packages/combo-box/src/vaadin-combo-box-overlay.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ export class ComboBoxOverlay extends PositionMixin(OverlayElement) {
loader.setAttribute('part', 'loader');
const content = this.shadowRoot.querySelector('[part~="content"]');
content.parentNode.insertBefore(loader, content);
this.requiredVerticalSpace = 200;
}

_outsideClickListener(event) {
Expand Down
72 changes: 58 additions & 14 deletions packages/combo-box/test/overlay-position.test.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,9 @@
import { expect } from '@esm-bundle/chai';
import { aTimeout, fire, fixtureSync, isIOS } from '@vaadin/testing-helpers';
import '../src/vaadin-combo-box.js';
import { html, PolymerElement } from '@polymer/polymer/polymer-element.js';
import '../vaadin-combo-box.js';
import './not-animated-styles.js';
import { makeItems } from './helpers.js';

class XFixed extends PolymerElement {
static get template() {
return html`
<div style="position: fixed;">
<slot></slot>
</div>
`;
}
}

customElements.define('x-fixed', XFixed);

describe('overlay position', () => {
let comboBox, dropdown, overlayPart, inputField, input;

Expand All @@ -33,6 +21,14 @@ describe('overlay position', () => {
}

beforeEach(async () => {
fixtureSync(`
<style>
vaadin-combo-box-overlay::part(overlay) {
margin: 0 !important;
}
</style>
`);

comboBox = fixtureSync(`<vaadin-combo-box label='comboBox' style='width: 300px;' items='[1]'></vaadin-combo-box>`);
const comboBoxRect = comboBox.getBoundingClientRect();
comboBox.items = makeItems(20);
Expand Down Expand Up @@ -171,6 +167,54 @@ describe('overlay position', () => {
await aTimeout(0);
expect(overlayPart.getBoundingClientRect().bottom).to.closeTo(inputField.getBoundingClientRect().top, 1);
});

it('should be above input when near bottom', async () => {
moveComboBox(xCenter, yBottom - 200, 300);

comboBox.open();
await aTimeout(1);
expect(overlayPart.getBoundingClientRect().bottom).to.closeTo(inputField.getBoundingClientRect().top, 1);
});

it('should be below input when far from bottom', async () => {
moveComboBox(xCenter, yBottom - 220, 300);

comboBox.open();
await aTimeout(1);
expect(overlayPart.getBoundingClientRect().top).to.closeTo(inputField.getBoundingClientRect().bottom, 1);
});

describe('lazy data provider', () => {
beforeEach(() => {
comboBox.items = undefined;

comboBox.dataProvider = (params, callback) => {
const index = params.page * params.pageSize;
const size = 20;
const result = [...Array(size).keys()].map((i) => ({
label: `Item ${index + i}`,
value: `item-${index + i}`,
}));
setTimeout(() => callback(result, size), 100);
};
});

it('should be above input when near bottom', async () => {
moveComboBox(xCenter, yBottom - 200, 300);

comboBox.open();
await aTimeout(1);
expect(overlayPart.getBoundingClientRect().bottom).to.closeTo(inputField.getBoundingClientRect().top, 1);
});

it('should be below input when far from bottom', async () => {
moveComboBox(xCenter, yBottom - 220, 300);

comboBox.open();
await aTimeout(1);
expect(overlayPart.getBoundingClientRect().top).to.closeTo(inputField.getBoundingClientRect().bottom, 1);
});
});
});

describe('overlay resizing', () => {
Expand Down
18 changes: 16 additions & 2 deletions packages/vaadin-overlay/src/vaadin-overlay-position-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,24 @@ export const PositionMixin = (superClass) =>
type: Boolean,
value: false,
},

/**
* If the overlay content has no intrinsic height, this property can be used to set
* the minimum vertical space (in pixels) required by the overlay. Setting a value to
* the property effectively disables the content measurement in favor of using this
* fixed value for determining the open direction.
*
* @attr {number} required-vertical-space
*/
requiredVerticalSpace: {
type: Number,
value: 0,
},
};
}
static get observers() {
return [
'__positionSettingsChanged(positionTarget, horizontalAlign, verticalAlign, noHorizontalOverlap, noVerticalOverlap)',
'__positionSettingsChanged(positionTarget, horizontalAlign, verticalAlign, noHorizontalOverlap, noVerticalOverlap, requiredVerticalSpace)',
'__overlayOpenedChanged(opened)',
];
}
Expand Down Expand Up @@ -195,7 +208,8 @@ export const PositionMixin = (superClass) =>
__shouldAlignStartVertically(targetRect) {
// Using previous size to fix a case where window resize may cause the overlay to be squeezed
// smaller than its current space before the fit-calculations.
const contentHeight = Math.max(this.__oldContentHeight || 0, this.$.overlay.offsetHeight);
const contentHeight =
this.requiredVerticalSpace || Math.max(this.__oldContentHeight || 0, this.$.overlay.offsetHeight);
this.__oldContentHeight = this.$.overlay.offsetHeight;

const viewportHeight = Math.min(window.innerHeight, document.documentElement.clientHeight);
Expand Down
32 changes: 32 additions & 0 deletions packages/vaadin-overlay/test/position-target.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,22 @@ describe('position target', () => {
expectEdgesAligned(TOP, TOP);
});

it('should flip when out of required vertical space', () => {
overlay.requiredVerticalSpace = 200;
target.style.top = `${targetPositionToFlipOverlay - 100}px`;
updatePosition();
expectEdgesAligned(BOTTOM, BOTTOM);
});

it('should flip back when enough required vertical space', () => {
overlay.requiredVerticalSpace = 200;
target.style.top = `${targetPositionToFlipOverlay - 100}px`;
updatePosition();
target.style.top = `${targetPositionToFlipOverlay - 200}px`;
updatePosition();
expectEdgesAligned(TOP, TOP);
});

it('should choose the bigger side when it fits neither', () => {
overlayContent.style.height = document.documentElement.clientHeight + 'px';

Expand Down Expand Up @@ -258,6 +274,22 @@ describe('position target', () => {
expectEdgesAligned(BOTTOM, BOTTOM);
});

it('should flip when out of required vertical space', () => {
overlay.requiredVerticalSpace = 200;
target.style.top = `${targetPositionToFlipOverlay + 100}px`;
updatePosition();
expectEdgesAligned(TOP, TOP);
});

it('should flip back when enough required vertical space', () => {
overlay.requiredVerticalSpace = 200;
target.style.top = `${targetPositionToFlipOverlay + 100}px`;
updatePosition();
target.style.top = `${targetPositionToFlipOverlay + 200}px`;
updatePosition();
expectEdgesAligned(BOTTOM, BOTTOM);
});

it('should choose the bigger side when it fits neither', () => {
overlayContent.style.height = document.documentElement.clientHeight + 'px';

Expand Down

0 comments on commit 160abba

Please sign in to comment.