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

SliverInfiniteList sometimes requires an extra pull to load more items #41

Closed
b-nugent opened this issue Oct 11, 2022 · 5 comments · Fixed by #42
Closed

SliverInfiniteList sometimes requires an extra pull to load more items #41

b-nugent opened this issue Oct 11, 2022 · 5 comments · Fixed by #42
Assignees
Labels
bug Something isn't working as expected

Comments

@b-nugent
Copy link

Describe the bug
My team had a SliverInfiniteList created by @alestiago (thanks Alejandro!!) before it was provided through this package in version 0.5.0. We attempted to swap in the SliverInfiniteList and noticed that by default the SliverInfiniteList sometimes requires an additional pull-up in order to load and display new items when the list reaches the end of what is loaded. We believe this may be due to how the effectiveItemCount is calculated, how the end of the list is determined, and how that results in the display of the loadingBuilder.

To Reproduce
Using a SliverInfiniteList that is the last sliver in a CustomScrollView, which has a loadingBuilder and the ability to load more list items:

  • Quickly scroll through the list
    • The list reaches the end of displayable items and stops scrolling
  • Pull up to attempt to keep scrolling
    • While pulling up, either the loading indicator displays or the list over-scrolls when using iOS BouncingScrollPhysics and shows no indicator
    • New list items are eventually loaded in and scrolling can continue

Expected behavior
What we expected (which reflects the behavior of the custom SliverInfiniteList made by VGV for our team) is that as the user scrolls down the list and reaches the end of the loaded data, the loading indicator (supplied by the loadingBuilder) is displayed at the very bottom of the list, and new list items begin to load. This causes the overall scrolling and loading to flow more naturally and reduces the need for the user to pull up again when the list runs out of displayable entries.

Additional context
Let us know if you'd like videos showcasing the difference in behavior, we're happy to send them to the team if needed.

@felangel felangel added the bug Something isn't working as expected label Oct 11, 2022
@felangel felangel moved this to Needs Triage in VGV Open Source 🦄 🧙🌟 Oct 11, 2022
@felangel felangel moved this from Needs Triage to Todo in VGV Open Source 🦄 🧙🌟 Oct 11, 2022
@renancaraujo
Copy link
Contributor

Hey @b-nugent, thanks for taking the time to report this.

To add more context, this behavior is also observed in the nonsliver implementation before 0.5.0.

LMK if you want to tackle it (based on that impl) otherwise we add this to the queue of stuff to do.

@alestiago
Copy link
Contributor

@b-nugent thanks for the lovely comment and for opening the issue 🥹💙

@renancaraujo worked on the SliverList implementation. So I'm not sure straight away where this can be coming from.

I was wondering if you have a minimal reproductive code you could share to have a look at 👀

@b-nugent
Copy link
Author

Hey @renancaraujo and @alestiago! Here is the implementation that we're using:

class InfiniteSliverList extends StatefulWidget {
  const InfiniteSliverList({
    super.key,
    required this.onFetchData,
    required this.itemCount,
    required this.itemBuilder,
    required this.hasReachedMax,
    required this.isLoading,
  });
  
  final VoidCallback? onFetchData;
  final bool isLoading;
  final bool hasReachedMax;
  final int itemCount;
  final NullableIndexedWidgetBuilder itemBuilder;

  @override
  State<InfiniteSliverList> createState() =>
      _InfiniteSliverListState();
}

class _InfiniteSliverListState extends State<InfiniteSliverList> {
  @override
  Widget build(BuildContext context) {
    final childCount = widget.itemCount + (widget.hasReachedMax ? 0 : 1);

    return SliverList(
      delegate: SliverChildBuilderDelegate(
        childCount: childCount,
            (context, index) {
          final reachedEnd = index == childCount - 1;
          if (reachedEnd && !widget.hasReachedMax) {
            if (!widget.isLoading) {
              WidgetsBinding.instance.addPostFrameCallback((_) {
                widget.onFetchData?.call();
              });
              return const SizedBox.shrink();
            } else {
              return const Center(
                child: Padding(
                  padding: EdgeInsets.only(top: 16, bottom: 32),
                  child: CircularProgressIndicator(
                    color: Colors.grey,
                  ),
                ),
              );
            }
          }

          return widget.itemBuilder(context, index);
        },
      ),
    );
  }
}

This list brings up the loading indicator at the bottom of the list as the user is scrolling and more list items need to be loaded in. We swapped in the SliverInfiniteList with the same values for onFetchData, itemCount, itemBuilder, hasReachedMax, and isLoading, but found that the loading indicator doesn't show when the last of the list items is scrolled to. Instead, the user has to pull up a second time to see the loading indicator and bring up more items.

@renancaraujo renancaraujo moved this from Todo to In Progress in VGV Open Source 🦄 🧙🌟 Oct 13, 2022
@renancaraujo renancaraujo self-assigned this Oct 13, 2022
@renancaraujo
Copy link
Contributor

After investigating, found that maxScrollExtent is not always up to date. We cannot rely on ScrollPosition anymore since it is not trustworthy for our case. The technique of refetching upon the last item build is quite clever since we can rely on the sliver list dynamics to trigger refetch.

@renancaraujo renancaraujo moved this from In Progress to In Review in VGV Open Source 🦄 🧙🌟 Oct 13, 2022
@renancaraujo
Copy link
Contributor

renancaraujo commented Oct 13, 2022

Added a pr for this on #42
I would love some feedback @b-nugent

Repository owner moved this from In Review to Done in VGV Open Source 🦄 🧙🌟 Oct 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants