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

[scrollable_positioned_list] Expose ScrollController to be observed by Scrollbar #305

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nabil6391
Copy link

@nabil6391 nabil6391 commented Nov 14, 2021

Description

This PR exposes the primary listview's scrollController inside the ScrollablePositionedList from the ItemScrollController so that it can be observer by ScrollBars.

This also allows passing the scrollController to the ItemScrollController.

As a result we will finally be able to use like this

    Scrollbar(
        thickness: 8,
        radius: Radius.circular(4),
        controller: itemScrollController.scrollController,
        child: ScrollablePositionedList.builder(
              itemCount: totalItems,
              itemScrollController: itemScrollController,
              itemBuilder: (context, index) {
                return Container(
                    color: getItemColor(index, context),
                    child: buildDropdownList(context, index),
                );
              },
          ),
     )

Related Issues

#273
#303
#278
#235
#175

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I signed the [CLA].
  • All tests from running flutter test pass.
  • flutter analyze does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

@google-cla google-cla bot added the cla: no label Nov 14, 2021
@nabil6391 nabil6391 marked this pull request as draft November 14, 2021 05:38
@nabil6391 nabil6391 closed this Nov 14, 2021
@google-cla google-cla bot added cla: yes CLA has been signed by all contributors and removed cla: no labels Nov 14, 2021
…troller so that it can be observer by ScrollBars.

This also allows passing the scrollController to the ItemScrollController.
Pass the scrollController from the itemController inside the ListDisplayDetail
Specify ScrollController type
Use late to specifically assign ListDisplayDetails to primary and secondary

Signed-off-by: Nabil Mosharraf <nabil.mosharraf@photobookworldwide.com>
@nabil6391 nabil6391 reopened this Nov 14, 2021
@nabil6391 nabil6391 marked this pull request as ready for review November 14, 2021 05:42
@nabil6391
Copy link
Author

nabil6391 commented Nov 15, 2021

@tarobins please have a look at this PR at your convenience

@tarobins
Copy link
Collaborator

Has this been well tested in the use case you are trying it with? Does the scrollbar behave as expected when you jump around etc?

@nabil6391
Copy link
Author

nabil6391 commented Nov 23, 2021

I used the above code into your example main.dart and wrapped the ScrollablePositionedList with the Scrollbar.

Here are the results:

Horizontal around 30

Screenshot_20211124-012507

Horizontal around 100

Screenshot_20211124-012501

Vertical around 1000

Screenshot_20211124-012306

Vertical around 5000

Screenshot_20211124-012249

According to my manual tests, both jump and scroll to also works fine and the scroll bar automatically is updated.

@nabil6391 nabil6391 changed the title Expose ScrollController to be observed by Scrollbar [scrollable_positioned_list] Expose ScrollController to be observed by Scrollbar Nov 26, 2021
@yokemuratakeshi
Copy link

I incorporated this change into my project and it's working pretty well.

@yokemuratakeshi
Copy link

It looked like working, but when performing a long distance scroll my listener seems to have lost track of the scroll.
Is it because the internal behavior that it uses two ListView's to make the long scroll smooth?

@lff5
Copy link

lff5 commented Jan 3, 2022

Hi. Any progress on this?

@nabil6391
Copy link
Author

I am happily using it in my app after forking, depends on the authors to decide if they want to merge this or not.

@korvan
Copy link

korvan commented Feb 10, 2022

There is a problem with exposed scrollController.
After list is scrolled to a long distance, primary and secondary _ListDisplayDetails are swapped. After such scroll, exposed ItemScrollController.scrollController is not attached to any ScrollPosition and can't be used any more.

@TheCarpetMerchant
Copy link

@korvan As long as that is properly documented in-code (so on ItemScrollController.scrollController) , I do not think this to be an issue as this is purely added functionality.
Also, I'm not sure why this PR exists, as scroll notifications are passed up the tree if not intercepted. My Scrollbar works perfectly fine.

But I would like to be able to scroll a precise number of pixels, and for that I need access to ScrollController. It could also be added as another API to ItemScrollController (ie jumpToPixel for example).
I'll see about making my own PR for that, but merging this one would also solve my problem.

@nabil6391
Copy link
Author

In my case, I required exposing the scrollController to jump to a position and also use it with a custom ScrollBar such as DraggableScrollbar. Thats the reason for me to create this PR, in order to expose it for whatever requirements we need.

@TheCarpetMerchant
Copy link

@nabil6391 Ah of course, that makes sense then.
In the meantime I made #388 for my own needs. Oh well.

@saibotma
Copy link

saibotma commented Jul 7, 2022

Will this be merged eventually @tarobins?

@felipecastrosales
Copy link

👁️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes CLA has been signed by all contributors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants