Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prepend is ignored if page size is less than prefetch index #8

Closed
wants to merge 3 commits into from

Conversation

dinaraparanid
Copy link

Fixes #7

Comment on lines 477 to 479
reverse ? index >= itemCount - fetchIndex : index <= fetchIndex;
final nearBottom =
reverse ? index == fetchIndex : index == itemCount - fetchIndex;
reverse ? index <= fetchIndex : index >= itemCount - fetchIndex;
Copy link
Owner

Choose a reason for hiding this comment

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

Hey, thank you for the PR but this will also trigger notification for all the items after the prefetch item. A better solution would be to count the items of last or first page based on reverse and then match it with
min(fetchIndex, length).

Copy link
Author

Choose a reason for hiding this comment

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

Hello, thank you for reply! It seems that checking exact item index with == will still not work even for min(fetchIndex, length). There is an example that will break it:

  1. Suppose id of message is the key of page (e.g. start page from message with id = ?)
  2. Initial key = 20 (message id), page size = 45, prefetchIndex = 22
  3. We cannot use == with min(fetchIndex, len) since we have not reached item with index 22 (because we are loaded page from item=20, but it has initial index = 0, meaning that we have to scroll down to item 20+22=42 to trigger prepend operation)

Solution seems to be straightforward: in PageFetcher we disallow to trigger multiple _doLoad calls (we cannot do it on Ui, because of WidgetsBinding.instance.addPostFrameCallback { pager.load(...) }), which will protect pager from multiple requests of items that have passed fetchIndex

@xsahil03x
Copy link
Owner

Hey @dinaraparanid , can you try this PR #9 and check if this fixes your issue? Thanks

@dinaraparanid
Copy link
Author

Hello! PR #9 fixes the main issue, but there is still an issue with preloading, if fetch index is far from visible zone.

  1. Suppose that page key is the id of message [each message has id = text = ordering number, e.g. Message(id=1, text=1)]
  2. We load the page with key = 20 (id of first visible message), pageSize = 45, prefetchIndex = 22.
  3. It is expected that pager will trigger previous page (with key = 1)
  4. We have loaded full page (from 20 to 65), but the prefetchIndex is far from visible zone, we have to scroll to item 20+22 = 42 to trigger prepend. In that case prefetchIndex < itemsCount, meaning that fix with if (prefetchIndex > itemCount) { ... } will be ignored
Снимок экрана 2024-11-30 в 11 27 22 Снимок экрана 2024-11-30 в 11 27 22 Снимок экрана 2024-11-30 в 11 27 46

@xsahil03x
Copy link
Owner

@dinaraparanid I think i understood your issue. THis will only happen for the first page right? Maybe we can add a additional check for this.

if (pages.length == 1) i.e it's the first page, we set the prefetchIndex = 0

What do you think?

@dinaraparanid
Copy link
Author

Yeah, it will fix the issue, thanks!

@xsahil03x
Copy link
Owner

xsahil03x commented Dec 1, 2024

Yeah, it will fix the issue, thanks!

Hey, I just released a new v0.2.0 with the fix. Thanks

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.

Prepend is ignored if page size is less than prefetch index
2 participants