letsar / flutter_staggered_grid_view

A Flutter staggered grid view
MIT License
3.14k stars 510 forks source link

Scrolling issue with two SliverMasonryGrid in CustomScrollView #338

Open kawi15 opened 8 months ago

kawi15 commented 8 months ago

Hi,

I tried to implement MasonryGridView with separator between items. I used for this CustomScrollView and put two SliverMasonryGrid.count working on the same data inside separated by my custom divider. Unfortunately when scrolling through second SliverMasonryGrid scroll is coming back to top of the first list.

As I tested this with SliverGrid.count and then it worked properly, this is looking like a bug to me.

Issue mentioned #335 and here suggestion for creating a solution of what I wanted to achieve #337

and here is my code:

Expanded(
  child: CustomScrollView(
    controller: widget.scrollController,
    physics: const BouncingScrollPhysics(
        parent: AlwaysScrollableScrollPhysics()),
    shrinkWrap: true,
    slivers: [
      SliverPadding(
        padding: EdgeInsets.symmetric(horizontal: size.width * 0.03)
        sliver: SliverMasonryGrid.count(
            crossAxisCount: 2,
            mainAxisSpacing: size.width * 0.03,
            crossAxisSpacing: size.width * 0.03,
            childCount: 2,
            itemBuilder: (context, index) {
              return CustomItem(
                content: widget.dataList[index].content
              );
            }
        ),
      ),
      SliverToBoxAdapter(
        child: Container(...),
      ),
      SliverPadding(
        padding: EdgeInsets.symmetric(horizontal: size.width * 0.03)
        sliver: SliverMasonryGrid.count(
            crossAxisCount: 2,
            mainAxisSpacing: size.width * 0.03,
            crossAxisSpacing: size.width * 0.03,
            childCount: widget.dataList.length - 2,
            itemBuilder: (context, index) {
              return CustomItem(
                content: widget.dataList[index + 2].content
              );
            }
        ),
      ),
    ],
  ),
)

Any chance to fix this bug soon?

letsar commented 8 months ago

Hi, I can reproduce. It happens when there is something after a SliverMasonryGrid. I'll try to fix this. In the meantime, if the first grid is finite, maybe you use a MasonryGridView instead of a SliverMasonryGrid. Something like this:

Column(
  children: [
    MasonryGridView(..),
    Separator(...),
    Expanded(
      child: MasonryGridView(...),
    ),
  ],
);
letsar commented 8 months ago

For the issue with the multi slivers, I'm not sure this is something simple to fix, at least not completely.

The issue we have here is because of this part of the code:

          // We ran out of children before reaching the scroll offset.
          // We must inform our parent that this sliver cannot fulfill
          // its contract and that we need a scroll offset correction.
          geometry = SliverGeometry(
            scrollOffsetCorrection: -scrollOffset,
          );
          return;

To fix this, I probably need to fix the condition in the enclosing while. But In that case, we have an issue when we try to build this component from the end. The current algorithm is not fit for this. I would need to find another way to be able to correctly position the items before the last one, and since the offset of each child depends on the ones before, it's not trivial.

I will need to think about it. I hope that my solution above can help you in the meantime.

kawi15 commented 8 months ago

Thank you for quick response.

Unfortunately your solution is not suiting for me as I want both these list with separator to be scrolled as one list and also above this I have two horizontal lists which I also want to be scrolled as one list with the masonry ones. So I think CustomScrollView is necessary here.

I hope you will find good and easy solution.

kawi15 commented 8 months ago

I don't know, but maybe this will be helpful for you. I tried another package - https://pub.dev/packages/masonry_grid and with it it's working as it should. Maybe you will find there something what can help you. I still waiting for you to find a good solution as mentioned package have some performance issues.

letsar commented 8 months ago

Hi, I think I fixed the issue, can you test on this branch to see if it's also ok for you: https://github.com/letsar/flutter_staggered_grid_view/tree/feature/masonry_cache ?

kawi15 commented 8 months ago

Hi, I found some bug around sixth index on second list. I will show you it on video recording as it is hard to explain. https://drive.google.com/file/d/1TMF-IDR07D5pNq6YnO19a8fvE4vFgLeK/view?usp=sharing.

Except that it is working really good.

letsar commented 8 months ago

Thanks for trying this branch. I can try to pinpoint the issue, but if you have a reproducible code it would help me a lot.

kawi15 commented 8 months ago

