Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: use overlay-position-mixin with combo-box #2497

Merged
merged 15 commits into from
Sep 11, 2021
Merged

refactor: use overlay-position-mixin with combo-box #2497

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
d71660b
refactor: use position-mixin with combo-box overlay
tomivirkki Sep 10, 2021
72a6c27
refactor: update overlay position mixin to use boundingclientrect diff
tomivirkki Sep 10, 2021
2e86e17
test: use not animated styles in a test
tomivirkki Sep 10, 2021
b83359b
test: test fixes
tomivirkki Sep 10, 2021
1b27746
test: test fixes
tomivirkki Sep 10, 2021
f8b81a0
chore: restore previous overlay max height
tomivirkki Sep 10, 2021
9b3fc0d
chore: cleanup
tomivirkki Sep 10, 2021
b02ab1e
refactor: move outside click logic to overlay
tomivirkki Sep 11, 2021
0df41f0
refactor overlay position mixin
tomivirkki Sep 11, 2021
f21b97d
fix: set overlay width on open, revert test changes
tomivirkki Sep 11, 2021
191fb14
chore: address review comments
tomivirkki Sep 11, 2021
262067b
fix: use the overlay host element to calculate position
tomivirkki Sep 11, 2021
6f86306
Update packages/vaadin-overlay/src/vaadin-overlay-position-mixin.js
tomivirkki Sep 11, 2021
cf36999
Update packages/vaadin-overlay/src/vaadin-overlay-position-mixin.js
tomivirkki Sep 11, 2021
ad50e31
chore: cleanup
tomivirkki Sep 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
158 changes: 9 additions & 149 deletions packages/vaadin-combo-box/src/vaadin-combo-box-dropdown.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ import './vaadin-combo-box-item.js';
import './vaadin-combo-box-overlay.js';
import './vaadin-combo-box-scroller.js';

const ONE_THIRD = 0.3;

