Skip to content

fix: Prevent refreshViewport from reloading data from data provider#24895

Merged
tltv merged 2 commits into
mainfrom
fix/refresh-viewport-no-reload
Jul 2, 2026
Merged

fix: Prevent refreshViewport from reloading data from data provider#24895
tltv merged 2 commits into
mainfrom
fix/refresh-viewport-no-reload

Conversation

@vursen

@vursen vursen commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Description

DataCommunicator#refreshViewport used to set the same flag as reset(), which made the flush re-query the item count and re-fetch the viewport range from the data provider. Now it only regenerates and resends data for the items that are already active, matching the behavior of HierarchicalDataCommunicator#refreshViewport.

Changes:

  • The former resendEntireRange flag is renamed to reloadViewportRange (it reloads the viewport range from the data provider and resends it).
  • A new resendViewportRange flag drives the resend-only path used by refreshViewport: the active range is regenerated through the data generators and resent via update.set without any data provider queries, using the same wire format as before.
  • If the viewport range changes in the same roundtrip, newly visible items are still fetched from the data provider as they were never loaded.

Type of change

  • Bugfix
  • Feature

Checklist

  • I have read the contribution guide: https://vaadin.com/docs/latest/contributing/overview
  • 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.
  • I have performed self-review and corrected misspellings.

🤖 Generated with Claude Code

vursen and others added 2 commits July 2, 2026 10:52
DataCommunicator#refreshViewport used to set the same flag as reset(),
which made the flush re-query the item count and re-fetch the viewport
range from the data provider. Now it only regenerates and resends data
for the items that are already active, matching the behavior of
HierarchicalDataCommunicator#refreshViewport.

The former resendEntireRange flag is renamed to reloadViewportRange,
and the new resend-only behavior is driven by a separate
resendViewportRange flag.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@vursen vursen changed the title fix: prevent refreshViewport from reloading data from the data provider fix: prevent refreshViewport from reloading data from data provider Jul 2, 2026
@github-actions github-actions Bot added the +0.0.1 label Jul 2, 2026
@vursen vursen marked this pull request as ready for review July 2, 2026 07:08
@sonarqubecloud

sonarqubecloud Bot commented Jul 2, 2026

Copy link
Copy Markdown

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Test Results

 1 456 files  ±0   1 456 suites  ±0   1h 26m 55s ⏱️ - 5m 5s
10 303 tests +2  10 235 ✅ +2  68 💤 ±0  0 ❌ ±0 
10 775 runs  +2  10 706 ✅ +2  69 💤 ±0  0 ❌ ±0 

Results for commit d5d10c0. ± Comparison against base commit 9977f32.

@vursen vursen requested review from mcollovati and tltv July 2, 2026 07:45
@vursen vursen changed the title fix: prevent refreshViewport from reloading data from data provider fix: Prevent refreshViewport from reloading data from data provider Jul 2, 2026

@tltv tltv left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, one case to think about with Grid is how Column.setPartNameGenerator or Column.setTooltipGenerator are affected. Looks like it could now require additional DataProvider.refreshAll() to keep it function exactly same as before. But considering how JavaDoc never said refreshViewport() to reload data, approving.

@mcollovati mcollovati removed their request for review July 2, 2026 11:32
@tltv tltv added this pull request to the merge queue Jul 2, 2026
Merged via the queue into main with commit d750ca4 Jul 2, 2026
34 checks passed
@tltv tltv deleted the fix/refresh-viewport-no-reload branch July 2, 2026 11:53
@tltv

tltv commented Jul 2, 2026

Copy link
Copy Markdown
Member

For reference regarding last review comment: Grid.Column.setPartNameGenerator calls refreshViewport().

@vursen

vursen commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

For reference regarding last review comment: Grid.Column.setPartNameGenerator calls refreshViewport().

Yes, that's actually the expected behavior. Part name and tooltip generation shouldn't trigger another data provider call if the data is already there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants