[SuperTextField][SuperEditor] - Don't conflict with saved scroll offsets in page storage

[matthew-carroll]

A bug was found in a client app related to SuperTextField. I don't have a repro to paste here, but I found the following.

Situation: A tab view with multiple pages. The first page of the tab view has a SuperTextField at the top of the page. Below the SuperTextField is a list of items that can be scrolled up/down.

Repro: On the page with the SuperTextField, scroll the list item of items down a bit. Switch to another tab page. Switch back to the tab page with the SuperTextField. The SuperTextField now appears blank, because internally the SuperTextField has scrolled itself.

Root cause: Flutter seems to include code that automatically tries to load a scroll offset from PageStorage, and that automatic behavior is running within SuperTextField. I assume the reason we haven't seen this before is because we've never used a SuperTextField within a situation that used PageStorage to store a scroll offset.

The Flutter code that does this is within ScrollPosition, itself: https://github.com/flutter/flutter/blob/5d83a98331aca84d8489adda246c05c6084b058a/packages/flutter/lib/src/widgets/scroll_position.dart#L532

Apparently, somehow, the built-in Flutter TextField doesn't have this problem. Perhaps Flutter's TextField added its own PageStorage? I haven't dug into it.

In either event, we need to ensure that neither SuperTextField nor SuperEditor or SuperReader apply someone else's scroll offset when being built, or rebuilt.

[angelosilvestre]

@matthew-carroll I was able to reproduce this issue with the following code in the example app:

@override
Widget build(BuildContext context) {
  return DefaultTabController(
    length: 3,
    child: Scaffold(
      appBar: AppBar(
        bottom: TabBar(
          tabs: const [
            Tab(text: 'Tab 1'),
            Tab(text: 'Tab 2'),
            Tab(text: 'Tab 3'),
          ],
        ),
      ),
      body: TabBarView(
        children: [
          KeyedSubtree(
            key: const PageStorageKey('page1'),
            child: Column(children: [
              Container(
                width: double.infinity,
                height: 300,
                child: ExampleEditor(),
              ),
              Expanded(
                child: ListView.builder(
                  itemCount: 100,
                  itemBuilder: (context, index) => Text(index.toString()),
                ),
              )
            ]),
          ),
          Center(
            child: Text('Tab 2'),
          ),
          Center(
            child: Text('Tab 3'),
          ),
        ],
      ),
    ),
  );

In this example, PageStorageKey is being used to store the scroll offset of the list view. Since there are two scrollables in the same subtree, both of them restore the scroll offset:

Screen.Recording.2024-09-16.at.22.50.39.mov

We can fix this by using another PageStorageKey to create a storage specific to the editor:

Container(
  width: double.infinity,
  height: 300,
  child: KeyedSubtree(
    key: const PageStorageKey('editor'),
    child: ExampleEditor(),
  ),
)

With this change it works as expected:

Screen.Recording.2024-09-16.at.22.55.08.mov

I'm not sure if we need to change anything in the editor/textfield. We cannot create a PageStorageKey inside the editor/textfield's state, because we would need to generate an unique key and the state will be thrown away when switching tabs, which will destroy the unique key. If we use a fixed key, then we will have problems when there are multiple textfields in the same subtree, because all of them will restore the same scroll offset.

Flutter's textfield seems to work because it uses the RestorationManager to restore the scroll offset:

/// {@template flutter.material.textfield.restorationId}
/// Restoration ID to save and restore the state of the text field.
///
/// If non-null, the text field will persist and restore its current scroll
/// offset and - if no [controller] has been provided - the content of the
/// text field. If a [controller] has been provided, it is the responsibility
/// of the owner of that controller to persist and restore it, e.g. by using
/// a [RestorableTextEditingController].
///
/// The state of this widget is persisted in a [RestorationBucket] claimed
/// from the surrounding [RestorationScope] using the provided restoration ID.
///
/// See also:
///
///  * [RestorationManager], which explains how state restoration works in
///    Flutter.
/// {@endtemplate}
final String? restorationId;

So, even if we implement the interaction with the RestorationManager, developers will still need to provide a restorationId.

[matthew-carroll]

I have a few questions.

First, do we currently expect scroll offsets to be maintained in SuperTextField and SuperEditor? If not, can we add a PageStorage within each of those and then not use it? I assume that would prevent SuperTextField and SuperEditor from restoring someone else's scroll offset.

@miguelcmedeiros - Do you guys currently retain scroll offsets in SuperTextField and SuperEditor across widget destruction and reconstruction?

Next, what's the relevance of RestorationManager with regard to Flutter's built in text field? I understand that RestorationManager talks to the platform to store data that can later be restored, but it's not clear how that info is relevant to this question of PageStorage use?

Lastly, if we decide we want to save and restore scroll offsets in SuperTextField and SuperEditor, should we add the storage ID as a widget property? Or should we use an ancestor configuration widget for the scrolling restoration, similar to what we do for per platform handle controls?

[angelosilvestre]

@matthew-carroll

First, do we currently expect scroll offsets to be maintained in SuperTextField and SuperEditor?

I think @miguelcmedeiros might have an answer for that.

If not, can we add a PageStorage within each of those and then not use it?

We can override PageStorageBucket and make it never write or read any state in the editor's subtree.

Next, what's the relevance of RestorationManager with regard to Flutter's built in text field? I understand that RestorationManager talks to the platform to store data that can later be restored, but it's not clear how that info is relevant to this question of PageStorage use?

It's not relevant to PageStore, I just wanted to point out that TextField uses something else.

Lastly, if we decide we want to save and restore scroll offsets in SuperTextField and SuperEditor, should we add the storage ID as a widget property? Or should we use an ancestor configuration widget for the scrolling restoration, similar to what we do for per platform handle controls?

We might have something like a SuperEditorRestorationPolicy, but if we have multiple SuperEditors in the same tree, each of them must have a separate configuration widget, because the keys must be different.

[miguelcmedeiros]

Do you guys currently retain scroll offsets in SuperTextField and SuperEditor across widget destruction and reconstruction?

@matthew-carroll, we don't but might be a interesting use case to be supported. In our case we can have multiple editors in the same tree.

[matthew-carroll]

@angelosilvestre what are the pros/cons of going with page storage vs restoration manager?

[angelosilvestre]

@matthew-carroll PageStore is simpler and stores the data on memory. The RestorationManager talks to the OS to store the data, and the data can be restored if the OS terminates the app and then opens it again. It's heavier, because it needs to talk to the engine whenever the restorable value changes, and there are limits on how much data can be stored.

If we only want to store the scroll position, we could expose a restorationId in SuperTextField and use the id in the inner SingleChildScrollView. That would persist the scroll position.

[matthew-carroll]

How about we go with PageStore to solve this problem, and then file an issue that says to consider adding a restorationId to SuperTextField? We can use both, right?