Skip to content

Commit

Permalink
fix: prevent infinite loop when measuring item height (#5433) (#5483)
Browse files Browse the repository at this point in the history
Co-authored-by: Diego Cardoso <diego@vaadin.com>
  • Loading branch information
vaadin-bot and DiegoCardoso authored Feb 7, 2023
1 parent f93fe2e commit c40abf8
Show file tree
Hide file tree
Showing 4 changed files with 140 additions and 37 deletions.
34 changes: 1 addition & 33 deletions packages/component-base/src/iron-list-core.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const DEFAULT_PHYSICAL_COUNT = 3;
* If something in the scrolling engine needs to be changed
* for the virtualizer's purposes, override a function
* in virtualizer-iron-list-adapter.js instead of changing it here.
* If a function on this file is no longer needed, the code can be safely deleted.
*
* This will allow us to keep the iron-list code here as close to
* the original as possible.
Expand Down Expand Up @@ -560,39 +561,6 @@ export const ironList = {
return this._virtualStart + (this._physicalCount - this._physicalStart) + pidx;
},

/**
* Updates the height for a given set of items.
*
* @param {!Array<number>=} itemSet
*/
_updateMetrics(itemSet) {
// Make sure we distributed all the physical items
// so we can measure them.
flush();

let newPhysicalSize = 0;
let oldPhysicalSize = 0;
const prevAvgCount = this._physicalAverageCount;
const prevPhysicalAvg = this._physicalAverage;

// eslint-disable-next-line @typescript-eslint/no-unused-vars
this._iterateItems((pidx, vidx) => {
oldPhysicalSize += this._physicalSizes[pidx];
this._physicalSizes[pidx] = this._physicalItems[pidx].offsetHeight;
newPhysicalSize += this._physicalSizes[pidx];
this._physicalAverageCount += this._physicalSizes[pidx] ? 1 : 0;
}, itemSet);

this._physicalSize = this._physicalSize + newPhysicalSize - oldPhysicalSize;

// Update the average if it measured something.
if (this._physicalAverageCount !== prevAvgCount) {
this._physicalAverage = Math.round(
(prevPhysicalAvg * prevAvgCount + newPhysicalSize) / this._physicalAverageCount,
);
}
},

/**
* Updates the position of the physical items.
*/
Expand Down
50 changes: 50 additions & 0 deletions packages/component-base/src/virtualizer-iron-list-adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,56 @@ export class IronListAdapter {
});
}

/**
* Updates the height for a given set of items.
*
* @param {!Array<number>=} itemSet
*/
_updateMetrics(itemSet) {
// Make sure we distributed all the physical items
// so we can measure them.
flush();

let newPhysicalSize = 0;
let oldPhysicalSize = 0;
const prevAvgCount = this._physicalAverageCount;
const prevPhysicalAvg = this._physicalAverage;

// eslint-disable-next-line @typescript-eslint/no-unused-vars
this._iterateItems((pidx, vidx) => {
oldPhysicalSize += this._physicalSizes[pidx];
this._physicalSizes[pidx] = Math.ceil(this.__getBorderBoxHeight(this._physicalItems[pidx]));
newPhysicalSize += this._physicalSizes[pidx];
this._physicalAverageCount += this._physicalSizes[pidx] ? 1 : 0;
}, itemSet);

this._physicalSize = this._physicalSize + newPhysicalSize - oldPhysicalSize;

// Update the average if it measured something.
if (this._physicalAverageCount !== prevAvgCount) {
this._physicalAverage = Math.round(
(prevPhysicalAvg * prevAvgCount + newPhysicalSize) / this._physicalAverageCount,
);
}
}

__getBorderBoxHeight(el) {
const style = getComputedStyle(el);

const itemHeight = parseFloat(style.height) || 0;

if (style.boxSizing === 'border-box') {
return itemHeight;
}

const paddingBottom = parseFloat(style.paddingBottom) || 0;
const paddingTop = parseFloat(style.paddingTop) || 0;
const borderBottomWidth = parseFloat(style.borderBottomWidth) || 0;
const borderTopWidth = parseFloat(style.borderTopWidth) || 0;

return itemHeight + paddingBottom + paddingTop + borderBottomWidth + borderTopWidth;
}