I created something like this for quick reproducible code, but in this case bug happening on last indexes when scrolling back, so kinda different than on video I posted before, maybe because there was more data?

I hope this will be good.

https://drive.google.com/file/d/1TP3QSUyVRsnrOXylbqKYJoFQfbJd82_1/view?usp=sharing

Expanded(
            child: CustomScrollView(
              shrinkWrap: true,
              slivers: [
                SliverPadding(
                  padding: EdgeInsets.symmetric(horizontal: 8) + const EdgeInsets.only(bottom: 8),
                  sliver: SliverMasonryGrid.count(
                      crossAxisCount: 2,
                      mainAxisSpacing: 8,
                      crossAxisSpacing: 8,
                      childCount: 2,
                      itemBuilder: (context, index) {
                        return Container(
                          height: 200,
                          child: Image.network('https://upload.wikimedia.org/wikipedia/commons/thumb/3/34/Hydrochoeris_hydrochaeris_in_Brazil_in_Petr%C3%B3polis%2C_Rio_de_Janeiro%2C_Brazil_09.jpg/1200px-Hydrochoeris_hydrochaeris_in_Brazil_in_Petr%C3%B3polis%2C_Rio_de_Janeiro%2C_Brazil_09.jpg'),
                        );
                      }
                  ),
                ),
                SliverToBoxAdapter(
                  child: Container(
                    margin: EdgeInsets.only(
                      bottom: size.height * 0.02,
                      top: size.height * 0.02,
                    ),
                    child: Stack(
                      children: [
                        Divider(
                            height: 37,
                            thickness: 1,
                            indent: 8,
                            endIndent: 8,
                            color: const Color(0xFF98B9DC)
                        ),
                        Container(
                            height: 37,
                            padding: const EdgeInsets.symmetric(
                                horizontal: 16,
                                vertical: 8
                            ),
                            alignment: Alignment.center,
                            margin: EdgeInsets.symmetric(horizontal: 32),
                            decoration: BoxDecoration(
                                borderRadius: BorderRadius.circular(24),
                                color: const Color(0xFFE4ECF6)

                            ),
                            child: Text(
                              'wydarzenia w dalszej odległości',
                              style: TextStyle(
                                  fontSize: 14,
                                  color:  const Color(0xFF405978)
                              ),
                            )
                        )
                      ],
                    ),
                  ),
                ),
                SliverPadding(
                  padding: EdgeInsets.symmetric(horizontal: 8) + const EdgeInsets.only(bottom: 8),
                  sliver: SliverMasonryGrid.count(
                      crossAxisCount: 2,
                      mainAxisSpacing: 8,
                      crossAxisSpacing: 8,
                      childCount: 20,
                      itemBuilder: (context, index) {
                        return Container(
                          height: 250,
                          child: Image.network(
                              index % 2 == 0
                                  ? 'https://www.rainforest-alliance.org/wp-content/uploads/2021/06/capybara-square-1.jpg.optimal.jpg'
                                  : 'https://hips.hearstapps.com/hmg-prod/images/dog-puppy-on-garden-royalty-free-image-1586966191.jpg?crop=0.752xw:1.00xh;0.175xw,0&resize=1200:*'
                          ),
                        );
                      }
                  ),
                ),
              ],
            ),
          )
kawi15 commented 7 months ago

Any proggress/update?

letsar commented 7 months ago

Hi, I can't reproduce with the above example. I'll try to find a layout which causes this, but it won't be easy

letsar commented 7 months ago

Can you send me the actual size of the CustomScrollView in the first video, along with the heights of each elements in the Sliver so that I can try to reproduce within the same conditions?

kawi15 commented 7 months ago

It's dynamic, when I scroll to the end of the list it's loading more from backend. Initially it's 15 elements. Height of each element in sliver is also dynamic, depending on how long title is.

Here is how it looks, but there is a lot of different parameters that are hard to reproduce.


