flutter / flutter

Flutter makes it easy and fast to build beautiful apps for mobile and beyond
https://flutter.dev
BSD 3-Clause "New" or "Revised" License
166.26k stars 27.51k forks source link

NestedScrollView with RefreshIndicator: dragging effect of RefreshIndicator seems to disappear with a slight drag #147363

Open FrankyLee-dev opened 6 months ago

FrankyLee-dev commented 6 months ago

Steps to reproduce

1.After placing RefreshIndicator inside NestedScrollView, 2.When there's a lot of content in NestedScrollView's headerSliverBuilder, 3.The dragging effect for pulling down to refresh disappears, and a slight drag immediately triggers the refresh action.

Expected results

The consistent dragging effect when content is less in headerSliverBuilder

Actual results

After nesting RefreshIndicator within NestedScrollView, when the content of NestedScrollView's headerSliverBuilder is extensive, the dragging effect of pulling down for refresh seems to disappear. A slight drag immediately triggers the refresh action.

Code sample

Code sample ```dart class MyHomePage extends StatefulWidget { const MyHomePage({super.key, required this.title}); final String title; @override State createState() => _MyHomePageState(); } class _MyHomePageState extends State with SingleTickerProviderStateMixin { late TabController _tabController; @override void initState() { super.initState(); _tabController = TabController(length: 3, vsync: this); } @override void dispose() { _tabController.dispose(); super.dispose(); } @override Widget build(BuildContext context) { return Scaffold( appBar: AppBar( backgroundColor: Theme.of(context).colorScheme.inversePrimary, title: Text(widget.title), ), body: RefreshIndicator( notificationPredicate: (notification) { return notification.depth == 2; }, onRefresh: () async { await Future.delayed(const Duration(seconds: 2), () {}); }, child: NestedScrollView( headerSliverBuilder: ( BuildContext context, bool innerBoxIsScrolled, ) { return [ SliverToBoxAdapter( child: Container( height: 1000, // Adjusting the simulated height of content in headerSliverBuilder color: Colors.amber, alignment: Alignment.center, child: const Text("header"), ), ), ]; }, body: TabBarView( controller: _tabController, children: [ ListView(), ListView(), ListView(), ], ), ), ), // This trailing comma makes auto-formatting nicer for build methods. ); } } ```

Screenshots or Video

https://github.com/flutter/flutter/assets/17977044/44ff84d3-b465-4871-b94e-d7e96735fafb

Logs

Logs ```console ```

Flutter Doctor output

Doctor output ```console [✓] Flutter (Channel stable, 3.19.5, on macOS 14.4.1 23E224 darwin-x64, locale zh-Hans-CN) [✓] Android toolchain - develop for Android devices (Android SDK version 30.0.3) [!] Xcode - develop for iOS and macOS (Xcode 14.2) ! CocoaPods 1.10.2 out of date (1.13.0 is recommended). CocoaPods is used to retrieve the iOS and macOS platform side's plugin code that responds to your plugin usage on the Dart side. Without CocoaPods, plugins will not work on iOS or macOS. For more info, see https://flutter.dev/platform-plugins To upgrade see https://guides.cocoapods.org/using/getting-started.html#updating-cocoapods for instructions. [✓] Chrome - develop for the web [✓] Android Studio (version 2021.2) [✓] VS Code (version 1.87.2) [✓] Connected device (3 available) ```
huycozy commented 6 months ago

Thank you for the report. I'm able to reproduce the issue as well.

