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

iOS 13 scrollbar #35829

Merged
merged 36 commits into from Jul 31, 2019

Conversation

@justinmc
Copy link
Contributor

commented Jul 9, 2019

Description

This PR implements iOS 13's draggable scrollbar. It can be activated by long pressing on the scrollbar or dragging in from the right on top of the scrollbar.

long press drag from right

Related Issues

Closes #35127

Tests

I test dragging the scrollbar with long press and drag from right in packages/flutter/test/cupertino/scrollbar_test.dart.

Usage

I struggled to come up with a nice way for developers to use this feature, because by default, Flutter has no scrollbars. You must wrap your scrollable widget in a Scrollbar or CupertinoScrollbar to get one, but then these scrollbar widgets have no command over the scroll position.

The solution I ended up using requires a ScrollController to be passed in, like this:

PrimaryScrollController(
  controller: scrollController,
  child: CupertinoScrollbar(
    controller: scrollController,
    child: MyLongListView(),
  ),
),

@justinmc justinmc force-pushed the justinmc:ios13-scrollbar branch from f604dea to 70b0801 Jul 10, 2019

justinmc added 4 commits Jul 10, 2019

@justinmc justinmc force-pushed the justinmc:ios13-scrollbar branch from bab14a5 to 7d6329c Jul 11, 2019

@justinmc justinmc force-pushed the justinmc:ios13-scrollbar branch from 17452ce to 7d467f0 Jul 19, 2019

@justinmc justinmc force-pushed the justinmc:ios13-scrollbar branch from 7d467f0 to e2a4af8 Jul 19, 2019

@justinmc justinmc changed the title WIP iOS 13 scrollbar iOS 13 scrollbar Jul 19, 2019

@justinmc justinmc force-pushed the justinmc:ios13-scrollbar branch from ef647c1 to d8cdb24 Jul 29, 2019

@justinmc justinmc merged commit fb2f3e5 into flutter:master Jul 31, 2019

78 of 79 checks passed

tool_coverage-linux Task Summary
Details
WIP Ready for review
Details
add2app-macos Task Summary
Details
add2app-macos
Details
analyze Task Summary
Details
analyze
Details
build_tests-linux Task Summary
Details
build_tests-linux
Details
build_tests-macos Task Summary
Details
build_tests-macos
Details
build_tests-windows Task Summary
Details
build_tests-windows
Details
cla/google All necessary CLAs are signed
customer_testing-linux Task Summary
Details
customer_testing-linux
Details
customer_testing-macos Task Summary
Details
customer_testing-macos
Details
customer_testing-windows Task Summary
Details
customer_testing-windows
Details
deploy_gallery Task Summary
Details
deploy_gallery
Details
deploy_gallery-macos Task Summary
Details
deploy_gallery-macos
Details
docs Task Summary
Details
docs
Details
flutter-build
Details
integration_tests-linux Task Summary
Details
integration_tests-linux
Details
integration_tests-macos Task Summary
Details
integration_tests-macos
Details
integration_tests-windows Task Summary
Details
integration_tests-windows
Details
integration_tests_gradle1-linux Task Summary
Details
integration_tests_gradle1-linux
Details
integration_tests_gradle1-windows Task Summary
Details
integration_tests_gradle1-windows
Details
integration_tests_gradle2-linux Task Summary
Details
integration_tests_gradle2-linux
Details
integration_tests_gradle2-windows Task Summary
Details
integration_tests_gradle2-windows
Details
release_smoke_tests Task Summary
Details
release_smoke_tests
Details
tests_extras-linux Task Summary
Details
tests_extras-linux
Details
tests_extras-macos Task Summary
Details
tests_extras-macos
Details
tests_extras-windows Task Summary
Details
tests_extras-windows
Details
tests_framework_other-linux Task Summary
Details
tests_framework_other-linux
Details
tests_framework_other-macos Task Summary
Details
tests_framework_other-macos
Details
tests_framework_other-windows Task Summary
Details
tests_framework_other-windows
Details
tests_widgets-linux Task Summary
Details
tests_widgets-linux
Details
tests_widgets-macos Task Summary
Details
tests_widgets-macos
Details
tests_widgets-windows Task Summary
Details
tests_widgets-windows
Details
tool_coverage-linux
Details
tool_tests-linux Task Summary
Details
tool_tests-linux
Details
tool_tests-macos Task Summary
Details
tool_tests-macos
Details
tool_tests-windows Task Summary
Details
tool_tests-windows
Details
tool_tests_create-linux Task Summary
Details
tool_tests_create-linux
Details
tool_tests_create-macos Task Summary
Details
tool_tests_create-macos
Details
tool_tests_create-windows Task Summary
Details
tool_tests_create-windows
Details
tool_tests_integration-linux Task Summary
Details
tool_tests_integration-linux
Details
tool_tests_integration-macos Task Summary
Details
tool_tests_integration-macos
Details
tool_tests_integration-windows Task Summary
Details
tool_tests_integration-windows
Details

@justinmc justinmc deleted the justinmc:ios13-scrollbar branch Jul 31, 2019

@Kavantix

This comment has been minimized.

Copy link

commented Aug 8, 2019

