Add delay to Watch / turn watch on/off

[iosephmagno]

CC: @rrousselGit moved conversation here to avoid spamming #1371.

Is your feature request related to a problem? Please describe.
The features potentially addresses below issues:

1. Screen animation can be janky if state is updated automatically via watch meanwhile screen animation is going on
2. Widget animation can be janky if state is updated automatically via watch meanwhile screen animation is going on

Video related to point 1)
This is a janky screen animation caused by watch.

video0.MOV

Video related to point 2)
This is a janky widget animation caused by watch.

video0.MP4

After investigating our code (see our conversation here #1371 (comment)), I've come to the conclusion that issues depend on flutter and we are left with just finding out a good workaround. Also, I've found that state auto-update is great in most times but not ideal in some scenarios where some degree of flexibility is needed to workaround flutter issues.

Here is a video of both issues (janky screen animation / janky widget animation) fixed by using read and rebuilding UI via Provider notifyListeners()

videozip_1155.MP4

Describe the solution you'd like
To fix point (1), we would like to assign a delay to watch
For example, with ref.watch(stateProvider, 250) state will be fetched via read for first 250ms and watch from 250ms on.
Another potential solution could be to have just a way to switch state from read to watch and we can do it outside plugin.
Like:

var state = ref.read(stateProvider);
Future.delayed(const Duration(milliseconds: 750), () {
    state = ref.watch(stateProvider);
});

We cannot use final but it is okay.

To fix point (2), we would like to turn watch on/off programmatically like:
StateNotifierProvider.AllowWatch(bool).
state.AllowWatch(false) will disable watch (listeners won't be notified),
state.AllowWatch(true) will enable watch back again.

Describe alternatives you've considered
I've considered to initially fetch state via read and then update the variable with watch, but that leads to bad state. refresh() and invalidate() don't help coz state gets wiped out.
I've considered using read and wrapping everything inside a Provider to update UI at need via notifyListeners(). That solution works but I don't like it.

[iosephmagno]

#1371 (comment)

That is not enough, also app is big and we need to manage everything from one place.
So, Im temporarily doing sth much ugly that makes my brain bleed:

From inside StateNotifierProvider

 void addAll(List<Message> messages) {
    state = [...state, ...messages];
    // Force rebuild WidgetNotifierProvider listeners
    Repository.widgetNotifierProvider.rebuild(650);
  }

Where widgetNotifierProvider is wrapping the UI and exposes just the rebuild() method:

class WidgetNotifierProvider extends ChangeNotifier {
  rebuild([int? delay]){
    if (delay != null) {
      Future.delayed(Duration(milliseconds: delay), () {
        notifyListeners();
      });
    } else {
      notifyListeners();
    }
  }

So when state changes via some method of StateNotifierProvider, we will update UI with or without some delay. This solution fixes both above points but is so ugly.

[iosephmagno]

Thought that we can just use ChangeNotifierProvider. That would clean the code and indeed seems to best suit our case. Closing this.

[iosephmagno]

@rrousselGit moving this here coz I didnt mean to publish it inside #1371

Discovered interesting things after testing below 3 solutions:
Tested on iphone pro 13 ios 16.7.2, flutter master, 3.18.0-18.0.pre.39

1 RiverPod only:
If I use ChangeNotifierProvider + watch, the screen animation will be janky if I do not delay notifyListeners(), and if I add a delay, it will be janky at times (like 1-2 times out of 10).

2 RiverPod + Provider:
If I use StateNotifierProvider + read, and wrap the UI inside a Provider and delay Provider notifyListeners(), the screen animation will be smooth all times. Notice that now Provider is the only responsible of firing rebuilds.

3 Provider only:
If I use Provider only and delay notifyListeners(), screen animation will be smooth all times.

I tested also with a simpler state, not a List, just simple Class. So issue is not related to the way we populate state but to some side effect of Riverpod watch due to maybe flutter issue.

Bottom line:
In case of state being updated meanwhile a screen animation is going on, smoothness is guaranteed only by reading state (not watching) and then firing a rebuild via notifyListeners() after some delay (enough delay to let screen animation ends).

Im currently using solution 2, coz we access more providers from same place and Riverpod code is cleaner in that case. Also, we have migrated to Riverpod and makes no sense to move back to Provider. We will keep using Provider as a companion. I would have gone with solution 1, but due to janks ChangeNotifierProvider is currently not a substitute of Provider in our case.

[iosephmagno]

@rrousselGit
#1371 (comment)
#1371 (comment)

Unfortunately I made above tests directly inside our code and don't have a sample.
A sample might be a 2-screens app. Inside the first screen we might have a button that just loads a state and opens the second screen with a cupertino animation. In the second screen we might have a widget listening to the state.

Checking above 3 cases would help find out perf issues / potential optimizations.

[Pupix]

I have ran into this same problem on android with auto_router but don't think that matters.

There's a ref.watch as the first line of a widget/page that gets loaded when the user clicks a list item and the data loads straight from firebase

The list view has something like

onTap() {
    context.pushRoute(NewsArticleDetailRoute(articleId: article.id));
}

And this is the detail page

@RoutePage()
class NewsArticleDetailPage extends ConsumerWidget {
  final String articleId;
  NewsArticleDetailPage({super.key, @PathParam('articleId') required this.articleId});

  @override
  Widget build(BuildContext context, WidgetRef ref) {
    final articleData = ref.watch(newsArticleDetailProvider(id: articleId));

    return articleData.when(error: (error, stack) {
      return Container(
          margin: EdgeInsets.all(48),
          child: Center(child: Text(AppLocalizations.of(context)!.newsArticleLoadingError)));
    }, loading: () {
      return Container();
    }, data: (article) {
      if (article == null) {
        return Container(
            margin: EdgeInsets.all(48), child: Center(child: Text(AppLocalizations.of(context)!.newsArticleNotFound)));
      }

      return Scaffold(
        appBar: AppBar(
          leading: IconButton(
            icon: Icon(Icons.arrow_back),
            onPressed: () => Navigator.of(context).pop(),
          ),
        ),
        body: Text('Will it lag?'),
      );
    });
  }
}

and the provider

@riverpod
class NewsArticleDetail extends _$NewsArticleDetail {
  FirebaseFirestore _firestore = FirebaseFirestore.instanceFor(app: Firebase.app(), databaseId: Constants.databaseId);

  late String? _id;

  @override
  Future<NewsArticle?> build({required String id}) async {
    CollectionReference newsColl = _firestore.collection(Constants.newsCollection);

    _id = id;

    final articleDoc = await newsColl
        .doc(_id)
        .withConverter(
          fromFirestore: (snapshot, _) => NewsArticle.fromJson(snapshot.data()),
          toFirestore: (article, _) => article.toJson(),
        )
        .get();

    return articleDoc.data();
  }
}

This is literally it, and the screen is janky during animation.

If I had to put my money on it I'd say that it's 2? or more fast rebuilds during the page transition. Because the data comes in right away, by the time the jank is over the article data is already visible.

@rrousselGit

[Pupix]

Here's something I've found:

If I wrap the data.when in a scaffold the jank goes away. I have no idea why. But it's been working on multiple pages.

@RoutePage()
class NewsArticleDetailPage extends ConsumerWidget {
  final String articleId;
  NewsArticleDetailPage(
      {super.key, @PathParam('articleId') required this.articleId});

  @override
  Widget build(BuildContext context, WidgetRef ref) {
    final articleData = ref.watch(newsArticleDetailProvider(id: articleId));

    return Scaffold(
        appBar: AppBar(
          leading: IconButton(
            icon: Icon(Icons.arrow_back),
            onPressed: () => Navigator.of(context).pop(),
          ),
        ),
        body: articleData.when(error: (error, stack) {
          return Container(
              margin: EdgeInsets.all(48),
              child: Center(
                  child: Text(
                      AppLocalizations.of(context)!.newsArticleLoadingError)));
        }, loading: () {
          return Container();
        }, data: (article) {
          if (article == null) {
            return Container(
                margin: EdgeInsets.all(48),
                child: Center(
                    child: Text(
                        AppLocalizations.of(context)!.newsArticleNotFound)));
          }

          return Text('Will it lag?');
        }));
  }
}

Doing this for some reason fixes the issue, but I have no idea why.

[delfme]

@rrousselGit was right in closing this issue. But the problem exists. We solved it by not watching when janks occur during screen animation and wrapping riverpod into Provider :) I know it might sound weird but it works and it is easy to handle things in a consistent way. So we use both Provider and Riverpod in our app.