Skip to content

Commit

Permalink
fix: mouse scroll doesn't fetch new items (#962)
Browse files Browse the repository at this point in the history
The function _scrollIntoView(index) is used instead of scrollToIndex(index) to provide more accurate scroll while restoring the old position in unknown size lazy loading, and ensure the next page after target index is also loaded and rendered.

Fixes #957
  • Loading branch information
mshabarov committed Oct 28, 2020
1 parent 9b8480d commit 7707868
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 5 deletions.
2 changes: 1 addition & 1 deletion src/vaadin-combo-box-dropdown-wrapper.html
Expand Up @@ -223,7 +223,7 @@
_restoreScrollerPosition(items) {
if (this._isNotEmpty(items) && this._selector && this._oldScrollerPosition !== 0) {
// new items size might be less than old scrolling position
this._selector.scrollToIndex(Math.min(items.length - 1, this._oldScrollerPosition));
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)
Expand Down
60 changes: 56 additions & 4 deletions test/lazy-loading.html
Expand Up @@ -878,13 +878,20 @@
const allItems = Array(...new Array(ESTIMATED_SIZE)).map((_, i) => `item ${i}`);

it('should restore the scroll position after size update', () => {
const targetItemIndex = 75;
comboBox.dataProvider = getDataProvider(allItems);
comboBox.opened = true;
comboBox.$.overlay._scrollIntoView(75);
comboBox.$.overlay._scrollIntoView(targetItemIndex);
comboBox.size = 300;
// verify whether the scroller not jumped to 0 pos and restored
// approx between 1 and 2 page (exact value may vary depending of window size)
expect(comboBox.$.overlay._selector.firstVisibleIndex).to.be.greaterThan(comboBox.pageSize).and.lessThan(comboBox.pageSize * 2);
// verify whether the scroller not jumped to 0 pos and restored properly,
// having the item with 'targetItemIndex' in the bottom
// (exact visible items may vary depending of window size),
// and sometimes the 'ironList.scrollToIndex' does not point
// 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);
});

it('should reset to 0 when filter applied and filtered items size more than page size', () => {
Expand All @@ -895,6 +902,51 @@
expect(comboBox.$.overlay._selector.firstVisibleIndex).to.be.equal(0);
});

// Verifies https://github.com/vaadin/vaadin-combo-box/issues/957
it('should fetch the items after scrolling to the bottom with scrollbar', done => {
const REAL_SIZE = 1234;
let ESTIMATED_SIZE = 200;
const allItems = Array(...new Array(REAL_SIZE)).map((_, i) => `item ${i}`);

// DataProvider for unknown size lazy loading
const getDataProvider = (allItems) => (params, callback) => {
const filteredItems = allItems.filter(item => item.indexOf(params.filter) > -1);
const offset = params.page * params.pageSize;
const end = offset + params.pageSize;

dataProviderItems = filteredItems.slice(offset, end);

if (dataProviderItems.size === 0) {
ESTIMATED_SIZE = offset;
} else if (dataProviderItems.size < params.pageSize) {
ESTIMATED_SIZE = offset + dataProviderItems.size;
} else if (end > ESTIMATED_SIZE - params.pageSize) {
ESTIMATED_SIZE += 200;
}
callback(dataProviderItems, ESTIMATED_SIZE);
};

comboBox.dataProvider = getDataProvider(allItems);
comboBox.opened = true;
Polymer.flush();

// Scroll to the end, as though if we drag the scrollbar and move it
// to the bottom
const scrollHeight = comboBox.$.overlay._selector._scrollHeight;
comboBox.$.overlay._scroller.scrollTop += scrollHeight;

// Flush the pending changes after the scrolling
flush(() => {
const lastVisibleIndex = comboBox.$.overlay._selector.lastVisibleIndex;
// 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];
expect(lastItem instanceof Vaadin.ComboBoxPlaceholder).is.false;
}
done();
});
});

it('should not show the loading when exact size is suddenly reached in the middle of requested range', () => {
const REAL_SIZE = 294;
const ESTIMATED_SIZE = 400;
Expand Down

0 comments on commit 7707868

Please sign in to comment.