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

SelectableText with NeverScrollableScrollPhysics needs unique PageStorageKey to not conflict with parent Scrollable #129117

Open
2 tasks done
saibotma opened this issue Jun 19, 2023 · 5 comments
Labels
f: material design flutter/packages/flutter/material repository. f: scrolling Viewports, list views, slivers, etc. found in release: 3.10 Found to occur in 3.10 found in release: 3.12 Found to occur in 3.12 framework flutter/packages/flutter repository. See also f: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on P2 Important issues not at the top of the work list team-design Owned by Design Languages team triaged-design Triaged by Design Languages team

Comments

@saibotma
Copy link

saibotma commented Jun 19, 2023

Is there an existing issue for this?

Steps to reproduce

  1. Run the example (best on iOS)
  2. Open "Tab 0"
  3. Scroll down until the text is visible
  4. Make sure that scrolling has ended, so that it got persisted in page storage
  5. Switch to "Tab 1"
  6. Switch back to "Tab 0"
  7. Notice that the scroll position got restored correctly, but that the text scrolls weirdly
  8. Switch back to "Tab 1"
  9. Switch back to "Tab 0" and notice that the scroll position did reset to 0

Expected results

  • Text should not scroll, because it has NeverScrollableScrollPhysics set.
  • Scroll position should not reset to 0, but stay at the previous position.

Actual results

  • Text scrolls, obviously restoring its scroll state from the previously stored scroll position of the parent ListView.
  • Scroll position resets to 0, because this is what the SelectableText persisted to page storage.

It works, when the SelectableText gets its own unique PageStorageKey, however this is not very obvious because:

  • you (me at least) don't expect that SelectableText can actually scroll.
  • a scrollable with NeverScrollableScrollPhysics should have scrolling disabled.

Code sample

Code sample
import 'package:flutter/material.dart';

void main() {
  runApp(const MyApp());
}

final _bucket = PageStorageBucket();

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  // This widget is the root of your application.
  @override
  Widget build(BuildContext context) {
    return const MaterialApp(
      home: PageStorageBug(),
    );
  }
}

class PageStorageBug extends StatefulWidget {
  const PageStorageBug({Key? key}) : super(key: key);

  @override
  State<PageStorageBug> createState() => _PageStorageBugState();
}

class _PageStorageBugState extends State<PageStorageBug> {
  int _tabIndex = 0;

  @override
  Widget build(BuildContext context) {
    return PageStorage(
      bucket: _bucket,
      child: Scaffold(
        body: _tabIndex == 0
            ? const Tab(key: ValueKey(0), title: "Tab 0")
            : const Tab(key: ValueKey(1), title: "Tab 1"),
        bottomNavigationBar: BottomNavigationBar(
          currentIndex: _tabIndex,
          items: const [
            BottomNavigationBarItem(
              icon: Icon(Icons.access_alarm),
              label: "Tab 0",
            ),
            BottomNavigationBarItem(icon: Icon(Icons.add), label: "Tab 1")
          ],
          onTap: (index) => setState(() => _tabIndex = index),
        ),
      ),
    );
  }
}

class Tab extends StatelessWidget {
  final String title;

  const Tab({required this.title, Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Scrollbar(
        child: ListView(
          key: PageStorageKey("$title-main"),
          children: [
            Container(height: 1000, color: Colors.blue),
            SelectableText(
              // It obviously works, when this key is set.
              //key: PageStorageKey("$title-text"),
              title,
              style: const TextStyle(fontSize: 50),
              scrollPhysics: const NeverScrollableScrollPhysics(),
            ),
            Container(height: 3000, color: Colors.red),
          ],
        ),
      ),
    );
  }
}

Screenshots or Video

Screenshots / Video demonstration
Screen.Recording.2023-06-19.at.14.56.24.mov

Logs

Logs
[Paste your logs here]

Flutter Doctor output