__updateElement(el, index, forceSameIndexUpdates) {
// Clean up temporary placeholder sizing
if (el.style.paddingTop) {
Expand Down
85 changes: 85 additions & 0 deletions packages/component-base/test/virtualizer-item-height.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,88 @@ describe('virtualizer - item height', () => {
expect(firstItem.offsetHeight).to.equal(firstItemHeight);
});
});

describe('virtualizer - item height - sub-pixel', () => {
let elementsContainer;
let virtualizer;
const ITEM_HEIGHT = 30.25;

beforeEach(() => {
const fixture = fixtureSync(`
<div style="height: auto;">
<div class="scroller">
<div style="min-height: 1px;"></div>
</div>
</div>
`);
const scrollTarget = fixture.firstElementChild;
const scrollContainer = scrollTarget.firstElementChild;
elementsContainer = scrollContainer;

virtualizer = new Virtualizer({
createElements: (count) => Array.from({ length: count }, () => document.createElement('div')),
updateElement: (el, index) => {
el.style.width = '100%';
el.id = `item-${index}`;

if (el.id !== index) {
el.textContent = `item-${index}`;
el.style.height = `${ITEM_HEIGHT}px`;
}
},
scrollTarget,
scrollContainer,
});

virtualizer.size = 1;
});

it('should take sub-pixel value into account when measuring items height', () => {
const containerHeight = elementsContainer.getBoundingClientRect().height;
expect(containerHeight).to.equal(Math.ceil(ITEM_HEIGHT));
});

it('should measure height correctly if some transform is applied to virtualizer', () => {
elementsContainer.style.transform = 'scale(0.5)';

virtualizer.scrollToIndex(0);

let containerHeight = elementsContainer.offsetHeight;
expect(containerHeight).to.equal(Math.ceil(ITEM_HEIGHT));

elementsContainer.style.transform = 'scale(1.5)';

virtualizer.scrollToIndex(0);

containerHeight = elementsContainer.offsetHeight;
expect(containerHeight).to.equal(Math.ceil(ITEM_HEIGHT));
});

it('should measure item height when box-sizing content-box is used', () => {
const firstItem = elementsContainer.querySelector('#item-0');

firstItem.style.boxSizing = 'content-box';
firstItem.style.paddingBottom = '3px';
firstItem.style.borderTop = '4px solid red';
firstItem.style.borderBottom = '5px solid red';

virtualizer.scrollToIndex(0);

const containerHeight = elementsContainer.offsetHeight;
expect(containerHeight).to.equal(Math.ceil(ITEM_HEIGHT + 3 + 4 + 5));
});

it('should measure item height when box-sizing border-box is used', () => {
const firstItem = elementsContainer.querySelector('#item-0');

firstItem.style.boxSizing = 'border-box';
firstItem.style.paddingBottom = '3px';
firstItem.style.borderTop = '4px solid red';
firstItem.style.borderBottom = '5px solid red';

virtualizer.scrollToIndex(0);

const containerHeight = elementsContainer.offsetHeight;
expect(containerHeight).to.equal(Math.ceil(ITEM_HEIGHT));
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,8 @@ describe('unlimited size', () => {
const item = elementsContainer.querySelector(`#item-${virtualizer.lastVisibleIndex}`);
const itemRect = item.getBoundingClientRect();
expect(scrollTarget.getBoundingClientRect().bottom).to.be.within(
Math.round(itemRect.top),
Math.round(itemRect.bottom),
Math.floor(itemRect.top),
Math.ceil(itemRect.bottom),
);
});

Expand All @@ -198,8 +198,8 @@ describe('unlimited size', () => {
const item = elementsContainer.querySelector(`#item-${virtualizer.lastVisibleIndex}`);
const itemRect = item.getBoundingClientRect();
expect(scrollTarget.getBoundingClientRect().bottom).to.be.within(
Math.round(itemRect.top),
Math.round(itemRect.bottom),
Math.floor(itemRect.top),
Math.ceil(itemRect.bottom),
);
});
});

0 comments on commit c40abf8

Please sign in to comment.