@justinmc I was just playing around with the scrollbar and comparing it to the native one.
What I noticed was firstly that the drag from the right was not something I can reproduce on a native app.
And also that the long press delay on a native app is almost instant, I would say around 150ms whereas on flutter right now it is half a second.

I managed to get the feel very close by removing the drag from the right gesture and adding a deadline field to the constructor of LongPressGestureRecognizer which I set to 150ms for the drag gesture.

@justinmc

This comment has been minimized.

Copy link
Contributor Author

commented Aug 8, 2019

@Kavantix I'm able to get the drag-from-right thing in the iOS 13 simulator, but it seems a little harder to activate than it is in Flutter now. I don't have an iOS 13 device at the moment to try it on. Maybe we could make it more strict. There are a few words mentioned about it in a Reddit thread.

You seem to be right that the long press threshold is shorter. Something like a deadline parameter might be a good way to solve that.

Would you mind opening an issue for these two points? That way anyone else having this problem can find your workaround and add to the discussion.

@Kavantix

This comment has been minimized.

Copy link

commented Aug 8, 2019

@justinmc

This comment has been minimized.

Copy link
Contributor Author

commented Aug 8, 2019

Thanks I appreciate it!

@justinmc justinmc referenced this pull request Sep 6, 2019
6 of 6 tasks complete
@Chimba123

This comment has been minimized.

Copy link

commented Sep 12, 2019

Hello, guys just trying this draggable scrollbar implementation in my flutter 1.9 stable upgrade but the draggable scrollbar is not working. Could it be that it is not yet available on the stable channel?

@justinmc

This comment has been minimized.

Copy link
Contributor Author

commented Sep 12, 2019

@Chimba123 It looks like this should be in >1.8.4, so your version looks good. Did you enable the feature properly? See the code block in the description up top that shows you how to enable this. For now it's not automatic.

@Chimba123

This comment has been minimized.

Copy link

commented Sep 12, 2019

@justinmc I did exactly that but it cannot find the controller in the PrimaryScrollView wrapping
I have initialized like so "ScrollController scrollController;"

@justinmc

This comment has been minimized.

Copy link
Contributor Author

commented Sep 12, 2019

Try this gist containing a full demo app that I know should work: https://gist.github.com/justinmc/3c58cb125644b230fe66832b0ea541c2

@Chimba123

This comment has been minimized.

Copy link

commented Sep 12, 2019

@justinmc "final ScrollController scrollController = ScrollController();" was the key thanks it works appreciate it

@justinmc

This comment has been minimized.

Copy link
Contributor Author

commented Sep 12, 2019

No problem!

rcunning added a commit to rcunning/flutter-slide-container that referenced this pull request Sep 12, 2019
@Chimba123

This comment has been minimized.

Copy link

commented Sep 15, 2019

Then another issue, how do we use this functionality in a CustomScrollView Widget with a linear list of scrolling widgets?

@justinmc

This comment has been minimized.

Copy link
Contributor Author

commented Sep 16, 2019

@Chimba123

This comment has been minimized.

Copy link

commented Sep 16, 2019

@justinmc my problem is like this (snippet below)

 Widget build(BuildContext context) {
    return CupertinoPageScaffold(
      child: CustomScrollView(
        slivers: <Widget>[
          SliverPersistentHeader(
            delegate: MyNav(),
            pinned: true,
            floating: false,
          ),
SliverList(),
        ],
      ),
    );
  }
}

I have a navbar if I use that method in the link the scrollbar scrolls above the navbar, I want it under

@Chimba123

This comment has been minimized.

Copy link

commented Sep 16, 2019

@justinmc I have found a way just wrap it in a SliverToBoxAdapter and set the media constraints of the containing widget like below

@override
  Widget build(BuildContext context) {
    return CupertinoPageScaffold(
      child: CustomScrollView(
        slivers: <Widget>[
          SliverPersistentHeader(
            delegate: MyNav(),
            pinned: true,
            floating: false,
          ),
          SliverToBoxAdapter(
           child: Container(
             height: MediaQuery.of(context).size.height,
              width:MediaQuery.of(context).size.width,
             child: PrimaryScrollController(
               controller: scrollController,
               child: CupertinoScrollbar(
                 controller: scrollController,
                 child: MediaQuery.removePadding(
                     context: context,
                     removeTop: true,
                     child: Container()
                   ),
               ),
             ),
           ),
           ),
        ],
      ),
    );
  }
}
@justinmc

This comment has been minimized.

Copy link
Contributor Author

commented Sep 16, 2019

Ok good to hear you got it, thanks for following up. I will edit your comments for formatting really quick.

@Liuzhiyang94

This comment has been minimized.

Copy link

commented Sep 17, 2019

@justinmc Hi. Could u add a property to set the Scrollbar show or hide. Or just like this typedef ScrollBarCanShow = bool Function(); let the user make Scrollbar show or hide according to logic.

@justinmc

This comment has been minimized.

Copy link
Contributor Author

commented Sep 17, 2019

Could you get the same outcome by removing the scrollbar from the widget tree? Like:

if (_showScrollbar) {
  return Scrollbar(
    child: MyListView(),
  );
}
return MyListView(); 
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.