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

Scrolling issue with two SliverMasonryGrid in CustomScrollView #338

Open
kawi15 opened this issue Feb 19, 2024 · 19 comments
Open

Scrolling issue with two SliverMasonryGrid in CustomScrollView #338

kawi15 opened this issue Feb 19, 2024 · 19 comments

Comments

@kawi15
Copy link

kawi15 commented Feb 19, 2024

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
Copy link
Owner

letsar commented Feb 19, 2024

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
Copy link
Owner

letsar commented Feb 19, 2024

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
Copy link
Author

kawi15 commented Feb 20, 2024

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
Copy link
Author

kawi15 commented Feb 21, 2024

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
Copy link
Owner

letsar commented Feb 26, 2024

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
Copy link
Author

kawi15 commented Feb 27, 2024

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
Copy link
Owner

letsar commented Feb 27, 2024

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
Copy link
Author

kawi15 commented Feb 27, 2024

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
Copy link
Author

kawi15 commented Mar 6, 2024

Any proggress/update?

@letsar
Copy link
Owner

letsar commented Mar 25, 2024

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
Copy link
Owner

letsar commented Mar 25, 2024

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
Copy link
Author

kawi15 commented Mar 25, 2024

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
Copy link

lwj1994 commented May 28, 2024

it also happened with any other sliver list.

CustomScrollView{
  // SliverMasonryGrid
  SliverMasonryGrid.count() ,
  // SliverList wrap with DecoratedSliver
  DecoratedSliver(
    SliverList.separated()
  )
}

@tara-pogancev
Copy link

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

@ghayas36
Copy link

ghayas36 commented Nov 5, 2024

Having same issue 😞

@shaunhossain
Copy link

i am also facing the same issue

@dodatw
Copy link

dodatw commented Dec 16, 2024

Facing same issue

@dodatw
Copy link

dodatw commented Dec 16, 2024

I saw it fixed in 0.8.0, is there any plan release new version ?

@ghayas36
Copy link

ghayas36 commented Dec 16, 2024 via email

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

No branches or pull requests

7 participants