flutter doctor -v (stable and master) ```bash [✓] Flutter (Channel stable, 3.19.6, on macOS 14.1 23B74 darwin-x64, locale en-VN) • Flutter version 3.19.6 on channel stable at /Users/huynq/Documents/GitHub/flutter • Upstream repository https://github.com/flutter/flutter.git • Framework revision 54e66469a9 (31 hours ago), 2024-04-17 13:08:03 -0700 • Engine revision c4cd48e186 • Dart version 3.3.4 • DevTools version 2.31.1 [✓] Android toolchain - develop for Android devices (Android SDK version 34.0.0) • Android SDK at /Users/huynq/Library/Android/sdk • Platform android-34, build-tools 34.0.0 • ANDROID_HOME = /Users/huynq/Library/Android/sdk • Java binary at: /Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java • Java version OpenJDK Runtime Environment (build 17.0.9+0-17.0.9b1087.7-11185874) • All Android licenses accepted. [✓] Xcode - develop for iOS and macOS (Xcode 15.3) • Xcode at /Applications/Xcode15.3.app/Contents/Developer • Build 15E204a • CocoaPods version 1.15.2 [✓] Chrome - develop for the web • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome [✓] Android Studio (version 2023.2) • Android Studio at /Applications/Android Studio.app/Contents • Flutter plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/9212-flutter • Dart plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/6351-dart • android-studio-dir = /Applications/Android Studio.app/ • Java version OpenJDK Runtime Environment (build 17.0.9+0-17.0.9b1087.7-11185874) [✓] VS Code (version 1.88.1) • VS Code at /Applications/Visual Studio Code.app/Contents • Flutter extension version 3.86.0 [✓] Connected device (3 available) • RMX2001 (mobile) • EUYTFEUSQSRGDA6D • android-arm64 • Android 11 (API 30) • macOS (desktop) • macos • darwin-x64 • macOS 14.1 23B74 darwin-x64 • Chrome (web) • chrome • web-javascript • Google Chrome 123.0.6312.124 [✓] Network resources • All expected network resources are available. • No issues found! ``` ```bash [!] Flutter (Channel master, 3.22.0-16.0.pre.71, on macOS 14.1 23B74 darwin-x64, locale en-VN) • Flutter version 3.22.0-16.0.pre.71 on channel master at /Users/huynq/Documents/GitHub/flutter_master ! Warning: `flutter` on your path resolves to /Users/huynq/Documents/GitHub/flutter/bin/flutter, which is not inside your current Flutter SDK checkout at /Users/huynq/Documents/GitHub/flutter_master. Consider adding /Users/huynq/Documents/GitHub/flutter_master/bin to the front of your path. ! Warning: `dart` on your path resolves to /Users/huynq/Documents/GitHub/flutter/bin/dart, which is not inside your current Flutter SDK checkout at /Users/huynq/Documents/GitHub/flutter_master. Consider adding /Users/huynq/Documents/GitHub/flutter_master/bin to the front of your path. • Upstream repository https://github.com/flutter/flutter.git • Framework revision 014cf332d7 (4 hours ago), 2024-04-25 19:16:33 -0400 • Engine revision a09295fe03 • Dart version 3.5.0 (build 3.5.0-101.0.dev) • DevTools version 2.35.0-dev.8 • If those were intentional, you can disregard the above warnings; however it is recommended to use "git" directly to perform update checks and upgrades. [✓] Android toolchain - develop for Android devices (Android SDK version 34.0.0) • Android SDK at /Users/huynq/Library/Android/sdk • Platform android-34, build-tools 34.0.0 • ANDROID_HOME = /Users/huynq/Library/Android/sdk • Java binary at: /Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java • Java version OpenJDK Runtime Environment (build 17.0.9+0-17.0.9b1087.7-11185874) • All Android licenses accepted. [✓] Xcode - develop for iOS and macOS (Xcode 15.3) • Xcode at /Applications/Xcode15.3.app/Contents/Developer • Build 15E204a • CocoaPods version 1.15.2 [✓] Chrome - develop for the web • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome [✓] Android Studio (version 2023.2) • Android Studio at /Applications/Android Studio.app/Contents • Flutter plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/9212-flutter • Dart plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/6351-dart • android-studio-dir = /Applications/Android Studio.app/ • Java version OpenJDK Runtime Environment (build 17.0.9+0-17.0.9b1087.7-11185874) [✓] VS Code (version 1.88.1) • VS Code at /Applications/Visual Studio Code.app/Contents • Flutter extension version 3.86.0 [✓] Connected device (3 available) • iPhone (mobile) • d9a94afe2b649fef56ba0bfeb052f0f2a7dae95e • ios • iOS 15.8 19H370 • macOS (desktop) • macos • darwin-x64 • macOS 14.1 23B74 darwin-x64 • Chrome (web) • chrome • web-javascript • Google Chrome 124.0.6367.92 [✓] Network resources • All expected network resources are available. ! Doctor found issues in 1 category. ```
ujjwaltwitx commented 6 months ago

