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

when child is a listView,scrollbar behaves improperly. #46

Closed
leavky opened this issue Aug 23, 2023 · 19 comments
Closed

when child is a listView,scrollbar behaves improperly. #46

leavky opened this issue Aug 23, 2023 · 19 comments
Labels
bug Something isn't working Stale

Comments

@leavky
Copy link

leavky commented Aug 23, 2023

Thank you very much for releasing this package, it's been very helpful. I have encountered an issue while using it, specifically with the scrolling behavior. When I set the child to be a ListView or ReorderableListView, the scrollbar behaves improperly. Additionally, when the child is a ReorderableListView, the drag-and-drop sorting becomes very laggy.

@ulusoyca
Copy link
Collaborator

Hi, thanks for the feedback. Can you please provide sample code to reproduce and also the screen recording?

@ulusoyca ulusoyca added the bug Something isn't working label Aug 31, 2023
@leavky
Copy link
Author

leavky commented Sep 9, 2023

code in example, just replace child as ListView.builder.

    WoltModalSheetPage page1(BuildContext modalSheetContext, TextTheme textTheme) {
      return WoltModalSheetPage.withSingleChild(
        stickyActionBar: Padding(
          padding: const EdgeInsets.all(_pagePadding),
          child: Column(
            children: [
              ElevatedButton(
                onPressed: () => Navigator.of(modalSheetContext).pop(),
                child: const SizedBox(
                  height: _buttonHeight,
                  width: double.infinity,
                  child: Center(child: Text('Cancel')),
                ),
              ),
              const SizedBox(height: 8),
              ElevatedButton(
                onPressed: () => pageIndexNotifier.value = pageIndexNotifier.value + 1,
                child: const SizedBox(
                  height: _buttonHeight,
                  width: double.infinity,
                  child: Center(child: Text('Next page')),
                ),
              ),
            ],
          ),
        ),
        topBarTitle: Text('Pagination', style: textTheme.titleSmall),
        isTopBarLayerAlwaysVisible: true,
        leadingNavBarWidget: IconButton(
          padding: const EdgeInsets.all(_pagePadding),
          icon: const Icon(Icons.arrow_back_rounded),
          onPressed: () => pageIndexNotifier.value = pageIndexNotifier.value - 1,
        ),
        trailingNavBarWidget: IconButton(
          padding: const EdgeInsets.all(_pagePadding),
          icon: const Icon(Icons.close),
          onPressed: Navigator.of(modalSheetContext).pop,
        ),
        child: Padding(
          padding: const EdgeInsets.fromLTRB(
            _pagePadding,
            _pagePadding,
            _pagePadding,
            _bottomPaddingForButton,
          ),
          child: SizedBox(
            height: 400,
            child: ListView.builder(
                itemCount: 100,
                itemBuilder: (context, index) {
                  return Text(index.toString());
                }),
          ),
        ),
      );
    }

image

@leavky
Copy link
Author

leavky commented Sep 9, 2023

also in coffee_maker, in windows, the scroll hehavior so strangeness, when drag scrollerbar print Exception caught by gestures library
image

@ulusoyca
Copy link
Collaborator

ulusoyca commented Sep 9, 2023

@leavky First of all, if the main content is scrollable, you should use sliverList factory constructor for the page instead of withSingleChild since sliverList builds the main content items lazily.

