diff --git a/packages/vaadin-combo-box/package.json b/packages/vaadin-combo-box/package.json index 28adffa966..ab8085149c 100644 --- a/packages/vaadin-combo-box/package.json +++ b/packages/vaadin-combo-box/package.json @@ -25,7 +25,6 @@ ], "dependencies": { "@polymer/iron-a11y-announcer": "^3.0.0", - "@polymer/iron-list": "^3.0.0", "@polymer/iron-resizable-behavior": "^3.0.0", "@polymer/polymer": "^3.0.0", "@vaadin/vaadin-control-state-mixin": "^22.0.0-alpha4", @@ -35,7 +34,8 @@ "@vaadin/vaadin-material-styles": "^22.0.0-alpha4", "@vaadin/vaadin-overlay": "^22.0.0-alpha4", "@vaadin/vaadin-text-field": "^22.0.0-alpha4", - "@vaadin/vaadin-themable-mixin": "^22.0.0-alpha4" + "@vaadin/vaadin-themable-mixin": "^22.0.0-alpha4", + "@vaadin/vaadin-virtual-list": "^22.0.0-alpha4" }, "devDependencies": { "@esm-bundle/chai": "^4.3.4", diff --git a/packages/vaadin-combo-box/src/vaadin-combo-box-data-provider-mixin.js b/packages/vaadin-combo-box/src/vaadin-combo-box-data-provider-mixin.js index b1098bf6fe..ad0e60817a 100644 --- a/packages/vaadin-combo-box/src/vaadin-combo-box-data-provider-mixin.js +++ b/packages/vaadin-combo-box/src/vaadin-combo-box-data-provider-mixin.js @@ -198,10 +198,6 @@ export const ComboBoxDataProviderMixin = (superClass) => if (Object.keys(this._pendingRequests).length === 0) { this.loading = false; } - if (page === 0 && this.__repositionOverlayDebouncer && items.length > (this.__maxRenderedItems || 0)) { - setTimeout(() => this.__repositionOverlayDebouncer.flush()); - this.__maxRenderedItems = items.length; - } } }; diff --git a/packages/vaadin-combo-box/src/vaadin-combo-box-dropdown-wrapper.js b/packages/vaadin-combo-box/src/vaadin-combo-box-dropdown-wrapper.js index 89a88ac6af..a4ac3d8051 100644 --- a/packages/vaadin-combo-box/src/vaadin-combo-box-dropdown-wrapper.js +++ b/packages/vaadin-combo-box/src/vaadin-combo-box-dropdown-wrapper.js @@ -4,7 +4,7 @@ * This program is available under Apache License Version 2.0, available at https://vaadin.com/license/ */ import { PolymerElement, html } from '@polymer/polymer/polymer-element.js'; -import '@polymer/iron-list/iron-list.js'; +import { Virtualizer } from '@vaadin/vaadin-virtual-list/src/virtualizer.js'; import './vaadin-combo-box-item.js'; import './vaadin-combo-box-dropdown.js'; import { ComboBoxPlaceholder } from './vaadin-combo-box-placeholder.js'; @@ -49,25 +49,16 @@ class ComboBoxDropdownWrapperElement extends PolymerElement { /* Fixes scrollbar disappearing when 'Show scroll bars: Always' enabled in Safari */ box-shadow: 0 0 0 white; } + + #selector { + border-width: var(--_vaadin-combo-box-items-container-border-width); + border-style: var(--_vaadin-combo-box-items-container-border-style); + border-color: var(--_vaadin-combo-box-items-container-border-color); + } -
- - - + +
+
@@ -117,26 +108,6 @@ class ComboBoxDropdownWrapperElement extends PolymerElement { */ theme: String, - /** - * Used to recognize if the filter changed, so to skip the - * scrolling restore. If true, then scroll to 0 position. Restore - * the previous position otherwise. - */ - filterChanged: { - type: Boolean, - value: false - }, - - /** - * Used to recognize scroller reset after new items have been set - * to iron-list and to ignore unwanted pages load. If 'true', then - * skip loading of the pages until it becomes 'false'. - */ - _resetScrolling: { - type: Boolean, - value: false - }, - _selectedItem: { type: Object }, @@ -170,19 +141,58 @@ class ComboBoxDropdownWrapperElement extends PolymerElement { _itemIdPath: String, - /** - * Stores the scroller position before updating the 'items', in - * order to restore it immediately after 'items' have been updated - */ - _oldScrollerPosition: { - type: Number, - value: 0 + __effectiveItems: { + computed: '_getItems(opened, _items)', + observer: '__effectiveItemsChanged' } }; } static get observers() { - return ['_loadingChanged(loading)', '_openedChanged(opened, _items, loading)', '_restoreScrollerPosition(_items)']; + return [ + '_loadingChanged(loading)', + '_openedChanged(opened, _items, loading)', + '__updateAllItems(_selectedItem, renderer)' + ]; + } + + constructor() { + super(); + this.__boundOnItemClick = this._onItemClick.bind(this); + } + + __effectiveItemsChanged(effectiveItems) { + if (this.__virtualizer && effectiveItems) { + this.__virtualizer.size = effectiveItems.length; + this.__virtualizer.flush(); + } + } + + __createElements(count) { + return [...Array(count)].map(() => { + const item = document.createElement('vaadin-combo-box-item'); + item.addEventListener('click', this.__boundOnItemClick); + item.tabIndex = '-1'; + item.style.width = '100%'; + return item; + }); + } + + __updateElement(el, index) { + const item = this.__effectiveItems[index]; + + el.setProperties({ + item, + index: this.__requestItemByIndex(item, index), + label: this.getItemLabel(item, this._itemLabelPath), + selected: this._isItemSelected(item, this._selectedItem, this._itemIdPath), + renderer: this.renderer, + focused: this._isItemFocused(this._focusedIndex, index) + }); + + el.setAttribute('role', this._getAriaRole(index)); + el.setAttribute('aria-selected', this._getAriaSelected(this._focusedIndex, index)); + el.setAttribute('theme', this.theme); } _fireTouchAction(sourceEvent) { @@ -195,41 +205,11 @@ class ComboBoxDropdownWrapperElement extends PolymerElement { _getItems(opened, items) { if (opened) { - if (this._isNotEmpty(items) && this._selector && !this.filterChanged) { - // iron-list triggers the scroller's reset after items update, and - // this is not appropriate for undefined size lazy loading. - // see https://github.com/vaadin/vaadin-combo-box-flow/issues/386 - // We store iron-list scrolling position in order to restore - // it later on after the items have been updated. - const currentScrollerPosition = this._selector.firstVisibleIndex; - if (currentScrollerPosition !== 0) { - this._oldScrollerPosition = currentScrollerPosition; - this._resetScrolling = true; - } - } - // Let the position to be restored in the future calls unless it's not - // caused by filtering - this.filterChanged = false; return items; } return []; } - _restoreScrollerPosition(items) { - if (this._isNotEmpty(items) && this._selector && this._oldScrollerPosition !== 0) { - // new items size might be less than old scrolling position - this._scrollIntoView(Math.min(items.length - 1, this._oldScrollerPosition)); - this._resetScrolling = false; - // reset position to 0 again in order to properly handle the filter - // cases (scroll to 0 after typing the filter) - this._oldScrollerPosition = 0; - } - } - - _isNotEmpty(items) { - return !this._isEmpty(items); - } - _isEmpty(items) { return !items || !items.length; } @@ -246,7 +226,7 @@ class ComboBoxDropdownWrapperElement extends PolymerElement { if (this._isEmpty(items)) { this.$.dropdown.__emptyItems = true; } - this.$.dropdown.opened = !!(opened && (loading || this._isNotEmpty(items))); + this.$.dropdown.opened = !!(opened && (loading || !this._isEmpty(items))); this.$.dropdown.__emptyItems = false; } @@ -265,6 +245,13 @@ class ComboBoxDropdownWrapperElement extends PolymerElement { // Prevent blurring the input when clicking inside the overlay. this.$.dropdown.$.overlay.addEventListener('mousedown', (e) => e.preventDefault()); + + this.__virtualizer = new Virtualizer({ + createElements: this.__createElements.bind(this), + updateElement: this.__updateElement.bind(this), + scrollTarget: this._scroller, + scrollContainer: this._selector + }); } _loadingChanged(loading) { @@ -272,15 +259,15 @@ class ComboBoxDropdownWrapperElement extends PolymerElement { return; } - if (loading) { - this.$.dropdown.$.overlay.setAttribute('loading', ''); - } else { - this.$.dropdown.$.overlay.removeAttribute('loading'); + this.$.dropdown.$.overlay.toggleAttribute('loading', loading); + + if (!loading && this.__virtualizer) { + setTimeout(() => this.__virtualizer.update()); } } _setOverlayHeight() { - if (!this.opened || !this.positionTarget) { + if (!this.__virtualizer || !this.opened || !this.positionTarget) { return; } @@ -293,11 +280,6 @@ class ComboBoxDropdownWrapperElement extends PolymerElement { // overlay max height is restrained by the #scroller max height which is set to 65vh in CSS. this.$.dropdown.$.overlay.style.maxHeight = maxHeight; - - // we need to set height for iron-list to make its `firstVisibleIndex` work correctly. - this._selector.style.maxHeight = maxHeight; - - this.updateViewportBoundaries(); } _maxOverlayHeight(targetRect) { @@ -327,7 +309,7 @@ class ComboBoxDropdownWrapperElement extends PolymerElement { } _onItemClick(e) { - this.dispatchEvent(new CustomEvent('selection-changed', { detail: { item: e.model.item } })); + this.dispatchEvent(new CustomEvent('selection-changed', { detail: { item: e.currentTarget.item } })); } /** @@ -352,8 +334,8 @@ class ComboBoxDropdownWrapperElement extends PolymerElement { * * @return {number} */ - __requestItemByIndex(item, index, resetScrolling) { - if (item instanceof ComboBoxPlaceholder && index !== undefined && !resetScrolling) { + __requestItemByIndex(item, index) { + if (item instanceof ComboBoxPlaceholder && index !== undefined) { this.dispatchEvent( new CustomEvent('index-requested', { detail: { index, currentScrollerPos: this._oldScrollerPosition } }) ); @@ -394,42 +376,45 @@ class ComboBoxDropdownWrapperElement extends PolymerElement { } _scrollIntoView(index) { - if (!(this.opened && index >= 0)) { + if (!this.__virtualizer || !(this.opened && index >= 0)) { return; } const visibleItemsCount = this._visibleItemsCount(); let targetIndex = index; - if (index > this._selector.lastVisibleIndex - 1) { + if (index > this.__virtualizer.lastVisibleIndex - 1) { // Index is below the bottom, scrolling down. Make the item appear at the bottom. // First scroll to target (will be at the top of the scroller) to make sure it's rendered. - this._selector.scrollToIndex(index); + this.__virtualizer.scrollToIndex(index); // Then calculate the index for the following scroll (to get the target to bottom of the scroller). targetIndex = index - visibleItemsCount + 1; - } else if (index > this._selector.firstVisibleIndex) { + } else if (index > this.__virtualizer.firstVisibleIndex) { // The item is already visible, scrolling is unnecessary per se. But we need to trigger iron-list to set // the correct scrollTop on the scrollTarget. Scrolling to firstVisibleIndex. - targetIndex = this._selector.firstVisibleIndex; + targetIndex = this.__virtualizer.firstVisibleIndex; } - this._selector.scrollToIndex(Math.max(0, targetIndex)); + this.__virtualizer.scrollToIndex(Math.max(0, targetIndex)); // Sometimes the item is partly below the bottom edge, detect and adjust. - const pidx = this._selector._getPhysicalIndex(index), - physicalItem = this._selector._physicalItems[pidx]; - if (!physicalItem) { + const lastPhysicalItem = [...this._selector.children].find( + (el) => !el.hidden && el.index === this.__virtualizer.lastVisibleIndex + ); + if (!lastPhysicalItem || index !== lastPhysicalItem.index) { return; } - const physicalItemRect = physicalItem.getBoundingClientRect(), - scrollerRect = this._scroller.getBoundingClientRect(), - scrollTopAdjust = physicalItemRect.bottom - scrollerRect.bottom + this._viewportTotalPaddingBottom; + const lastPhysicalItemRect = lastPhysicalItem.getBoundingClientRect(); + const scrollerRect = this._scroller.getBoundingClientRect(); + const scrollTopAdjust = lastPhysicalItemRect.bottom - scrollerRect.bottom + this._viewportTotalPaddingBottom; if (scrollTopAdjust > 0) { this._scroller.scrollTop += scrollTopAdjust; } } - ensureItemsRendered() { - this._selector._render(); + __updateAllItems() { + if (this.__virtualizer) { + this.__virtualizer.update(); + } } adjustScrollPosition() { @@ -444,12 +429,10 @@ class ComboBoxDropdownWrapperElement extends PolymerElement { * scrolling the parent similarly to touch scrolling. */ _patchWheelOverScrolling() { - const selector = this._selector; - selector.addEventListener('wheel', (e) => { - const scroller = selector._scroller || selector.scrollTarget; + this._selector.addEventListener('wheel', (e) => { + const scroller = this._scroller; const scrolledToTop = scroller.scrollTop === 0; const scrolledToBottom = scroller.scrollHeight - scroller.scrollTop - scroller.clientHeight <= 1; - if (scrolledToTop && e.deltaY < 0) { e.preventDefault(); } else if (scrolledToBottom && e.deltaY > 0) { @@ -458,14 +441,9 @@ class ComboBoxDropdownWrapperElement extends PolymerElement { }); } - updateViewportBoundaries() { - this._cachedViewportTotalPaddingBottom = undefined; - this._selector.updateViewportBoundaries(); - } - get _viewportTotalPaddingBottom() { if (this._cachedViewportTotalPaddingBottom === undefined) { - const itemsStyle = window.getComputedStyle(this._selector.$.items); + const itemsStyle = window.getComputedStyle(this._selector); this._cachedViewportTotalPaddingBottom = [itemsStyle.paddingBottom, itemsStyle.borderBottomWidth] .map((v) => { return parseInt(v, 10); @@ -480,10 +458,9 @@ class ComboBoxDropdownWrapperElement extends PolymerElement { _visibleItemsCount() { // Ensure items are positioned - this._selector.scrollToIndex(this._selector.firstVisibleIndex); - // Ensure viewport boundaries are up-to-date - this.updateViewportBoundaries(); - return this._selector.lastVisibleIndex - this._selector.firstVisibleIndex + 1; + this.__virtualizer.scrollToIndex(this.__virtualizer.firstVisibleIndex); + const hasItems = this.__virtualizer.size > 0; + return hasItems ? this.__virtualizer.lastVisibleIndex - this.__virtualizer.firstVisibleIndex + 1 : 0; } _stopPropagation(e) { diff --git a/packages/vaadin-combo-box/src/vaadin-combo-box-mixin.js b/packages/vaadin-combo-box/src/vaadin-combo-box-mixin.js index 499c038f6c..1f226e9388 100644 --- a/packages/vaadin-combo-box/src/vaadin-combo-box-mixin.js +++ b/packages/vaadin-combo-box/src/vaadin-combo-box-mixin.js @@ -3,9 +3,6 @@ * Copyright (c) 2021 Vaadin Ltd. * This program is available under Apache License Version 2.0, available at https://vaadin.com/license/ */ -import { timeOut } from '@polymer/polymer/lib/utils/async.js'; -import { Debouncer } from '@polymer/polymer/lib/utils/debounce.js'; -import { flush } from '@polymer/polymer/lib/utils/flush.js'; import { IronA11yAnnouncer } from '@polymer/iron-a11y-announcer/iron-a11y-announcer.js'; import { processTemplates } from '@vaadin/vaadin-element-mixin/templates.js'; import { ComboBoxPlaceholder } from './vaadin-combo-box-placeholder.js'; @@ -599,23 +596,6 @@ export const ComboBoxMixin = (subclass) => /** @private */ _onOpened() { - // Pre P2 iron-list used a debouncer to render. Now that we synchronously render items, - // we need to flush the DOM to make sure it doesn't get flushed in the middle of _render call - // because that will cause problems to say the least. - flush(); - - // With iron-list v1.3.9, calling `notifyResize()` no longer renders - // the items synchronously. It is required to have the items rendered - // before we update the overlay and the list positions and sizes. - this.$.overlay.ensureItemsRendered(); - this.$.overlay._selector.toggleScrollListener(true); - - // Ensure metrics are up-to-date - this.$.overlay.updateViewportBoundaries(); - // Force iron-list to create reusable nodes. Otherwise it will only start - // doing that in scroll listener, which might affect performance. - // See https://github.com/vaadin/vaadin-combo-box/pull/776 - this.$.overlay._selector._increasePoolIfNeeded(); setTimeout(() => this._resizeDropdown(), 1); // Defer scroll position adjustment to improve performance. window.requestAnimationFrame(() => this.$.overlay.adjustScrollPosition()); @@ -742,9 +722,8 @@ export const ComboBoxMixin = (subclass) => return; } - // Notify the dropdown about filter changing, so to let it skip the - // scrolling restore - this.$.overlay.filterChanged = true; + // Scroll to the top of the list whenever the filter changes. + this.$.overlay._scrollIntoView(0); if (this.items) { this.filteredItems = this._filterItems(this.items, filter); @@ -905,10 +884,6 @@ export const ComboBoxMixin = (subclass) => this.opened || this.autoOpenDisabled ? this.$.overlay.indexOfLabel(this.filter) : this._indexOfValue(this.value, this.filteredItems); - - if (this.opened) { - this._repositionOverlay(); - } } } @@ -949,42 +924,11 @@ export const ComboBoxMixin = (subclass) => this.$.overlay.set('_items', items); } - /** @private */ - _repositionOverlay() { - // async needed to reposition correctly after filtering - // (especially when aligned on top of input) - this.__repositionOverlayDebouncer = Debouncer.debounce( - this.__repositionOverlayDebouncer, - // Long debounce: sizing updates invoke multiple styling rounds, - // which might affect performance, especially in old browsers. - // See https://github.com/vaadin/vaadin-combo-box/pull/800 - timeOut.after(500), - () => { - const selector = this.$.overlay._selector; - if (!selector._isClientFull()) { - // Due to the mismatch of the Y position of the item rendered - // at the top of the scrolling list with some specific scroll - // position values (2324, 3486, 6972, 60972, 95757 etc.) - // iron-list loops the increasing of the pool and adds - // too many items to the DOM. - // Adjusting scroll position to equal the current scrollTop value - // to avoid looping. - selector._resetScrollPosition(selector._physicalTop); - } - this._resizeDropdown(); - this.$.overlay.updateViewportBoundaries(); - this.$.overlay.ensureItemsRendered(); - selector.notifyResize(); - flush(); - } - ); - } - /** @private */ _indexOfValue(value, items) { if (items && this._isValidValue(value)) { for (let i = 0; i < items.length; i++) { - if (this._getItemValue(items[i]) === value) { + if (items[i] !== this.__placeHolder && this._getItemValue(items[i]) === value) { return i; } } diff --git a/packages/vaadin-combo-box/test/basic.test.js b/packages/vaadin-combo-box/test/basic.test.js index 943c925adf..1e15217340 100644 --- a/packages/vaadin-combo-box/test/basic.test.js +++ b/packages/vaadin-combo-box/test/basic.test.js @@ -1,8 +1,9 @@ import { expect } from '@esm-bundle/chai'; import sinon from 'sinon'; -import { aTimeout, fixtureSync, nextFrame } from '@vaadin/testing-helpers'; +import { fixtureSync, nextFrame } from '@vaadin/testing-helpers'; import './not-animated-styles.js'; import '../vaadin-combo-box.js'; +import { getViewportItems } from './helpers.js'; describe('Properties', () => { let comboBox; @@ -27,17 +28,11 @@ describe('Properties', () => { expect(comboBox.items).to.be.undefined; }); - it('should be bound to items list', () => { - comboBox.opened = true; - comboBox.items = ['qux']; - expect(comboBox.$.overlay._selector.items).to.eql(['qux']); - }); - - it('should update items list on mutation', () => { + it('should update items list on update', () => { comboBox.opened = true; comboBox.items = []; - comboBox.push('items', 'foo'); - expect(comboBox.$.overlay._selector._virtualCount).to.eql(1); + comboBox.items = ['foo']; + expect(getViewportItems(comboBox).length).to.eql(1); }); it('should set focused index on items set', () => { @@ -61,32 +56,16 @@ describe('Properties', () => { comboBox.items = ['foo', 'bar']; comboBox.items = undefined; comboBox.opened = true; - expect(comboBox.$.overlay._selector._virtualCount).to.eql(0); + expect(getViewportItems(comboBox).length).to.eql(0); }); }); - // TODO: these tests are here to prevent possible regressions with using - // the internal properties of iron-list. These can be removed after this - // logic no longer is implemented in vaadin-combo-box. describe('visible item count', () => { - it('should calculate items correctly when all items are visible', async () => { + it('should calculate items correctly when all items are visible', () => { comboBox.items = ['foo', 'bar', 'baz', 'qux']; comboBox.open(); - await aTimeout(0); - expect(comboBox.$.overlay._visibleItemsCount()).to.eql(4); - expect(comboBox.$.overlay._selector.lastVisibleIndex).to.eql(3); - }); - - it('should calculate items correctly when some items are hidden', async () => { - const items = []; - for (let i = 0; i < 100; i++) { - items.push(i.toString()); - } - - comboBox.items = items; - comboBox.open(); - await aTimeout(0); - expect(comboBox.$.overlay._visibleItemsCount()).to.eql(comboBox.$.overlay._selector.lastVisibleIndex + 1); + expect(getViewportItems(comboBox).length).to.equal(4); + expect(getViewportItems(comboBox).pop().index).to.equal(3); }); }); diff --git a/packages/vaadin-combo-box/test/combo-box-light.test.js b/packages/vaadin-combo-box/test/combo-box-light.test.js index d499c65650..8e23e7eb43 100644 --- a/packages/vaadin-combo-box/test/combo-box-light.test.js +++ b/packages/vaadin-combo-box/test/combo-box-light.test.js @@ -9,7 +9,8 @@ import { mouseup, touchend, touchstart, - isDesktopSafari + isDesktopSafari, + nextFrame } from '@vaadin/testing-helpers'; import { resetMouseCanceller } from '@polymer/polymer/lib/utils/gestures.js'; import { PolymerElement, html } from '@polymer/polymer/polymer-element.js'; @@ -415,9 +416,10 @@ describe('nested template', () => { expect(() => comboBox.open()).not.to.throw(Error); }); - it('should not use nested template as the item template', () => { + it('should not use nested template as the item template', async () => { comboBox.open(); const firstItem = comboBox.$.overlay._selector.querySelector('vaadin-combo-box-item'); + await nextFrame(); expect(comboBox.querySelector('[slot="prefix"]').innerHTML).to.contain('1 foo'); expect(firstItem.shadowRoot.querySelector('#content').innerHTML).to.equal('bar'); }); diff --git a/packages/vaadin-combo-box/test/dynamic-size.test.js b/packages/vaadin-combo-box/test/dynamic-size.test.js index 579323f1a2..75d16ed644 100644 --- a/packages/vaadin-combo-box/test/dynamic-size.test.js +++ b/packages/vaadin-combo-box/test/dynamic-size.test.js @@ -1,5 +1,6 @@ import { expect } from '@esm-bundle/chai'; import { fixtureSync, nextFrame } from '@vaadin/testing-helpers'; +import { getViewportItems } from './helpers.js'; import '../src/vaadin-combo-box.js'; describe('dynamic size change', () => { @@ -7,16 +8,6 @@ describe('dynamic size change', () => { comboBox.$.overlay._scrollIntoView(index); } - function getVisibleItems(comboBox) { - return Array.from(comboBox.$.overlay._selector.querySelectorAll('vaadin-combo-box-item')) - .filter((item) => !item.hidden) - .filter((item) => { - const itemRect = item.getBoundingClientRect(); - const overlayRect = comboBox.$.overlay.$.dropdown.$.overlay.$.content.getBoundingClientRect(); - return itemRect.bottom >= overlayRect.top && itemRect.top <= overlayRect.bottom; - }); - } - describe('reduce size once scrolled to end', () => { let comboBox; const INITIAL_SIZE = 600; @@ -48,7 +39,7 @@ describe('dynamic size change', () => { comboBox.opened = true; scrollToIndex(comboBox, comboBox.size - 1); await nextFrame(); - const items = getVisibleItems(comboBox); + const items = getViewportItems(comboBox); expect(items.length).to.be.above(5); items.forEach((item) => { expect(item.$.content.textContent).to.be.ok; diff --git a/packages/vaadin-combo-box/test/filtering.test.js b/packages/vaadin-combo-box/test/filtering.test.js index 48f977aefa..41f62db46f 100644 --- a/packages/vaadin-combo-box/test/filtering.test.js +++ b/packages/vaadin-combo-box/test/filtering.test.js @@ -135,7 +135,7 @@ describe('filtering items', () => { setInputValue('b'); requestAnimationFrame(() => { - expect(spy.callCount).to.eql(0); + expect(spy.firstCall.firstArg).to.eql(0); done(); }); }); @@ -301,7 +301,6 @@ describe('filtering items', () => { comboBox.close(); setInputValue('bar'); - comboBox.__repositionOverlayDebouncer.flush(); expect(comboBox.opened).to.be.true; expect(comboBox._focusedIndex).to.equal(1); @@ -313,7 +312,6 @@ describe('filtering items', () => { comboBox.close(); setInputValue(''); - comboBox.__repositionOverlayDebouncer.flush(); expect(comboBox.opened).to.be.true; expect(comboBox._focusedIndex).to.equal(-1); @@ -367,13 +365,6 @@ describe('filtering items', () => { expect(resizeSpy.called).to.be.false; }); - it('should not re-position the overlay if not opened', () => { - const repositionSpy = sinon.spy(comboBox, '_repositionOverlay'); - comboBox.filteredItems = ['foo', 'bar', 'baz']; - - expect(repositionSpy.called).to.be.false; - }); - it('should not perform measurements when loading changes if not opened', () => { const measureSpy = sinon.spy(comboBox.inputElement, 'getBoundingClientRect'); comboBox.loading = true; diff --git a/packages/vaadin-combo-box/test/helpers.js b/packages/vaadin-combo-box/test/helpers.js index 477e9b7397..501a8b18d9 100644 --- a/packages/vaadin-combo-box/test/helpers.js +++ b/packages/vaadin-combo-box/test/helpers.js @@ -62,3 +62,25 @@ export const onceScrolled = (scroller) => { export const makeItems = (length) => { return Array(...new Array(length)).map((_, i) => `item ${i}`); }; + +/** + * Returns the items that are inside the bounds of the given combo box's dropdown viewport. + */ +export const getViewportItems = (comboBox) => { + const overlayRect = comboBox.$.overlay.$.dropdown.$.overlay.$.content.getBoundingClientRect(); + + return Array.from(comboBox.$.overlay._selector.querySelectorAll('vaadin-combo-box-item')) + .sort((a, b) => a.index - b.index) + .filter((item) => !item.hidden) + .filter((item) => { + const itemRect = item.getBoundingClientRect(); + return itemRect.bottom > overlayRect.top && itemRect.top < overlayRect.bottom; + }); +}; + +/** + * Scrolls the combo box dropdown to the given index. + */ +export const scrollToIndex = (comboBox, index) => { + comboBox.$.overlay.__virtualizer.scrollToIndex(index); +}; diff --git a/packages/vaadin-combo-box/test/keyboard.test.js b/packages/vaadin-combo-box/test/keyboard.test.js index ccb0427b6b..4160f8b0bb 100644 --- a/packages/vaadin-combo-box/test/keyboard.test.js +++ b/packages/vaadin-combo-box/test/keyboard.test.js @@ -10,11 +10,12 @@ import { enterKeyDown, escKeyDown, fire, - isDesktopSafari + isDesktopSafari, + nextFrame } from '@vaadin/testing-helpers'; -import { onceScrolled } from './helpers.js'; +import { getViewportItems, onceScrolled, scrollToIndex } from './helpers.js'; import './not-animated-styles.js'; -import '../vaadin-combo-box.js'; +import '../src/vaadin-combo-box.js'; describe('keyboard', () => { let comboBox; @@ -340,8 +341,6 @@ describe('keyboard', () => { }); describe('scrolling items', () => { - let selector; - beforeEach(async () => { const items = []; @@ -350,88 +349,90 @@ describe('keyboard', () => { } comboBox.open(); - selector = comboBox.$.overlay._selector; comboBox.items = items; await aTimeout(1); }); it('should scroll down after reaching the last visible item', () => { - selector.scrollToIndex(0); + scrollToIndex(comboBox, 0); comboBox._focusedIndex = comboBox.$.overlay._visibleItemsCount() - 1; - expect(selector.firstVisibleIndex).to.eql(0); + expect(getViewportItems(comboBox)[0].index).to.eql(0); arrowDownKeyDown(comboBox.inputElement); - expect(selector.firstVisibleIndex).to.eql(1); + expect(getViewportItems(comboBox)[0].index).to.eql(1); }); - it('should scroll up after reaching the first visible item', () => { - comboBox._focusedIndex = 1; - selector.scrollToIndex(1); - expect(selector.firstVisibleIndex).to.eql(1); + it('should scroll up after reaching the first visible item', async () => { + comboBox._focusedIndex = 2; + scrollToIndex(comboBox, 2); + await nextFrame(); + + expect(getViewportItems(comboBox)[0].index).to.eql(2); arrowUpKeyDown(comboBox.inputElement); - expect(selector.firstVisibleIndex).to.eql(0); + expect(getViewportItems(comboBox)[0].index).to.eql(1); }); it('should scroll to first visible when navigating down above viewport', () => { comboBox._focusedIndex = 5; - selector.scrollToIndex(50); + scrollToIndex(comboBox, 50); arrowDownKeyDown(comboBox.inputElement); - expect(selector.firstVisibleIndex).to.eql(6); + expect(getViewportItems(comboBox)[0].index).to.eql(6); }); it('should scroll to first visible when navigating up above viewport', () => { comboBox._focusedIndex = 5; - selector.scrollToIndex(50); + scrollToIndex(comboBox, 50); arrowUpKeyDown(comboBox.inputElement); - expect(selector.firstVisibleIndex).to.eql(4); + expect(getViewportItems(comboBox)[0].index).to.eql(4); }); it('should scroll to last visible when navigating up below viewport', () => { comboBox._focusedIndex = 50; - selector.scrollToIndex(0); - expect(selector.firstVisibleIndex).to.eql(0); + scrollToIndex(comboBox, 0); + expect(getViewportItems(comboBox)[0].index).to.eql(0); arrowUpKeyDown(comboBox.inputElement); - expect(selector.firstVisibleIndex).to.eql(49 - comboBox.$.overlay._visibleItemsCount() + 1); + expect(getViewportItems(comboBox)[0].index).to.eql(49 - comboBox.$.overlay._visibleItemsCount() + 1); }); it('should scroll to last visible when navigating down below viewport', () => { comboBox._focusedIndex = 50; - selector.scrollToIndex(0); - expect(selector.firstVisibleIndex).to.eql(0); + scrollToIndex(comboBox, 0); + expect(getViewportItems(comboBox)[0].index).to.eql(0); arrowDownKeyDown(comboBox.inputElement); - expect(selector.firstVisibleIndex).to.eql(51 - comboBox.$.overlay._visibleItemsCount() + 1); + expect(getViewportItems(comboBox)[0].index).to.eql(51 - comboBox.$.overlay._visibleItemsCount() + 1); }); it('should scroll to start if no items focused when opening overlay', async () => { - selector.scrollToIndex(50); + scrollToIndex(comboBox, 50); comboBox.close(); comboBox.open(); await aTimeout(0); - expect(selector.firstVisibleIndex).to.eql(0); + + expect(getViewportItems(comboBox)[0].index).to.eql(0); }); it('should scroll to focused item when opening overlay', async () => { - selector.scrollToIndex(0); + scrollToIndex(comboBox, 0); comboBox.close(); comboBox.value = '50'; comboBox.open(); await onceScrolled(comboBox.$.overlay._scroller); - expect(selector.firstVisibleIndex).to.be.within(50 - comboBox.$.overlay._visibleItemsCount(), 50); + expect(getViewportItems(comboBox)[0].index).to.be.within(50 - comboBox.$.overlay._visibleItemsCount(), 50); }); }); diff --git a/packages/vaadin-combo-box/test/lazy-loading.test.js b/packages/vaadin-combo-box/test/lazy-loading.test.js index e8c533cd4e..e33d36479e 100644 --- a/packages/vaadin-combo-box/test/lazy-loading.test.js +++ b/packages/vaadin-combo-box/test/lazy-loading.test.js @@ -1,13 +1,23 @@ import { expect } from '@esm-bundle/chai'; import sinon from 'sinon'; -import { fixtureSync, nextFrame, enterKeyDown, fire } from '@vaadin/testing-helpers'; +import { fixtureSync, nextFrame, aTimeout, enterKeyDown, fire } 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'; -import { makeItems } from './helpers.js'; +import { getViewportItems, makeItems } from './helpers.js'; import './not-animated-styles.js'; import '../vaadin-combo-box.js'; import '../vaadin-combo-box-light.js'; +import { registerStyles, css } from '@vaadin/vaadin-themable-mixin/register-styles.js'; + +registerStyles( + 'vaadin-combo-box*', + css` + :host { + --vaadin-combo-box-overlay-max-height: 400px; + } + ` +); describe('lazy loading', () => { const DEFAULT_PAGE_SIZE = 50; @@ -297,6 +307,32 @@ describe('lazy loading', () => { }; comboBox.open(); }); + + it('should rerender once loaded updated items', (done) => { + comboBox.dataProvider = (_, callback) => { + if (!comboBox.filteredItems.length) { + // First batch of items for page 0 + callback(['foo'], 1); + // Asynchronously clear the cache which leads to another request for page 0 + setTimeout(() => comboBox.clearCache()); + } else { + // Second batch of items for page 0 + callback(['bar'], 1); + + setTimeout(() => { + // Expect the renderer to have run for the updated items. + expect(getViewportItems(comboBox)[0].$.content.textContent).to.equal('bar'); + + // Avoid getting done called multiple times + if (!done._called) { + done._called = true; + done(); + } + }); + } + }; + comboBox.open(); + }); }); describe('when selecting item', () => { @@ -566,8 +602,6 @@ describe('lazy loading', () => { it('should set selectedItem matching value when items are loaded', () => { comboBox.opened = true; // loads first page of dataProvider items - comboBox.$.overlay.updateViewportBoundaries(); - comboBox.$.overlay.ensureItemsRendered(); comboBox.value = 'item 0'; expect(comboBox.selectedItem).to.equal('item 0'); }); @@ -634,8 +668,6 @@ describe('lazy loading', () => { it('should set matching selectedItem when items are loaded', () => { comboBox.opened = true; // loads first page of dataProvider items - comboBox.$.overlay.updateViewportBoundaries(); - comboBox.$.overlay.ensureItemsRendered(); comboBox.value = 'value 0'; expect(comboBox.selectedItem).to.eql({ id: 0, value: 'value 0', label: 'label 0' }); }); @@ -657,8 +689,6 @@ describe('lazy loading', () => { it('should select value matching selectedItem when items are loading', () => { comboBox.selectedItem = 'item 0'; comboBox.opened = true; - comboBox.$.overlay.updateViewportBoundaries(); - comboBox.$.overlay.ensureItemsRendered(); expect(comboBox.value).to.equal('item 0'); const selectedRenderedItemElements = Array.from( comboBox.$.overlay._selector.querySelectorAll('vaadin-combo-box-item') @@ -669,8 +699,6 @@ describe('lazy loading', () => { it('should select value matching selectedItem when items are loaded', async () => { comboBox.opened = true; - comboBox.$.overlay.updateViewportBoundaries(); - comboBox.$.overlay.ensureItemsRendered(); comboBox.selectedItem = 'item 0'; expect(comboBox.value).to.equal('item 0'); await nextFrame(); @@ -703,8 +731,6 @@ describe('lazy loading', () => { it('should select value matching selectedItem when items are loading', () => { comboBox.selectedItem = { id: 0, value: 'value 0', label: 'label 0' }; comboBox.opened = true; - comboBox.$.overlay.updateViewportBoundaries(); - comboBox.$.overlay.ensureItemsRendered(); expect(comboBox.value).to.equal('value 0'); const selectedRenderedItemElements = Array.from( comboBox.$.overlay._selector.querySelectorAll('vaadin-combo-box-item') @@ -715,8 +741,6 @@ describe('lazy loading', () => { it('should select value matching selectedItem when items are loaded', async () => { comboBox.opened = true; - comboBox.$.overlay.updateViewportBoundaries(); - comboBox.$.overlay.ensureItemsRendered(); comboBox.selectedItem = { id: 0, value: 'value 0', label: 'label 0' }; expect(comboBox.value).to.equal('value 0'); await nextFrame(); @@ -740,8 +764,6 @@ describe('lazy loading', () => { comboBox.selectedItem = { id: 0 }; comboBox.dataProvider = objectDataProvider; comboBox.opened = true; - comboBox.$.overlay.updateViewportBoundaries(); - comboBox.$.overlay.ensureItemsRendered(); await nextFrame(); flush(); const selectedRenderedItemElements = Array.from( @@ -767,8 +789,6 @@ describe('lazy loading', () => { ); }; comboBox.opened = true; - comboBox.$.overlay.updateViewportBoundaries(); - comboBox.$.overlay.ensureItemsRendered(); await nextFrame(); flush(); const selectedRenderedItemElements = Array.from( @@ -807,6 +827,21 @@ describe('lazy loading', () => { comboBox.opened = true; }); + it('should be scrolled to start on reopen', async () => { + comboBox.dataProvider = spyAsyncDataProvider; + comboBox.size = SIZE; + comboBox.opened = false; + + // Wait for the async data provider to respond + await aTimeout(0); + + // Reopen + comboBox.open(); + await nextFrame(); + + expect(getViewportItems(comboBox)[0].index).to.eql(0); + }); + it('should replace filteredItems with placeholders', () => { comboBox.dataProvider = spyAsyncDataProvider; comboBox.size = SIZE; @@ -1019,8 +1054,8 @@ describe('lazy loading', () => { // precisely to the given index, so use some margin const scrollMargin = 5; const expectedFirstVisibleIndex = targetItemIndex - comboBox.$.overlay._visibleItemsCount() - scrollMargin; - expect(comboBox.$.overlay._selector.firstVisibleIndex).to.be.greaterThan(expectedFirstVisibleIndex); - expect(comboBox.$.overlay._selector.lastVisibleIndex).to.be.lessThan(targetItemIndex + 1); + expect(getViewportItems(comboBox)[0].index).to.be.greaterThan(expectedFirstVisibleIndex); + expect(getViewportItems(comboBox).pop().index).to.be.lessThan(targetItemIndex + 1); }); it('should reset to 0 when filter applied and filtered items size more than page size', () => { @@ -1028,7 +1063,7 @@ describe('lazy loading', () => { comboBox.opened = true; comboBox.$.overlay._scrollIntoView(500); comboBox.filter = '1'; - expect(comboBox.$.overlay._selector.firstVisibleIndex).to.be.equal(0); + expect(getViewportItems(comboBox)[0].index).to.be.equal(0); }); // Verifies https://github.com/vaadin/vaadin-combo-box/issues/957 @@ -1068,7 +1103,7 @@ describe('lazy loading', () => { await nextFrame(); flush(); - const lastVisibleIndex = comboBox.$.overlay._selector.lastVisibleIndex; + const lastVisibleIndex = getViewportItems(comboBox).pop().index; // Check if the next few items after the last visible item are not empty for (let nextIndexIncrement = 0; nextIndexIncrement < 5; nextIndexIncrement++) { const lastItem = comboBox.filteredItems[lastVisibleIndex + nextIndexIncrement]; diff --git a/packages/vaadin-combo-box/test/lit.test.js b/packages/vaadin-combo-box/test/lit.test.js index 68b61d3d26..d584344c5c 100644 --- a/packages/vaadin-combo-box/test/lit.test.js +++ b/packages/vaadin-combo-box/test/lit.test.js @@ -2,15 +2,12 @@ import { expect } from '@esm-bundle/chai'; import { fixtureSync } from '@vaadin/testing-helpers'; import { html, render } from 'lit'; import '../vaadin-combo-box.js'; +import { getViewportItems } from './helpers.js'; describe('lit', () => { describe('renderer', () => { let comboBox; - function getFirstItem() { - return comboBox.$.overlay._selector.querySelector('vaadin-combo-box-item'); - } - beforeEach(() => { comboBox = fixtureSync(``); @@ -27,7 +24,7 @@ describe('lit', () => { it('should render the content', () => { comboBox.opened = true; - expect(getFirstItem().$.content.textContent).to.equal('value-0'); + expect(getViewportItems(comboBox)[0].$.content.textContent).to.equal('value-0'); }); it('should render new content after assigning a new renderer', () => { @@ -35,7 +32,7 @@ describe('lit', () => { comboBox.renderer = (root, _, { index }) => { render(html`new-${index}`, root); }; - expect(getFirstItem().$.content.textContent).to.equal('new-0'); + expect(getViewportItems(comboBox)[0].$.content.textContent).to.equal('new-0'); }); }); }); diff --git a/packages/vaadin-combo-box/test/object-values.test.js b/packages/vaadin-combo-box/test/object-values.test.js index c1e0b88c68..e4a2bb3193 100644 --- a/packages/vaadin-combo-box/test/object-values.test.js +++ b/packages/vaadin-combo-box/test/object-values.test.js @@ -1,6 +1,7 @@ import { expect } from '@esm-bundle/chai'; import sinon from 'sinon'; import { aTimeout, fixtureSync, fire } from '@vaadin/testing-helpers'; +import { getViewportItems } from './helpers'; import './not-animated-styles.js'; import '../vaadin-combo-box.js'; @@ -83,9 +84,10 @@ describe('object values', () => { selectItem(0); comboBox.itemLabelPath = 'custom'; + comboBox.opened = true; expect(comboBox.inputElement.value).to.eql('bazs'); - expect(comboBox.$.overlay._selector.querySelector('vaadin-combo-box-item').label).to.eql('bazs'); + expect(getViewportItems(comboBox)[0].label).to.eql('bazs'); }); it('should use toString if default label and value paths are not found', () => { diff --git a/packages/vaadin-combo-box/test/overlay-position.test.js b/packages/vaadin-combo-box/test/overlay-position.test.js index 7eae95bbdc..c10db4b863 100644 --- a/packages/vaadin-combo-box/test/overlay-position.test.js +++ b/packages/vaadin-combo-box/test/overlay-position.test.js @@ -1,5 +1,4 @@ import { expect } from '@esm-bundle/chai'; -import sinon from 'sinon'; import { aTimeout, fixtureSync, isIOS, fire } from '@vaadin/testing-helpers'; import { PolymerElement, html } from '@polymer/polymer/polymer-element.js'; import { makeItems } from './helpers.js'; @@ -93,34 +92,6 @@ describe('overlay', () => { expect(dropContentRect().top).to.be.closeTo(inputContentRect().bottom, 1); }); - it('should notify resize on items change', () => { - comboBox.opened = true; - const spy = sinon.spy(); - comboBox.$.overlay.$.dropdown.notifyResize = spy; - comboBox.items = [1, 2, 3]; - comboBox.__repositionOverlayDebouncer.flush(); - expect(spy.called).to.be.true; - }); - - it('should not notify resize when not opened', () => { - comboBox.open(); - comboBox.close(); - - const spy = sinon.spy(); - comboBox.$.overlay.$.dropdown.notifyResize = spy; - comboBox.items = [1, 2, 3]; - expect(spy.called).to.be.false; - }); - - it('should reposition on scroll', () => { - comboBox.opened = true; - comboBox.$.overlay.updateViewportBoundaries = sinon.spy(); - - fire(document, 'scroll'); - - expect(comboBox.$.overlay.updateViewportBoundaries.callCount).to.eql(1); - }); - it('should be aligned with input container', () => { comboBox.open(); diff --git a/packages/vaadin-combo-box/test/selecting-items.test.js b/packages/vaadin-combo-box/test/selecting-items.test.js index 13db0dc335..286261db86 100644 --- a/packages/vaadin-combo-box/test/selecting-items.test.js +++ b/packages/vaadin-combo-box/test/selecting-items.test.js @@ -4,6 +4,7 @@ import { aTimeout, click, fixtureSync, fire } from '@vaadin/testing-helpers'; import { flush } from '@polymer/polymer/lib/utils/flush.js'; import './not-animated-styles.js'; import '../vaadin-combo-box.js'; +import { scrollToIndex } from './helpers.js'; describe('selecting items', () => { let comboBox; @@ -74,7 +75,7 @@ describe('selecting items', () => { // Scroll start could delay, for example, with full SD polyfill comboBox.$.overlay._scroller.addEventListener('scroll', listener); - comboBox.$.overlay._selector.scrollToIndex(20); + scrollToIndex(comboBox, 20); }); it('should select by using JS api', () => { @@ -150,6 +151,7 @@ describe('selecting items', () => { if (comboBox.value === 'foo') { comboBox.value = 'bar'; setTimeout(() => { + comboBox.open(); expect(comboBox.value).to.eql('bar'); expect(comboBox.selectedItem).to.eql('bar'); expect(items[0].selected).to.be.false; diff --git a/packages/vaadin-combo-box/theme/lumo/vaadin-combo-box-dropdown-styles.js b/packages/vaadin-combo-box/theme/lumo/vaadin-combo-box-dropdown-styles.js index 3e7db64f3d..7ed5c7dc7a 100644 --- a/packages/vaadin-combo-box/theme/lumo/vaadin-combo-box-dropdown-styles.js +++ b/packages/vaadin-combo-box/theme/lumo/vaadin-combo-box-dropdown-styles.js @@ -13,12 +13,9 @@ registerStyles( } :host { - /* TODO: using a legacy mixin (unsupported) */ - --iron-list-items-container: { - border-width: var(--lumo-space-xs); - border-style: solid; - border-color: transparent; - } + --_vaadin-combo-box-items-container-border-width: var(--lumo-space-xs); + --_vaadin-combo-box-items-container-border-style: solid; + --_vaadin-combo-box-items-container-border-color: transparent; } /* Loading state */ diff --git a/packages/vaadin-combo-box/theme/material/vaadin-combo-box-dropdown-styles.js b/packages/vaadin-combo-box/theme/material/vaadin-combo-box-dropdown-styles.js index cbaf146f4b..7bdb9968c5 100644 --- a/packages/vaadin-combo-box/theme/material/vaadin-combo-box-dropdown-styles.js +++ b/packages/vaadin-combo-box/theme/material/vaadin-combo-box-dropdown-styles.js @@ -6,12 +6,9 @@ registerStyles( 'vaadin-combo-box-overlay', css` :host { - /* TODO using a legacy mixin (unsupported) */ - --iron-list-items-container: { - border-width: 8px 0; - border-style: solid; - border-color: transparent; - } + --_vaadin-combo-box-items-container-border-width: 8px 0; + --_vaadin-combo-box-items-container-border-style: solid; + --_vaadin-combo-box-items-container-border-color: transparent; } [part='overlay'] { diff --git a/yarn.lock b/yarn.lock index 2bd09be1a1..6bfa93a418 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1407,16 +1407,6 @@ "@polymer/iron-validatable-behavior" "^3.0.0-pre.26" "@polymer/polymer" "^3.0.0" -"@polymer/iron-list@^3.0.0": - version "3.1.0" - resolved "https://registry.yarnpkg.com/@polymer/iron-list/-/iron-list-3.1.0.tgz#9b525249a90a53b6ae249330640b54b12141202a" - integrity sha512-Eiv6xd3h3oPmn8SXFntXVfC3ZnegH+KHAxiKLKcOASFSRY3mHnr2AdcnExUJ9ItoCMA5UzKaM/0U22eWzGERtA== - dependencies: - "@polymer/iron-a11y-keys-behavior" "^3.0.0-pre.26" - "@polymer/iron-resizable-behavior" "^3.0.0-pre.26" - "@polymer/iron-scroll-target-behavior" "^3.0.0-pre.26" - "@polymer/polymer" "^3.0.0" - "@polymer/iron-location@^3.0.0-pre.26": version "3.0.2" resolved "https://registry.yarnpkg.com/@polymer/iron-location/-/iron-location-3.0.2.tgz#7ca2df089c57deb8a6f54f5cc3722af0ff871441"