Doctor output
[!] Flutter (Channel stable, 3.10.5, on macOS 13.4 22F66 darwin-arm64, locale en-DE)
    • Flutter version 3.10.5 on channel stable at /Users/tobias/dev/tools/flutter
    ! Warning: `flutter` on your path resolves to /Users/tobias/Dev/Tools/flutter/bin/flutter, which is not inside your current Flutter SDK checkout at /Users/tobias/dev/tools/flutter. Consider adding /Users/tobias/dev/tools/flutter/bin to the front of your path.
    ! Warning: `dart` on your path resolves to /Users/tobias/Dev/Tools/flutter/bin/dart, which is not inside your current Flutter SDK checkout at /Users/tobias/dev/tools/flutter. Consider adding /Users/tobias/dev/tools/flutter/bin to the front of your path.
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 796c8ef792 (6 days ago), 2023-06-13 15:51:02 -0700
    • Engine revision 45f6e00911
    • Dart version 3.0.5
    • DevTools version 2.23.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 33.0.2)
    • Android SDK at /Users/tobias/Library/Android/sdk
    • Platform android-33, build-tools 33.0.2
    • ANDROID_HOME = /Users/tobias/Library/Android/sdk
    • Java binary at: /Users/tobias/Library/Application Support/JetBrains/Toolbox/apps/AndroidStudio/ch-0/222.4459.24.2221.9971841/Android Studio.app/Contents/jbr/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b802.4-9586694)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 14.3.1)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 14E300c
    • CocoaPods version 1.12.0

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2022.1)
    • Android Studio at /Users/tobias/Library/Application Support/JetBrains/Toolbox/apps/AndroidStudio/ch-0/221.6008.13.2211.9619390/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 11.0.15+0-b2043.56-8887301)

[✓] Android Studio (version 2022.2)
    • Android Studio at /Users/tobias/Library/Application Support/JetBrains/Toolbox/apps/AndroidStudio/ch-0/222.4459.24.2221.9971841/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.6+0-17.0.6b802.4-9586694)

[✓] IntelliJ IDEA Ultimate Edition (version 2023.1.2)
    • IntelliJ at /Users/tobias/Applications/JetBrains Toolbox/IntelliJ IDEA Ultimate.app
    • Flutter plugin version 73.1.1
    • Dart plugin version 231.9065

[✓] IntelliJ IDEA Ultimate Edition (version 2023.1)
    • IntelliJ at /Users/tobias/Library/Application Support/JetBrains/Toolbox/apps/IDEA-U/ch-0/231.8109.175/IntelliJ IDEA.app
    • 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

[✓] IntelliJ IDEA Ultimate Edition (version 2023.1.2)
    • IntelliJ at /Users/tobias/Library/Application Support/JetBrains/Toolbox/apps/IDEA-U/ch-0/231.9011.34/IntelliJ IDEA.app
    • 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

[✓] VS Code (version 1.79.1)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension can be installed from:
      🔨 https://marketplace.visualstudio.com/items?itemName=Dart-Code.flutter

[✓] Connected device (4 available)
    • KrasserDude (mobile)   • 79aa79a8f05d850905d52818fde5eea8d1b898ff • ios            • iOS 15.1 19B74
    • iPhone 14 Pro (mobile) • 81F2A201-82A9-4AF1-B8E6-6069F3222FB0     • ios            • com.apple.CoreSimulator.SimRuntime.iOS-16-4 (simulator)
    • macOS (desktop)        • macos                                    • darwin-arm64   • macOS 13.4 22F66 darwin-arm64
    • Chrome (web)           • chrome                                   • web-javascript • Google Chrome 114.0.5735.133

[✓] Network resources
    • All expected network resources are available.

! Doctor found issues in 1 category.
@darshankawar darshankawar added the in triage Presently being triaged by the triage team label Jun 20, 2023
@darshankawar
Copy link
Member

Thanks for the detailed report. This is replicable on Android too, so doesn't look specific to iOS.

