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.36k stars 27.54k forks source link

[SingleChildScrollView] Jitter when updating text during overscroll #145078

Open teunklijn opened 8 months ago

teunklijn commented 8 months ago

Steps to reproduce

In my prodution app I have a visible stopwatch on a SingleChildScrollView. I've included sample code which has the same issue. I think it's related to https://github.com/flutter/flutter/pull/136239, but that one is already closed.

  1. run the example app
  2. try to scroll up

Expected results

The scroll position stays the same and there's no visible jitter.

Actual results

The scroll position is reset everytime the text gets updated.

Code sample

Code sample ```dart import 'dart:async'; import 'dart:math'; import 'package:flutter/material.dart'; void main() { runApp(const MyApp()); } class MyApp extends StatelessWidget { const MyApp({super.key}); @override Widget build(BuildContext context) { return const MaterialApp( home: MyHomePage(), ); } } class MyHomePage extends StatelessWidget { const MyHomePage({super.key}); @override Widget build(BuildContext context) { return Scaffold( appBar: AppBar(), body: SingleChildScrollView( child: Column( children: [ const _Counter(), for (var i = 0; i < 15; i++) Container( height: 100, color: Color.fromRGBO(Random().nextInt(256), Random().nextInt(256), Random().nextInt(256), 1), ), ], ), ), ); } } class _Counter extends StatefulWidget { const _Counter(); @override State<_Counter> createState() => __CounterState(); } class __CounterState extends State<_Counter> { int _count = 0; @override void initState() { Timer.periodic(const Duration(milliseconds: 500), (timer) { setState(() { _count++; }); }); super.initState(); } @override Widget build(BuildContext context) { return Text('$_count'); } } ```

Screenshots or Video

Screenshots / Video demonstration **Expected behaviour** https://github.com/flutter/flutter/assets/27430651/a74ba7bb-9598-45f4-88bc-34be11faa897 **Actual behaviour** https://github.com/flutter/flutter/assets/27430651/350a7881-96d3-4eb3-b035-6e0ff84982a6

Logs

No response

Flutter Doctor output

Doctor output ```console flutter doctor Doctor summary (to see all details, run flutter doctor -v): [✓] Flutter (Channel stable, 3.19.3, on macOS 14.3.1 23D60 darwin-arm64, locale nl-NL) [✓] Android toolchain - develop for Android devices (Android SDK version 34.0.0) [✓] Xcode - develop for iOS and macOS (Xcode 15.3) [✗] Chrome - develop for the web (Cannot find Chrome executable at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome) ! Cannot find Chrome. Try setting CHROME_EXECUTABLE to a Chrome executable. [✓] Android Studio (version 2023.2) [✓] VS Code (version 1.87.0) [✓] Connected device (2 available) ! Error: Browsing on the local area network for iPhone van Govert. Ensure the device is unlocked and attached with a cable or associated with the same local area network as this Mac. The device must be opted into Developer Mode to connect wirelessly. (code -27) [✓] Network resources ! Doctor found issues in 1 category. ```
danagbemava-nc commented 8 months ago

Reproducible using the code sample provided above.