Thanks for reporting the issue regarding the extra scroll bar shown in web and desktop apps. This PR (#58) fixes this issue.

@rydmike I can reproduce the exception thrown when scrollbar is dragged in MAC app. It is due to the DragScrollBehavior I use here that I took from FlexColorScheme. Are you familiar with it?

ping @TahaTesser

======== Exception caught by gestures library ======================================================
The following assertion was thrown while handling a pointer data packet:
'package:flutter/src/rendering/proxy_box.dart': Failed assertion: line 2962 pos 12: '!debugNeedsLayout': is not true.


Either the assertion indicates an error in the framework itself, or we should provide substantially more information in this error message to help you determine and fix the underlying cause.
In either case, please report this assertion by filing a bug on GitHub:
  https://github.com/flutter/flutter/issues/new?template=2_bug.md

When the exception was thrown, this was the stack: 
#2      RenderFractionalTranslation.hitTestChildren (package:flutter/src/rendering/proxy_box.dart:2962:12)

@rydmike
Copy link

rydmike commented Sep 9, 2023

Without looking into the details of this particular case, one issue I have seen on desktop, that seems very related to this, is that on desktop, scrollbars will appear by default where they are not seen on phones/tablets, this may cause issues with multiple scrollbars and where the default scroll controller belongs to both, which will cause a similar assert error. At least that is what I recall from seeing this issue before in the Themes Playground app.

What worked for me, in cases where both scrolling regions were needed, was to totally disable the scrollbar on the widget that should not have one (in the issue image you have two scrollbars for different scrolling regions, you probably only want one) and make sure you have only one scroll controller per scrolling surface, meaning you may have to create a scroll controller manually for them, to make them unique.

You can also try setting the 'primary: true' property on the list view that should be the main one if there are multiple ones, but I honestly don't remember if that helped at all, probably not.

It has been a while since I looked at my similar case, so I am just throwing what I recall from it on the wall. Sorry, I am on a phone now, so I can't look at the details. Without trying the reproduction sample and seeing the assert, and attempting a fix for it, I can't be 100% sure it is the same situation, but it sounds and looks very similar.

Sometimes you can also get rid of the widget causing the 2nd scrolling area and having them in the same scrolling area, depends on the use case of course.

@ulusoyca
Copy link
Collaborator

ulusoyca commented Sep 9, 2023

I found out that it is not the scroll bars causing this exceptions but the drag gestures on desktop @rydmike since I did this:

final scrollView = CustomScrollView(
      scrollBehavior: (const DragScrollBehavior()).copyWith(scrollbars: false),

@TahaTesser
Copy link
Collaborator

I found out that it is not the scroll bars causing this exceptions but the drag gestures on desktop @rydmike since I did this:

final scrollView = CustomScrollView(
      scrollBehavior: (const DragScrollBehavior()).copyWith(scrollbars: false),

Looks correct. I forgot I added an official Flutter example just for this behavior
https://api.flutter.dev/flutter/widgets/RawScrollbar-class.html#cupertino.RawScrollbar.3

@ulusoyca
Copy link
Collaborator

ulusoyca commented Sep 9, 2023

I found out that it is not the scroll bars causing this exceptions but the drag gestures on desktop @rydmike since I did this:

final scrollView = CustomScrollView(
      scrollBehavior: (const DragScrollBehavior()).copyWith(scrollbars: false),

Looks correct. I forgot I added an official Flutter example just for this behavior https://api.flutter.dev/flutter/widgets/RawScrollbar-class.html#cupertino.RawScrollbar.3

Ok, however we still receive exceptions caused by drag to scroll gestures on listview in desktop app.

@TahaTesser
Copy link
Collaborator

@rydmike
Copy link

rydmike commented Sep 9, 2023

Perfect 💙👍

In this case dragscroll is causing extra scrollbars to appear, and you get multiple ones (two in the example issue case), this causes the default scroll controller to get confused about which scrollbar should be using it, it gets very unhappy when two scrollbars both inherit the same one.

Fix is like you did, disable the scrollbar(s) where not needed, only keep default scrollbars in one place. The scrollbar left is then no longer upset that another scrollbar is also using the same default controller. The issue then goes away.

Alternatively use separate scroll controllers for each scroll area and thus scrollbar. This is seldom desirable though (to have two scrollbars), but there are some rare use cases for it.

Both options work, I tried them when dealing with similar issue in the Themes Playground app.

@TahaTesser
Copy link
Collaborator

The error thrown in #46 (comment) is very generic.
If you could isolate the issue with a minimal sample It'll help debug the issue.

@TahaTesser
Copy link
Collaborator

Also worth testing this issue on a different Flutter branch to make sure it's not an error from Flutter itself e.g. master.

@ulusoyca
Copy link
Collaborator

ulusoyca commented Sep 9, 2023

I found out that this is due to shrinkWrap set to true in CustomScrollView

 CustomScrollView(
      scrollBehavior: ScrollConfiguration.of(context).copyWith(
        dragDevices: <PointerDeviceKind>{
          PointerDeviceKind.touch,
          PointerDeviceKind.mouse,
          PointerDeviceKind.trackpad,
          PointerDeviceKind.stylus,
        },
      ),
      shrinkWrap: true,
  /// Shrink wrapping the content of the scroll view is significantly more
  /// expensive than expanding to the maximum allowed size because the content
  /// can expand and contract during scrolling, which means the size of the
  /// scroll view needs to be recomputed whenever the scroll position changes.

ChatGPT says:
The error you're seeing, specifically the !debugNeedsLayout assertion failure, suggests that there's a widget in your tree that is being accessed or interacted with (in this case, during a hit test) before its layout phase is complete. This kind of error is often associated with how your widgets are being built, especially if you're doing dynamic or conditional widget construction.

@TahaTesser
Copy link
Collaborator

TahaTesser commented Sep 10, 2023

Should avoid using shrinkWrap anywhere in the package. Please see flutter/flutter#130360

@ulusoyca
Copy link
Collaborator

ulusoyca commented Sep 10, 2023

flutter/flutter#130360

Ok, this is important, and quite news to me. The whole modal sheet layout calculations rely on shrinkWrap. I need to find a way of getting the implicit height of the main content during rendering to set the layout sheet which is sliver. Let's have a meeting about it when you have time.

@TahaTesser
Copy link
Collaborator

Sure thing

@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove "Stale" label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale label Oct 11, 2023
@yunusemrebakir
Copy link
Collaborator

I will create a separate issue to continue shrinkWrap and size calculation discussions.

@iyar-avital
Copy link

iyar-avital commented Dec 7, 2023

Same issue, can't solve.
What is the actual solution?

If I cancel my scroll by the ScrollBehavior, The other scrollbar not work by touch @ulusoyca

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Stale
Projects
None yet
Development

No branches or pull requests

6 participants