stable, master flutter doctor -v
[!] Flutter (Channel stable, 3.10.5, on macOS 12.2.1 21D62 darwin-x64, locale
    en-GB)
    • Flutter version 3.10.5 on channel stable at
      /Users/dhs/documents/fluttersdk/flutter
    ! Warning: `flutter` on your path resolves to
      /Users/dhs/Documents/Fluttersdk/flutter/bin/flutter, which is not inside
      your current Flutter SDK checkout at
      /Users/dhs/documents/fluttersdk/flutter. Consider adding
      /Users/dhs/documents/fluttersdk/flutter/bin to the front of your path.
    ! Warning: `dart` on your path resolves to
      /Users/dhs/Documents/Fluttersdk/flutter/bin/dart, which is not inside your
      current Flutter SDK checkout at /Users/dhs/documents/fluttersdk/flutter.
      Consider adding /Users/dhs/documents/fluttersdk/flutter/bin to the front
      of your path.
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 796c8ef792 (2 days ago), 2023-06-13 15:51:02 -0700
    • Engine revision 45f6e00911
    • Dart version 3.0.5
    • DevTools version 2.23.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.

[!] Xcode - develop for iOS and macOS (Xcode 12.3)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    ! Flutter recommends a minimum Xcode version of 13.
      Download the latest version or update via the Mac App Store.
    • CocoaPods version 1.11.2

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] VS Code (version 1.62.0)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.21.0

[✓] Connected device (5 available)
    • SM G975F (mobile)       • RZ8M802WY0X • android-arm64   • Android 11 (API 30)
    • Darshan's iphone (mobile)  • 21150b119064aecc249dfcfe05e259197461ce23 •
      ios            • iOS 14.4.1 18D61
    • iPhone 12 Pro Max (mobile) • A5473606-0213-4FD8-BA16-553433949729     •
      ios            • com.apple.CoreSimulator.SimRuntime.iOS-14-3 (simulator)
    • macOS (desktop)            • macos                                    •
      darwin-x64     • Mac OS X 10.15.4 19E2269 darwin-x64
    • Chrome (web)               • chrome                                   •
      web-javascript • Google Chrome 98.0.4758.80

[✓] HTTP Host Availability
    • All required HTTP hosts are available

! Doctor found issues in 1 category.

[!] Flutter (Channel master, 3.12.0-4.0.pre.121, on macOS 12.2.1 21D62
    darwin-x64, locale en-GB)
    • Flutter version 3.12.0-4.0.pre.121 on channel master at
      /Users/dhs/documents/fluttersdk/flutter
    ! Warning: `flutter` on your path resolves to
      /Users/dhs/Documents/Fluttersdk/flutter/bin/flutter, which is not inside
      your current Flutter SDK checkout at
      /Users/dhs/documents/fluttersdk/flutter. Consider adding
      /Users/dhs/documents/fluttersdk/flutter/bin to the front of your path.
    ! Warning: `dart` on your path resolves to
      /Users/dhs/Documents/Fluttersdk/flutter/bin/dart, which is not inside your
      current Flutter SDK checkout at /Users/dhs/documents/fluttersdk/flutter.
      Consider adding /Users/dhs/documents/fluttersdk/flutter/bin to the front
      of your path.
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 5182697c0d (3 hours ago), 2023-06-19 21:37:33 -0400
    • Engine revision a91bb3f566
    • Dart version 3.1.0 (build 3.1.0-230.0.dev)
    • DevTools version 2.24.0
    • 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 30.0.3)
    • Android SDK at /Users/dhs/Library/Android/sdk
    ✗ cmdline-tools component is missing
      Run `path/to/sdkmanager --install "cmdline-tools;latest"`
      See https://developer.android.com/studio/command-line for more details.
    ✗ Android license status unknown.
      Run `flutter doctor --android-licenses` to accept the SDK licenses.
      See https://flutter.dev/docs/get-started/install/macos#android-setup for
      more details.

[✓] Xcode - develop for iOS and macOS (Xcode 13.2.1)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 13C100
    • CocoaPods version 1.11.2

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] IntelliJ IDEA Ultimate Edition (version 2021.3.2)
    • IntelliJ at /Applications/IntelliJ IDEA.app
    • Flutter plugin version 65.1.4
    • Dart plugin version 213.7228

[✓] VS Code (version 1.62.0)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.29.0

[✓] Connected device (3 available)
    • Darshan's iphone (mobile) • 21150b119064aecc249dfcfe05e259197461ce23 • ios
      • iOS 15.3.1 19D52
    • macOS (desktop)           • macos                                    •
      darwin-x64     • macOS 12.2.1 21D62 darwin-x64
    • Chrome (web)              • chrome                                   •
      web-javascript • Google Chrome 109.0.5414.119