flutter doctor -v ``` [!] Flutter (Channel stable, 3.19.3, on macOS 14.3.1 23D60 darwin-arm64, locale en-GB) • Flutter version 3.19.3 on channel stable at /Users/nexus/dev/sdks/flutter ! Warning: `flutter` on your path resolves to /Users/nexus/dev/sdks/flutters/bin/flutter, which is not inside your current Flutter SDK checkout at /Users/nexus/dev/sdks/flutter. Consider adding /Users/nexus/dev/sdks/flutter/bin to the front of your path. ! Warning: `dart` on your path resolves to /Users/nexus/dev/sdks/flutters/bin/dart, which is not inside your current Flutter SDK checkout at /Users/nexus/dev/sdks/flutter. Consider adding /Users/nexus/dev/sdks/flutter/bin to the front of your path. • Upstream repository https://github.com/flutter/flutter.git • Framework revision ba39319843 (6 days ago), 2024-03-07 15:22:21 -0600 • Engine revision 2e4ba9c6fb • Dart version 3.3.1 • DevTools version 2.31.1 • 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/nexus/Library/Android/sdk • Platform android-34, build-tools 34.0.0 • Java binary at: /Users/nexus/Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java • Java version OpenJDK Runtime Environment (build 17.0.7+0-17.0.7b1000.6-10550314) • All Android licenses accepted. [✓] Xcode - develop for iOS and macOS (Xcode 15.2) • Xcode at /Applications/Xcode-15.2.0.app/Contents/Developer • Build 15C500b • CocoaPods version 1.14.3 [✓] Chrome - develop for the web • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome [✓] Android Studio (version 2023.1) • Android Studio at /Users/nexus/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 • Java version OpenJDK Runtime Environment (build 17.0.7+0-17.0.7b1000.6-10550314) [✓] IntelliJ IDEA Ultimate Edition (version 2023.2.5) • IntelliJ at /Users/nexus/Applications/IntelliJ IDEA Ultimate.app • Flutter plugin version 77.2.2 • Dart plugin version 232.10286 [✓] VS Code (version 1.87.0) • VS Code at /Applications/Visual Studio Code.app/Contents • Flutter extension version 3.84.0 [✓] Connected device (6 available) • Pixel 7 (mobile) • 28291FDH2001SA • android-arm64 • Android 14 (API 34) • Pixel 7 (mobile) • 192.168.8.151:39133 • android-arm64 • Android 14 (API 34) • Nexus (mobile) • 00008020-001875E83A38002E • ios • iOS 17.3.1 21D61 • Dean’s iPad (mobile) • 00008103-000825C811E3401E • ios • iOS 17.3.1 21D61 • macOS (desktop) • macos • darwin-arm64 • macOS 14.3.1 23D60 darwin-arm64 • Chrome (web) • chrome • web-javascript • Google Chrome 122.0.6261.112 [✓] Network resources • All expected network resources are available. ! Doctor found issues in 1 category. ``` ``` [✓] Flutter (Channel master, 3.21.0-5.0.pre.23, on macOS 14.3.1 23D60 darwin-arm64, locale en-GB) • Flutter version 3.21.0-5.0.pre.23 on channel master at /Users/nexus/dev/sdks/flutters • Upstream repository https://github.com/flutter/flutter.git • Framework revision 8270872f86 (13 hours ago), 2024-03-12 17:52:24 -0700 • Engine revision ea848c3b24 • Dart version 3.4.0 (build 3.4.0-227.0.dev) • DevTools version 2.33.1 [✓] Android toolchain - develop for Android devices (Android SDK version 34.0.0) • Android SDK at /Users/nexus/Library/Android/sdk • Platform android-34, build-tools 34.0.0 • Java binary at: /Users/nexus/Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java • Java version OpenJDK Runtime Environment (build 17.0.7+0-17.0.7b1000.6-10550314) • All Android licenses accepted. [✓] Xcode - develop for iOS and macOS (Xcode 15.2) • Xcode at /Applications/Xcode-15.2.0.app/Contents/Developer • Build 15C500b • CocoaPods version 1.14.3 [✓] Chrome - develop for the web • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome [✓] Android Studio (version 2023.1) • Android Studio at /Users/nexus/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 • Java version OpenJDK Runtime Environment (build 17.0.7+0-17.0.7b1000.6-10550314) [✓] IntelliJ IDEA Ultimate Edition (version 2023.2.5) • IntelliJ at /Users/nexus/Applications/IntelliJ IDEA Ultimate.app • Flutter plugin version 77.2.2 • Dart plugin version 232.10286 [✓] VS Code (version 1.87.0) • VS Code at /Applications/Visual Studio Code.app/Contents • Flutter extension version 3.84.0 [✓] Connected device (7 available) • Pixel 7 (mobile) • 28291FDH2001SA • android-arm64 • Android 14 (API 34) • Pixel 7 (mobile) • 192.168.8.151:39133 • android-arm64 • Android 14 (API 34) • Nexus (mobile) • 00008020-001875E83A38002E • ios • iOS 17.3.1 21D61 • Dean’s iPad (mobile) • 00008103-000825C811E3401E • ios • iOS 17.3.1 21D61 • macOS (desktop) • macos • darwin-arm64 • macOS 14.3.1 23D60 darwin-arm64 • Mac Designed for iPad (desktop) • mac-designed-for-ipad • darwin • macOS 14.3.1 23D60 darwin-arm64 • Chrome (web) • chrome • web-javascript • Google Chrome 122.0.6261.112 [✓] Network resources • All expected network resources are available. • No issues found! ```
Alvish0407 commented 8 months ago

