fix: update all rendered rows on flat size change from page response #7101
Conversation
| @@ -247,6 +255,15 @@ describe('data provider', () => { | |||
| { path: 'name', direction: 'asc' }, | |||
| ]); | |||
| }); | |||
|
|
|||
| it('should not request again when resolving with an empty array', async () => { | |||
There was a problem hiding this comment.
One of my earlier fix attempts passed all other tests but it caused an infinite data request loop when the callback was resolved with an empty array, so I added this test to make sure some future change will not cause this issue.
There was a problem hiding this comment.
Testing with the same TreeGridScrollToPage mentioned in the ticket, I still get some odd behavior when expanding all items:
It now loads more second level items (Dad *), but not the children for all of them, and doesn't show them as expanded. It breaks similarly if I increase the page size to 10 again. It works fine with page size 10 if I don't use the changes from this PR.
There was a problem hiding this comment.
It turns out that applying this fix, or alternatively, reverting the original regression, were both feasible options up until this change in the Flow Grid connector. Investigating the reason...
There was a problem hiding this comment.
I did some investigation and found that Sascha's issue is actually a side effect of not just one but two changes:
- fix: prevent grid from requesting too many sub-level items flow-components#5916 (mentioned by Tomi)
- fix: reset fetch range when requested page is out of visible range flow-components#5897
A response with children for Granddad 0 changes the flat size which triggers the new logic added in this PR. The response contains only the first page while the virtualizer range also includes the second page. Therefore, the new logic immediately requests the second page for the sub-level of Granddad 0. The requested page gets into parentRequestDebouncer where it remains scheduled for a while. However, by the time the debouncer flushes, the page ends up far out of the viewport because Dad 0/0 is also expanded and by this time it has already received the sub-items (Flow sends the data for it in advance, in the same round-trip as the response for Granddad 0). Since the page is now out of the rendered range, the request range is reset to only request that second page. This causes Flow to clear the first page which doesn't get re-requested later, possibly due to #7097 (comment).
There was a problem hiding this comment.
This explains why moving the new logic to _debouncerApplyCachedData deals with the issue. It delays the new logic until all requests are resolved, which helps avoid that request for the second page.
There was a problem hiding this comment.
I've moved the logic to the debouncer. Could you please confirm that the issue is no longer reproducible, @sissbruecker?
There was a problem hiding this comment.
I can still reproduce it when:
- Scroll to end
- Expand all
- Scroll up
Also seems to happen sometimes when expanding all without scrolling down, but I couldn't identify a pattern.
Bildschirmaufnahme.2024-03-21.um.09.32.40.mov
There was a problem hiding this comment.
This seems to be caused by the key mapper issue again, rather than the current regression. I'm now inclined to wait until the key mapper issue is resolved, as it's currently causing interference in many scenarios.
There was a problem hiding this comment.
I can still reproduce it when:
Now that vaadin/flow#19006 has been fixed, I cannot reproduce this issue anymore. Can you please confirm, @sissbruecker?
There was a problem hiding this comment.
@vursen I also verified that the issue no longer reproduces after vaadin/flow#19006 is fixed.
|
yuriy-fix
changed the title
vursen
force-pushed
the
fix/grid/small-page-size-missing-items
branch
4 times, most recently
from
20d8e44
to
d97a205
Compare
vursen
changed the title
vursen
changed the title
vursen
changed the title
vursen
changed the title
This reverts commit d13cada.
vursen
force-pushed
the
fix/grid/small-page-size-missing-items
branch
from
f2a6548
to
a176c08
Compare
|
vursen
changed the title
|
This ticket/PR has been released with Vaadin 24.4.0.alpha24 and is also targeting the upcoming stable 24.4.0 version. |
Description
The PR causes grid to update all rendered rows once it receives a page response that affects the flat size. This guarantees that all pages associated with the rendered rows are loaded.
Warning
It's been decided to wait until vaadin/flow#19006 is fixed, as it's currently causing interference in many scenarios related to the current regression, making it difficult to distinguish the causes.
Fixes #7097
Type of change
Bugfix