SliverPadding(
                  padding: EdgeInsets.symmetric(horizontal: size.width * 0.03) + const EdgeInsets.only(bottom: 8),
                  sliver: SliverMasonryGrid.count(
                      crossAxisCount: 2,
                      mainAxisSpacing: size.width * 0.03,
                      crossAxisSpacing: size.width * 0.03,
                      childCount: widget.outOfLocationIndex,
                      itemBuilder: (context, index) {
                        return GestureDetector(
                          onTap: () async {
                            if (canClick) {
                              try {
                                widget.onClick(widget.dataList[index]);
                              } catch (e, stack) {
                                printl(stack);
                                errorMsg(e, context);
                                await Sentry.captureException(e,
                                    stackTrace: stack);
                              }
                            }
                            printl(
                                "DEBUG widget.dataList 346 render widget 88774982");
                          },
                          child: NetflixEventCard(
                            index: index,
                            query: widget.query,
                            eventName: widget.dataList[index].name,
                            eventDate: widget.dataList[index].event_date,
                            partySpotName: widget.dataList[index].party_spot != null
                                ? widget.dataList[index].party_spot!.name ?? ''
                                : '',
                            address: widget.dataList[index].address == null
                                ? ""
                                : widget.dataList[index].address!.city,
                            images: widget.dataList[index].images
                          ),
                        );
                      }
                  ),
                ),
               SliverToBoxAdapter(
                    child: Container(
                      margin: EdgeInsets.only(
                        bottom: size.height * 0.02,
                        top: size.height * 0.02,
                      ),
                      child: Stack(
                        children: [
                          Divider(
                              height: 37,
                              thickness: 1,
                              indent: size.width * 0.03,
                              endIndent: size.width * 0.03,
                              color: const Color(0xFF98B9DC)
                          ),
                          Container(
                              height: 37,
                              padding: const EdgeInsets.symmetric(
                                  horizontal: 16,
                                  vertical: 8
                              ),
                              alignment: Alignment.center,
                              margin: EdgeInsets.symmetric(horizontal: size.width * 0.12),
                              decoration: BoxDecoration(
                                  borderRadius: BorderRadius.circular(24),
                                  color: Theme.of(context).brightness == Brightness.light
                                      ? const Color(0xFFE4ECF6)
                                      : const Color(0xFF374352)
                              ),
                              child: Text(
                                'wydarzenia w dalszej odległości',
                                style: TextStyle(
                                    fontSize: 14,
                                    fontWeight: FontWeight.w700,
                                    color: Theme.of(context).brightness == Brightness.light
                                        ? const Color(0xFF405978)
                                        : const Color(0xFFDBE3EE)
                                ),
                              )
                          )
                        ],
                      ),
                    ),
                ),
               SliverPadding(
                    padding: EdgeInsets.symmetric(horizontal: size.width * 0.03) + const EdgeInsets.only(bottom: 8),
                    sliver: SliverMasonryGrid.count(
                        crossAxisCount: 2,
                        mainAxisSpacing: size.width * 0.03,
                        crossAxisSpacing: size.width * 0.03,
                        childCount: widget.dataList.length - widget.outOfLocationIndex,
                        itemBuilder: (context, index) {
                          return GestureDetector(
                            onTap: () async {
                              if (canClick) {
                                try {
                                  widget.onClick(widget.dataList[index + widget.outOfLocationIndex]);
                                } catch (e, stack) {
                                  printl(stack);
                                  errorMsg(e, context);
                                  await Sentry.captureException(e,
                                      stackTrace: stack);
                                }
                              }
                              printl(
                                  "DEBUG widget.dataList 346 render widget 88774982");
                            },
                            child: NetflixEventCard(
                                index: index + widget.outOfLocationIndex,
                                query: widget.query,
                                eventName: widget.dataList[index + widget.outOfLocationIndex].name,
                                eventDate: widget.dataList[index + widget.outOfLocationIndex].event_date,
                                partySpotName: widget.dataList[index + widget.outOfLocationIndex].party_spot != null
                                    ? widget.dataList[index + widget.outOfLocationIndex].party_spot!.name ?? ''
                                    : '',
                                address: widget.dataList[index + widget.outOfLocationIndex].address == null
                                    ? ""
                                    : widget.dataList[index + widget.outOfLocationIndex].address!.city,
                                images: widget.dataList[index + widget.outOfLocationIndex].images
                            ),
                          );
                        }
                    ),
                  ),
lwj1994 commented 5 months ago

it also happened with any other sliver list.


CustomScrollView{
  // SliverMasonryGrid
  SliverMasonryGrid.count() ,
  // SliverList wrap with DecoratedSliver
  DecoratedSliver(
    SliverList.separated()
  )
}
tara-pogancev commented 1 month ago

Facing the same issue when having 2 masonry grids in the same custom scroll view. The plugin is great otherwise, hoping for a fix. 😄