It happens in iOS Impeller

fritsvt commented 7 months ago

@teunklijn did you manage to find a workaround? We are also experiencing this issue and not sure if we should wait for a fix or try to work around it

teunklijn commented 7 months ago

@teunklijn did you manage to find a workaround? We are also experiencing this issue and not sure if we should wait for a fix or try to work around it

Not really, we've disabled BouncingScrollPhysics throughout the app until this gets fixed.

return MaterialApp.router(
   ...
   scrollBehavior: ScrollConfiguration.of(context).copyWith(
      physics: const ClampingScrollPhysics(),
   ),
   ...
);
fritsvt commented 7 months ago

Alright thanks @teunklijn, in our case that’s sadly not an option since it occurs in the pull to refresh indicator 😅 would look really weird without the bounce

PurplePolyhedron commented 7 months ago

@teunklijn did you manage to find a workaround? We are also experiencing this issue and not sure if we should wait for a fix or try to work around it

@fritsvt I found a work around by wrapping the updated part(Which is Text in the example) with SizedBox, Since child size doesn't change, updating it wouldn't trigger SingleChildScrollView re-layout.

This is very hacky, and will likely break in the future.

fritsvt commented 7 months ago

@PurplePolyhedron thanks! Unfortunately in our case it wouldn’t always work since the content might get updated after a pull to refresh :(

Edit: just tried and looks like it works but indeed very hacky

The fact they marked this P3 (not important) is really frustrating to see though.

PurplePolyhedron commented 7 months ago

@fritsvt There doesn't seems to be an easy fix to this issue. Meanwhile, you should be able to use ListView with shrinkWrap (which don't have this issue) instead of SingleChildScrollView in most cases.

fritsvt commented 7 months ago

Thanks for the suggestion @PurplePolyhedron I’ll give that a try later today. Also hope it will be fixed in the framework soon though!

Jure-BB commented 5 months ago

Overscroll reset happens here:

https://github.com/flutter/flutter/blob/35ae5191402bccbe573bb780c03f832dff0687ed/packages/flutter/lib/src/widgets/single_child_scroll_view.dart#L488

Added in #136871.

xu-baolin commented 5 months ago

@teunklijn hi, thank you for reporting this. You can give you Text widget a tight constraints to avoid to trigger unnecessary relayout for SingleChildScrollView, here is a migration for you, holp can help you:

  @override
  Widget build(BuildContext context) {
    return SizedBox(
      height: 30,
      width: 100,
      child: Text('$_count'),
    );
  }
Jure-BB commented 5 months ago

@teunklijn hi, thank you for reporting this. You can give you Text widget a tight constraints to avoid to trigger unnecessary relayout for SingleChildScrollView, here is a migration for you, holp can help you:

  @override
  Widget build(BuildContext context) {
    return SizedBox(
      height: 30,
      width: 100,
      child: Text('$_count'),
    );
  }

This works as a workaround, but is very error prone. It means that anywhere a SingleChildScrollView is used, we have to make sure that no content change can trigger a relayout. Since, content change can come from variety of sources (like FutureBuilder rebuilding, any animation that affects size etc.) this increases complexity as it adds another thing we need to keep an eye on.

Changing the performLayout method inside the SingleChildScrollView to something like the code below fixes this issue without the need to avoid relayouts.

  BoxConstraints? oldConstraints;
  @override
  void performLayout() {
    final BoxConstraints constraints = this.constraints;
    if (child == null) {
      size = constraints.smallest;
    } else {
      child!.layout(_getInnerConstraints(constraints), parentUsesSize: true);
      size = constraints.constrain(child!.size);
    }

    // restrict resetting an overscroll to zero to only when constraints change
    if (offset.hasPixels && constraints != oldConstraints) {
      if (offset.pixels > _maxScrollExtent) {
        offset.correctBy(_maxScrollExtent - offset.pixels);
      } else if (offset.pixels < _minScrollExtent) {
        offset.correctBy(_minScrollExtent - offset.pixels);
      }
    }
    oldConstraints = constraints;

    offset.applyViewportDimension(_viewportExtent);
    offset.applyContentDimensions(_minScrollExtent, _maxScrollExtent);
  }