const TOUCH_DEVICE = (() => {
try {
document.createEvent('TouchEvent');
Expand Down Expand Up @@ -43,8 +41,10 @@ class ComboBoxDropdown extends mixinBehaviors(IronResizableBehavior, PolymerElem
id="overlay"
hidden$="[[_isOverlayHidden(_items.*, loading)]]"
loading$="[[loading]]"
opened="[[_overlayOpened]]"
opened="{{_overlayOpened}}"
theme$="[[theme]]"
position-target="[[positionTarget]]"
no-vertical-overlap
></vaadin-combo-box-overlay>
`;
}
Expand All @@ -68,14 +68,6 @@ class ComboBoxDropdown extends mixinBehaviors(IronResizableBehavior, PolymerElem
type: Object
},

/**
* If `true`, overlay is aligned above the `positionTarget`
*/
alignedAbove: {
type: Boolean,
value: false
},

/**
* Custom function for rendering the content of the `<vaadin-combo-box-item>` propagated from the combo box element.
*/
Expand All @@ -87,8 +79,7 @@ class ComboBoxDropdown extends mixinBehaviors(IronResizableBehavior, PolymerElem
loading: {
type: Boolean,
value: false,
reflectToAttribute: true,
observer: '_loadingChanged'
reflectToAttribute: true
},

/**
Expand Down Expand Up @@ -142,17 +133,6 @@ class ComboBoxDropdown extends mixinBehaviors(IronResizableBehavior, PolymerElem
];
}

constructor() {
super();
this._boundSetPosition = this._setPosition.bind(this);
this._boundOutsideClickListener = this._outsideClickListener.bind(this);
}

connectedCallback() {
super.connectedCallback();
this.addEventListener('iron-resize', this._boundSetPosition);
}

ready() {
super.ready();

Expand Down Expand Up @@ -190,7 +170,6 @@ class ComboBoxDropdown extends mixinBehaviors(IronResizableBehavior, PolymerElem

disconnectedCallback() {
super.disconnectedCallback();
this.removeEventListener('iron-resize', this._boundSetPosition);

// Making sure the overlay is closed and removed from DOM after detaching the dropdown.
this._overlayOpened = false;
Expand All @@ -200,11 +179,7 @@ class ComboBoxDropdown extends mixinBehaviors(IronResizableBehavior, PolymerElem
super.notifyResize();

if (this.positionTarget && this.opened) {
this._setPosition();
// Schedule another position update (to cover virtual keyboard opening for example)
requestAnimationFrame(() => {
this._setPosition();
});
this._setOverlayWidth();
}
}

Expand All @@ -225,16 +200,12 @@ class ComboBoxDropdown extends mixinBehaviors(IronResizableBehavior, PolymerElem
return;
}

if (opened) {
this.$.overlay.style.position = this._isPositionFixed(this.positionTarget) ? 'fixed' : 'absolute';
this._setPosition();
this._scroller.style.maxHeight =
getComputedStyle(this).getPropertyValue('--vaadin-combo-box-overlay-max-height') || '65vh';

window.addEventListener('scroll', this._boundSetPosition, true);
document.addEventListener('click', this._boundOutsideClickListener, true);
if (opened) {
this.dispatchEvent(new CustomEvent('vaadin-combo-box-dropdown-opened', { bubbles: true, composed: true }));
} else if (!this.__emptyItems) {
window.removeEventListener('scroll', this._boundSetPosition, true);
document.removeEventListener('click', this._boundOutsideClickListener, true);
this.dispatchEvent(new CustomEvent('vaadin-combo-box-dropdown-closed', { bubbles: true, composed: true }));
}
}
Expand All @@ -249,36 +220,6 @@ class ComboBoxDropdown extends mixinBehaviors(IronResizableBehavior, PolymerElem
this.__emptyItems = false;
}

_loadingChanged() {
this._setOverlayHeight();
}

_setOverlayHeight() {
if (!this._scroller || !this.opened || !this.positionTarget) {
return;
}

const targetRect = this.positionTarget.getBoundingClientRect();

this._scroller.style.maxHeight =
getComputedStyle(this).getPropertyValue('--vaadin-combo-box-overlay-max-height') || '65vh';

const maxHeight = this._maxOverlayHeight(targetRect);

// overlay max height is restrained by the #scroller max height which is set to 65vh in CSS.
this.$.overlay.style.maxHeight = maxHeight;
}

_maxOverlayHeight(targetRect) {
const margin = 8;
const minHeight = 116; // Height of two items in combo-box
if (this.alignedAbove) {
return Math.max(targetRect.top - margin + Math.min(document.body.scrollTop, 0), minHeight) + 'px';
} else {
return Math.max(document.documentElement.clientHeight - targetRect.bottom - margin, minHeight) + 'px';
}
}

_getFocusedItem(focusedIndex) {
if (focusedIndex >= 0) {
return this._items[focusedIndex];
Expand Down Expand Up @@ -346,55 +287,6 @@ class ComboBoxDropdown extends mixinBehaviors(IronResizableBehavior, PolymerElem
return !this.loading && !(this._items && this._items.length);
}

// We need to listen on 'click' event and capture it and close the overlay before
// propagating the event to the listener in the button. Otherwise, if the clicked button would call
// open(), this would happen: https://www.youtube.com/watch?v=Z86V_ICUCD4
_outsideClickListener(event) {
const eventPath = event.composedPath();
if (eventPath.indexOf(this.positionTarget) < 0 && eventPath.indexOf(this.$.overlay) < 0) {
this.opened = false;
}
}

_isPositionFixed(element) {
const offsetParent = this._getOffsetParent(element);

return (
window.getComputedStyle(element).position === 'fixed' || (offsetParent && this._isPositionFixed(offsetParent))
);
}

_getOffsetParent(element) {
if (element.assignedSlot) {
return element.assignedSlot.parentElement;
} else if (element.parentElement) {
return element.offsetParent;
}

const parent = element.parentNode;

if (parent && parent.nodeType === 11 && parent.host) {
return parent.host; // parent is #shadowRoot
}
}

_verticalOffset(overlayRect, targetRect) {
return this.alignedAbove ? -overlayRect.height : targetRect.height;
}

_shouldAlignLeft(targetRect) {
const spaceRight = (window.innerWidth - targetRect.right) / window.innerWidth;

return spaceRight < ONE_THIRD;
}

_shouldAlignAbove(targetRect) {
const spaceBelow =
(window.innerHeight - targetRect.bottom - Math.min(document.body.scrollTop, 0)) / window.innerHeight;

return spaceBelow < ONE_THIRD;
}

_setOverlayWidth() {
const inputWidth = this.positionTarget.clientWidth + 'px';
const customWidth = getComputedStyle(this).getPropertyValue('--vaadin-combo-box-overlay-width');
Expand All @@ -406,40 +298,8 @@ class ComboBoxDropdown extends mixinBehaviors(IronResizableBehavior, PolymerElem
} else {
this.$.overlay.style.setProperty('--vaadin-combo-box-overlay-width', customWidth);
}
}

_setPosition(e) {
if (this._isOverlayHidden()) {
return;
}
if (e && e.target) {
const target = e.target === document ? document.body : e.target;
const parent = this.$.overlay.parentElement;
if (!(target.contains(this.$.overlay) || target.contains(this.positionTarget)) || parent !== document.body) {
return;
}
}

const targetRect = this.positionTarget.getBoundingClientRect();
const alignedLeft = this._shouldAlignLeft(targetRect);
this.alignedAbove = this._shouldAlignAbove(targetRect);

const overlayRect = this.$.overlay.getBoundingClientRect();
this._translateX = alignedLeft
? targetRect.right - overlayRect.right + (this._translateX || 0)
: targetRect.left - overlayRect.left + (this._translateX || 0);
this._translateY =
targetRect.top - overlayRect.top + (this._translateY || 0) + this._verticalOffset(overlayRect, targetRect);

const _devicePixelRatio = window.devicePixelRatio || 1;
this._translateX = Math.round(this._translateX * _devicePixelRatio) / _devicePixelRatio;
this._translateY = Math.round(this._translateY * _devicePixelRatio) / _devicePixelRatio;
this.$.overlay.style.transform = `translate3d(${this._translateX}px, ${this._translateY}px, 0)`;

this.$.overlay.style.justifyContent = this.alignedAbove ? 'flex-end' : 'flex-start';

this._setOverlayWidth();
this._setOverlayHeight();
this.$.overlay._updatePosition();
}

/**
Expand Down
20 changes: 16 additions & 4 deletions packages/vaadin-combo-box/src/vaadin-combo-box-overlay.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,20 @@
*/
import { OverlayElement } from '@vaadin/vaadin-overlay/src/vaadin-overlay.js';
import { registerStyles, css } from '@vaadin/vaadin-themable-mixin/register-styles.js';
import { PositionMixin } from '@vaadin/vaadin-overlay/src/vaadin-overlay-position-mixin.js';

registerStyles(
'vaadin-combo-box-overlay',
css`
:host {
margin: 0;
align-items: stretch;
#overlay {
width: var(--vaadin-combo-box-overlay-width, var(--_vaadin-combo-box-overlay-default-width, auto));
}

[part='content'] {
display: flex;
flex-direction: column;
height: 100%;
}
`,
{ moduleId: 'vaadin-combo-box-overlay-styles' }
);
Expand All @@ -24,7 +29,7 @@ registerStyles(
* @extends OverlayElement
* @private
*/
class ComboBoxOverlayElement extends OverlayElement {
class ComboBoxOverlayElement extends PositionMixin(OverlayElement) {
static get is() {
return 'vaadin-combo-box-overlay';
}
Expand All @@ -47,6 +52,13 @@ class ComboBoxOverlayElement extends OverlayElement {
const content = this.shadowRoot.querySelector('[part~="content"]');
content.parentNode.insertBefore(loader, content);
}

_outsideClickListener(event) {
const eventPath = event.composedPath();
if (!eventPath.includes(this.positionTarget) && !eventPath.includes(this)) {
this.close();
}
}
}

customElements.define(ComboBoxOverlayElement.is, ComboBoxOverlayElement);
6 changes: 4 additions & 2 deletions packages/vaadin-combo-box/test/dynamic-size.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { expect } from '@esm-bundle/chai';
import { fixtureSync } from '@vaadin/testing-helpers';
import { fixtureSync, nextRender } from '@vaadin/testing-helpers';
import { flushComboBox, getViewportItems, scrollToIndex } from './helpers.js';
import '../src/vaadin-combo-box.js';
import './not-animated-styles.js';
web-padawan marked this conversation as resolved.
Show resolved Hide resolved

describe('dynamic size change', () => {
describe('reduce size once scrolled to end', () => {
Expand Down Expand Up @@ -31,8 +32,9 @@ describe('dynamic size change', () => {
comboBox.dataProvider = dataProvider;
});

it('should not have item placeholders after size gets reduced', () => {
it('should not have item placeholders after size gets reduced', async () => {
comboBox.opened = true;
await nextRender(comboBox.$.overlay);
web-padawan marked this conversation as resolved.
Show resolved Hide resolved
scrollToIndex(comboBox, comboBox.size - 1);
flushComboBox(comboBox);
const items = getViewportItems(comboBox);
Expand Down
15 changes: 10 additions & 5 deletions packages/vaadin-combo-box/test/lazy-loading.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { expect } from '@esm-bundle/chai';
import sinon from 'sinon';
import { fixtureSync, nextFrame, aTimeout, enterKeyDown, fire } from '@vaadin/testing-helpers';
import { fixtureSync, nextFrame, aTimeout, enterKeyDown, fire, nextRender } from '@vaadin/testing-helpers';
import { flush } from '@polymer/polymer/lib/utils/flush.js';
import '@polymer/iron-input/iron-input.js';
import { ComboBoxPlaceholder } from '../src/vaadin-combo-box-placeholder.js';
Expand All @@ -14,8 +14,8 @@ import {
makeItems
} from './helpers.js';
import './not-animated-styles.js';
import '../vaadin-combo-box.js';
import '../vaadin-combo-box-light.js';
import '../src/vaadin-combo-box.js';
import '../src/vaadin-combo-box-light.js';
import { registerStyles, css } from '@vaadin/vaadin-themable-mixin/register-styles.js';

registerStyles(
Expand Down Expand Up @@ -432,7 +432,7 @@ describe('lazy loading', () => {
});

describe('changing dataProvider', () => {
it('should have correct items after changing dataProvider to return less items', () => {
it('should have correct items after changing dataProvider to return less items', async () => {
comboBox.dataProvider = (params, callback) => callback(['foo', 'bar'], 2);
comboBox.open();
comboBox.close();
Expand All @@ -441,6 +441,8 @@ describe('lazy loading', () => {
comboBox.dataProvider = (params, callback) => callback(['baz'], 1);
comboBox.open();

await nextRender(comboBox.$.overlay);
web-padawan marked this conversation as resolved.
Show resolved Hide resolved

expect(comboBox.filteredItems).to.eql(['baz']);
// The helper already excludes hidden items
const visibleItems = getAllItems(comboBox);
Expand Down Expand Up @@ -1031,10 +1033,13 @@ describe('lazy loading', () => {
const ESTIMATED_SIZE = 1234;
const allItems = makeItems(ESTIMATED_SIZE);

it('should restore the scroll position after size update', () => {
it('should restore the scroll position after size update', async () => {
await nextRender(comboBox);
const targetItemIndex = 75;
comboBox.dataProvider = getDataProvider(allItems);
await nextRender(comboBox);
comboBox.opened = true;
await nextRender(comboBox.$.overlay);
web-padawan marked this conversation as resolved.
Show resolved Hide resolved
comboBox.$.dropdown._scrollIntoView(targetItemIndex);
comboBox.size = 300;
// verify whether the scroller not jumped to 0 pos and restored properly,
Expand Down
Loading