[✓] Network resources
    • All expected network resources are available.

! Doctor found issues in 1 category.
      
[!] Xcode - develop for iOS and macOS (Xcode 12.3)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    ! Flutter recommends a minimum Xcode version of 13.
      Download the latest version or update via the Mac App Store.
    • CocoaPods version 1.11.2

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] VS Code (version 1.62.0)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.21.0

[✓] Connected device (5 available)
    • SM G975F (mobile)       • RZ8M802WY0X • android-arm64   • Android 11 (API 30)
    • Darshan's iphone (mobile)  • 21150b119064aecc249dfcfe05e259197461ce23 •
      ios            • iOS 14.4.1 18D61
    • iPhone 12 Pro Max (mobile) • A5473606-0213-4FD8-BA16-553433949729     •
      ios            • com.apple.CoreSimulator.SimRuntime.iOS-14-3 (simulator)
    • macOS (desktop)            • macos                                    •
      darwin-x64     • Mac OS X 10.15.4 19E2269 darwin-x64
    • Chrome (web)               • chrome                                   •
      web-javascript • Google Chrome 98.0.4758.80

[✓] HTTP Host Availability
    • All required HTTP hosts are available

! Doctor found issues in 1 category.



@darshankawar darshankawar added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. f: scrolling Viewports, list views, slivers, etc. has reproducible steps The issue has been confirmed reproducible and is ready to work on found in release: 3.10 Found to occur in 3.10 found in release: 3.12 Found to occur in 3.12 and removed in triage Presently being triaged by the triage team labels Jun 20, 2023
@Piinks
Copy link
Contributor

Piinks commented Jun 20, 2023

Hi @saibotma, this looks like based on the code in the OP this is working as expected.

a scrollable with NeverScrollableScrollPhysics should have scrolling disabled.

In the code sample, NeverScrollableScrollPhysics is set on the SelectableText, not the ListView. So the physics you have applied only applies to SelectableText.

To set scroll physics in a way that descendent scrollables will all uniformly adopt, use ScrollBehavior. I did notice looking at this that SelectableText does not inherit from ScrollBehavior, so I filed #129206 to follow up on that.

It looks like there is more than one issue stated in the OP, can you share details of the other issue, just to be sure we are mixing them up?

@Piinks Piinks added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Jun 20, 2023
@saibotma
Copy link
Author

Hi @Piinks, thanks for the answer. I think that you've not understood my issue correctly, yet.

The NeverScrollableScrollPhysics obviously should only apply to the SelectableText and they also do (you cannot scroll it with your finger). However when you take another close look at the video you will see that the SelectableText scrolls without user interaction, because (I assume) it tries to restore scroll state from PageStorage.

@github-actions github-actions bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Jun 21, 2023
@Piinks
Copy link
Contributor

Piinks commented Jun 21, 2023

Thank for providing more context! I am not sure what is happening here then... maybe @chunhtai or @Hangyujin have an idea? I think they are more familiar with SelectableText.

@Piinks Piinks added the P2 Important issues not at the top of the work list label Jun 21, 2023
@flutter-triage-bot flutter-triage-bot bot added multiteam-retriage-candidate team-design Owned by Design Languages team triaged-design Triaged by Design Languages team labels Jul 8, 2023
@gildaswise
Copy link
Contributor

gildaswise commented Sep 18, 2023

I'm having a similar issue in Flutter Web and I can confirm that adding a PageStorage above each SelectableText solves the issue. Without it, this happens even with NeverScrollableScrollPhysics:

Video demonstration
Screen.Recording.2023-09-18.at.11.45.22.mov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
f: material design flutter/packages/flutter/material repository. f: scrolling Viewports, list views, slivers, etc. found in release: 3.10 Found to occur in 3.10 found in release: 3.12 Found to occur in 3.12 framework flutter/packages/flutter repository. See also f: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on P2 Important issues not at the top of the work list team-design Owned by Design Languages team triaged-design Triaged by Design Languages team
Projects
None yet
Development

No branches or pull requests

5 participants