I also tested #105733 with this change and it still works as it should.

xu-baolin commented 5 months ago

@teunklijn hi, thank you for reporting this. You can give you Text widget a tight constraints to avoid to trigger unnecessary relayout for SingleChildScrollView, here is a migration for you, holp can help you:

  @override
  Widget build(BuildContext context) {
    return SizedBox(
      height: 30,
      width: 100,
      child: Text('$_count'),
    );
  }

This works as a workaround, but is very error prone. It means that anywhere a SingleChildScrollView is used, we have to make sure that no content change can trigger a relayout. Since, content change can come from variety of sources (like FutureBuilder rebuilding, any animation that affects size etc.) this increases complexity as it adds another thing we need to keep an eye on.

Changing the performLayout method inside the SingleChildScrollView to something like the code below fixes this issue without the need to avoid relayouts.

  BoxConstraints? oldConstraints;
  @override
  void performLayout() {
    final BoxConstraints constraints = this.constraints;
    if (child == null) {
      size = constraints.smallest;
    } else {
      child!.layout(_getInnerConstraints(constraints), parentUsesSize: true);
      size = constraints.constrain(child!.size);
    }

    // restrict resetting an overscroll to zero to only when constraints change
    if (offset.hasPixels && constraints != oldConstraints) {
      if (offset.pixels > _maxScrollExtent) {
        offset.correctBy(_maxScrollExtent - offset.pixels);
      } else if (offset.pixels < _minScrollExtent) {
        offset.correctBy(_minScrollExtent - offset.pixels);
      }
    }
    oldConstraints = constraints;

    offset.applyViewportDimension(_viewportExtent);
    offset.applyContentDimensions(_minScrollExtent, _maxScrollExtent);
  }

I also tested #105733 with this change and it still works as it should.

Thank you very much for you investigation. The FW will correct the offset during layout phase, This behavior is consistent with other scrollable widget(such as ListView). If we want to change it, we need to consider all scrollable widgets instead of modifying SingleChildScrollView alone. I would be happy to continue discussing this issue with you.

This works as a workaround, but is very error prone. We should aviod trigger relayout during overscrolling, this is unnecessary and inefficient, right?

Jure-BB commented 5 months ago

This works as a workaround, but is very error prone. We should aviod trigger relayout during overscrolling, this is unnecessary and inefficient, right?

It's true that it's best to avoid relayout if possible, but in some cases a relayout can be necessary or hard to avoid. For example, if an asynchronous data load event coincides with user performing an overscroll. The new data might need a different layout and interrupting the overscroll action will feel jarring to the user.

The FW will correct the offset during layout phase, This behavior is consistent with other scrollable widget(such as ListView). If we want to change it, we need to consider all scrollable widgets instead of modifying SingleChildScrollView alone. I would be happy to continue discussing this issue with you.

I agree behavior should be kept consistent among all scrollable widgets. Would a modification like this (to correct the offset during layout phase only when constraints change) cause any issues with other scrollable widgets?

github-actions[bot] commented 4 months ago

Without additional information, we are unfortunately not sure how to resolve this issue. We are therefore reluctantly going to close this bug for now. If you find this problem please file a new issue with the same description, what happens, logs and the output of 'flutter doctor -v'. All system setups can be slightly different so it's always better to open new issues and reference the related ones. Thanks for your contribution.

teunklijn commented 4 months ago

Is this closed because it was waiting for customer response? I agree that there are workarounds, but they are not always practical. I agree with the comments by @Jure-BB.

synstin commented 3 months ago

I didn't encounter this issue with Flutter 3.22.2, but the same problem is occurring in 3.24. There's a ReorderableList as a child of SingleChildScrollView.

flutter-triage-bot[bot] commented 3 weeks ago

This issue is assigned to @xu-baolin but has had no recent status updates. Please consider unassigning this issue if it is not going to be addressed in the near future. This allows people to have a clearer picture of what work is actually planned. Thanks!