Skip to content

fix: preserve large items when new items are added#7987

Merged
ugur-vaadin merged 3 commits into
mainfrom
fix-preserve-large-items-on-new-items
Oct 18, 2024
Merged

fix: preserve large items when new items are added#7987
ugur-vaadin merged 3 commits into
mainfrom
fix-preserve-large-items-on-new-items

Conversation

@ugur-vaadin

@ugur-vaadin ugur-vaadin commented Oct 16, 2024

Copy link
Copy Markdown
Contributor

Description

The iron list core uses the optimal physical size (_optPhysicalSize) when determining whether to increase the item pool. For the cases where some items are much larger than the average, the iron list core might not increase item pool. This can lead to the large item not being rendered.

This PR overrides the getter for _optPhysicalSize from iron list core and adds an item height buffer whenever necessary.

The calculation for the extra item buffer is based on the calculation of the regular buffer, which is _viewportHeight * (_maxPages - 1). _maxPages is 1.3 in our case. So there is a buffer with a height of _viewportHeight * 0.15 each for top and bottom. If the item heights are varied, then the extra buffer of maxItemHeight - _viewportHeight * 0.15 is added to the optimal physical size. This makes sure that there is always enough space so that the iron list creates more items in the pool.

_optPhysicalSize is private and only used when the virtualizer needs determine whether to add more items to the pool. Therefore this should not cause any changes in other calculations.

Note:
Playing with the value of _maxPages can hide the issue for certain combinations of _viewportHeight and maxItemHeight, however it will not completely fix it.

Fixes #7986

Type of change

  • Bugfix
  • Feature

Checklist

  • I have read the contribution guide: https://vaadin.com/docs/latest/contributing/pr
  • I have added a description following the guideline.
  • The issue is created in the corresponding repository and I have referenced it.
  • I have added tests to ensure my change is effective and works as intended.
  • New and existing tests are passing locally with my change.
  • I have performed self-review and corrected misspellings.

@ugur-vaadin ugur-vaadin marked this pull request as ready for review October 17, 2024 06:41
Comment thread packages/component-base/test/virtualizer-variable-row-height.test.js Outdated
@sonarqubecloud

Copy link
Copy Markdown

@ugur-vaadin ugur-vaadin removed the request for review from vursen October 18, 2024 06:53
@ugur-vaadin ugur-vaadin merged commit 632a8dd into main Oct 18, 2024
@ugur-vaadin ugur-vaadin deleted the fix-preserve-large-items-on-new-items branch October 18, 2024 06:53
@sissbruecker

Copy link
Copy Markdown
Contributor

@ugur-vaadin Should this be cherry-picked?

@ugur-vaadin

Copy link
Copy Markdown
Contributor Author

Yes, I missed the target tags for the PR. Will do the cherry-picking today.

ugur-vaadin added a commit that referenced this pull request Oct 21, 2024
* fix: preserve large items when new items are added

* fix: fix refactor typo

* test: extract initialization with large item count
ugur-vaadin added a commit that referenced this pull request Oct 21, 2024
* fix: preserve large items when new items are added

* fix: fix refactor typo

* test: extract initialization with large item count
ugur-vaadin added a commit that referenced this pull request Oct 21, 2024
* fix: preserve large items when new items are added

* fix: fix refactor typo

* test: extract initialization with large item count
ugur-vaadin added a commit that referenced this pull request Oct 21, 2024
* fix: preserve large items when new items are added

* fix: fix refactor typo

* test: extract initialization with large item count
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Vaadin Grid and Virtual List rendering issues when large content present when using AbstractBackEndDataProvider

2 participants