I would like to work on this issue.

FrankyLee-dev commented 6 months ago

@ujjwaltwitx Thank you for your willingness to research this topic. Please inform me here if there are any developments. Thank you.

ujjwaltwitx commented 6 months ago

I am trying few tweaks right now. Will keep updating you on the developments

ujjwaltwitx commented 6 months ago

I checked the refresh_indicator.dart file and found out there was this _checkDragOffset function

 void _checkDragOffset(double containerExtent) {
    containerExtent = MediaQuery.of(context).size.height;
    // print(containerExtent);
    assert(_mode == _RefreshIndicatorMode.drag || _mode == _RefreshIndicatorMode.armed);
    double newValue = _dragOffset! / (containerExtent * _kDragContainerExtentPercentage);
    if (_mode == _RefreshIndicatorMode.armed) {
      newValue = math.max(newValue, 1.0 / _kDragSizeFactorLimit);
    }
    _positionController.value = clampDouble(newValue, 0.0, 1.0); // this triggers various rebuilds
    _positionController.value = newValue;
    if (_mode == _RefreshIndicatorMode.drag && _valueColor.value!.alpha == _effectiveValueColor.alpha) {
      _mode = _RefreshIndicatorMode.armed;
    }
  }

The containerExtent parameter is the primary reason of the issue. When the height of headerSliver is significantly large, the value of containerExtent is significantly low which causes the issue. I assigned screen height to the containerExtent and the refreshIndicator started working normally.

I am not sure whether this is a good solution to the problem or not so I would like to hear your valuable suggestions and feedbacks.

ujjwaltwitx commented 6 months ago

I checked the refresh_indicator.dart file and found out there was this _checkDragOffset function

 void _checkDragOffset(double containerExtent) {
    containerExtent = MediaQuery.of(context).size.height;
    // print(containerExtent);
    assert(_mode == _RefreshIndicatorMode.drag || _mode == _RefreshIndicatorMode.armed);
    double newValue = _dragOffset! / (containerExtent * _kDragContainerExtentPercentage);
    if (_mode == _RefreshIndicatorMode.armed) {
      newValue = math.max(newValue, 1.0 / _kDragSizeFactorLimit);
    }
    _positionController.value = clampDouble(newValue, 0.0, 1.0); // this triggers various rebuilds
    _positionController.value = newValue;
    if (_mode == _RefreshIndicatorMode.drag && _valueColor.value!.alpha == _effectiveValueColor.alpha) {
      _mode = _RefreshIndicatorMode.armed;
    }
  }

The containerExtent parameter is the primary reason of the issue. When the height of headerSliver is significantly large, the value of containerExtent is significantly low which causes the issue. I assigned screen height to the containerExtent and the refreshIndicator started working normally.

I am not sure whether this is a good solution to the problem or not so I would like to hear your valuable suggestions and feedbacks.

This is not the final code. If community approves this as a good fix then I will do the required refactoring in the code.

FrankyLee-dev commented 6 months ago

Hello, thank you for your question. If I understand correctly, you're asking about setting the screen height and you're unsure about the height of the headerSliver. In an extreme case where the headerSliver exceeds the screen height by a large margin, would the issue still occur?

ujjwaltwitx commented 6 months ago

No there will be no issue as the drag height is determined by the height of the screen and not by the height of the headerSliver.

FrankyLee-dev commented 6 months ago

I have tested it, and so far I haven't encountered any issues.I will continue to monitor it to see if any other issues arise.Thank you for your research.

ujjwaltwitx commented 6 months ago

Thanks. Should I raise a pull request?

FrankyLee-dev commented 6 months ago

Yes, please raise a PR. need to close the issue?

ujjwaltwitx commented 6 months ago

Okay

ujjwaltwitx commented 6 months ago

I have raised the pr