ReorderableListView and Riverpod - flickering in release mode

[wujek-srujek]

I have a ReorderableListView and it is possible to sort items by dragging and dropping them, as one would expect. Reordering logic is implemented in the StateNotifier managed by Riverpod, and the list is part of the state. Once sorting logic (a simple List.removeAt followed by List.insert at the correct index) called by onReorder is done, a new state with the new order is emitted.

It works, but in release builds it results in flickering - for a split second after the drag and drop gesture, the old ordering is restored, and then immediately afterwards the new one is displayed - but the result is noticeable and pretty jarring. I assume this is because the new state (with correct order) from my provider (as received by watch) is available a bit later (maybe the next event loop cycle?) than immediately after the onReorder is finished.

This doesn't happen when a plain old StatefulWidget is used with a list in the state, and when setState is called in onReorder - I assume this is because there is no 'delay' between updating the state and getting the new ordered list, as it is right there, immediately available.

I guess I will be able to implement a workaround 'caching' the list in the state and reordering it, and syncing with the provider right afterwards, so that the flickering doesn't happen, but I'm wondering, is there anything better than this hack?

[wujek-srujek]

So what I'm doing now is:

class _BlaList extends ConsumerStatefulWidget {
  const _BlaList();

  @override
  _BlaListState createState() => _BlaListState();
}

class _BlaListState extends ConsumerState {
  late List<Bla> blas;

  @override
  Widget build(BuildContext context) {
    blas = ref.watch(blasProvider);

    return ReorderableListView.builder(
      itemCount: blas.length,
      onReorder: (oldIndex, newIndex) {
        final newBlas = ref
            .read(blasProvider.notifier)
            .moveBla(oldIndex: oldIndex, newIndex: newIndex);
        setState(() {
          blas = newBlas;
        });
      },
      itemBuilder: (context, index) {
        final bla = blas[index];
        // ...
      },
    );
  }
}

And blaProvider has the same moving logic as before, but after setting the internal state, it also returns the new ordered list. This list is then immediately set as the state in _BlaListState, which uses this field for all its build logic. Whenever a new state is emitted using watch, the list is assigned to the internal widget state as well.

This does the trick, but I find it awful and complex, and I am not even sure if it is correct.

This also means that rebuilding the widget resulting from setState happens much faster than rebuilds resulting from state changes using Riverpod. Is this something that can be worked on?

[rrousselGit]

Do you have a full example to reproduce the issue?

[wujek-srujek]

Not yet but I can try to create one, stay tuned.

[wujek-srujek]

I'm using Flutter 2.2.3 and Riverpod 1.0.0-dev.6.

Here is the code (again, the flickering occurs only in release mode, tested with Pixel 3XL and iPhone XS). The issue is most visible if you drag the topmost item to the very bottom (or vice versa) as in this case all of the items will flicker as they will change their places in the list.

import 'package:flutter/material.dart';
import 'package:flutter_riverpod/flutter_riverpod.dart';

void main() {
  runApp(SampleApp());
}

class SampleApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return const ProviderScope(
      child: MaterialApp(
        home: Scaffold(
          body: SafeArea(
            child: SampleList(),
          ),
        ),
      ),
    );
  }
}

class SampleList extends ConsumerWidget {
  const SampleList();

  @override
  Widget build(BuildContext context, WidgetRef ref) {
    final blahs = ref.watch(_onlyBlahListWithoutOtherStuffProvider);

    return ReorderableListView.builder(
      itemCount: blahs.length,
      onReorder: (oldIndex, newIndex) {
        ref
            .read(blahNotifierProvider.notifier)
            .moveBlah(oldIndex: oldIndex, newIndex: newIndex);
      },
      itemBuilder: (context, index) {
        final blah = blahs[index];

        return Center(
          key: ObjectKey(blah),
          child: ListTile(
            title: Text(
              blah,
              style: Theme.of(context).textTheme.headline2!.copyWith(
                    color: Colors.black,
                  ),
            ),
          ),
        );
      },
    );
  }
}

final _onlyBlahListWithoutOtherStuffProvider = // <==== This intermediate provider is causing the issue.
    Provider.autoDispose<List<String>>(
  (ref) => ref.watch(
    blahNotifierProvider.select((state) => state.blahs),
  ),
);

// provider.dart

@immutable
class BlahState {
  final List<String> blahs;
  final String otherStuff;

  const BlahState({
    required this.blahs,
    required this.otherStuff,
  });
}

class BlahNotifier extends StateNotifier<BlahState> {
  BlahNotifier()
      : super(const BlahState(
          blahs: [
            'AAAAA',
            'BBBBB',
            'CCCCC',
            'DDDDD',
            'EEEEE',
            'FFFFF',
          ],
          otherStuff: 'other stuff',
        ));

  void moveBlah({required int oldIndex, required int newIndex}) {
    final newBlahs = [...state.blahs];
    final movedBlah = newBlahs.removeAt(oldIndex);
    final insertionIndex = newIndex > oldIndex ? newIndex - 1 : newIndex;
    newBlahs.insert(insertionIndex, movedBlah);
    state = BlahState(
      blahs: newBlahs,
      otherStuff: state.otherStuff,
    );
  }
}

final blahNotifierProvider =
    StateNotifierProvider.autoDispose<BlahNotifier, BlahState>(
  (ref) => BlahNotifier(),
);

As I was preparing the sample, it turns out that the reason is that I'm using an 'intermediate' provider (_onlyBlahListWithoutOtherStuffProvider in the sample above). Explanation: BlahState contains some stuff relevant for other use cases, but in the page showing the list and allowing reordering, I only ever need BlahState.blahs. In the page, I need it in a few places and I didn't want to write ref.watch(blahProvider.select(...)) all the time, so I implemented a private intermediate provider which extracts the list (using select) and provides it, and all the places in the page which need the list use this provider instead of the original one. This results in the flickering, I'm guessing this causes the event to arrive at my UI a bit later.

So, one solution is to get rid of _onlyBlahListWithoutOtherStuffProvider and repeat calling select in multiple places - is there anything better than this?

The other workaround is to replace SampleList with this:

class SampleList extends ConsumerStatefulWidget {
  const SampleList();

  @override
  _SampleListState createState() => _SampleListState();
}

class _SampleListState extends ConsumerState {
  late List<String> _blahs;

  @override
  Widget build(BuildContext context) {
    _blahs = ref.watch(_onlyBlahListWithoutOtherStuffProvider);

    return ReorderableListView.builder(
      itemCount: _blahs.length,
      onReorder: (oldIndex, newIndex) {
        final newBlahs = ref
            .read(blahNotifierProvider.notifier)
            .moveBlah(oldIndex: oldIndex, newIndex: newIndex);
        setState(() => _blahs = newBlahs);
      },
      itemBuilder: (context, index) {
        final blah = _blahs[index];

        // The rest is the same.

and BlahNotifier.moveBlah with:

  List<String> moveBlah({required int oldIndex, required int newIndex}) {
    final newBlahs = [...state.blahs];
    final movedBlah = newBlahs.removeAt(oldIndex);
    final insertionIndex = newIndex > oldIndex ? newIndex - 1 : newIndex;
    newBlahs.insert(insertionIndex, movedBlah);
    state = BlahState(
      blahs: newBlahs,
      otherStuff: state.otherStuff,
    );
    return newBlahs;
  }
}

Is this something that you would like to look into, or am I doing things wrong with the intermediate provider?

[wujek-srujek]

Actually it seems using such 'intermediate' providers is breaking things in my app. I have another case where I employ this idea, and I had a minor bug I couldn't figure out but which was 100% reproducible (again, only in release mode) - the code looked right, but it didn't work (an event didn't arrive when it should and the UI wasn't rebuilt). I was using a similar 'intermediate' provider there and I've just gotten rid of it for fun, and guess what, the bug is magically gone, the code works as expected. (I won't be able to create a sample for this one as it is just way too many things involved, including animations etc.)

[KrisBraun]

The core issue is that Riverpod updates asynchronously. ReorderableListView re-renders after calling onReorder but before Riverpod has updated.

I found the simplest solution is to wrap ReorderableListView with a stateful component that caches the list, allowing it to be updated in onReorder and then overwritten when Riverpod updates. (I'm actually using Bloc, but found this similar issue and my solution should apply here, too.)

import 'package:flutter/material.dart';

typedef ListItemWidgetBuilder<T> = Widget Function(
    BuildContext context, T item);

class CachedReorderableListView<T> extends StatefulWidget {
  const CachedReorderableListView(
      {required this.list,
      required this.itemBuilder,
      required this.onReorder,
      super.key});

  final List<T> list;
  final ListItemWidgetBuilder<T> itemBuilder;
  final ReorderCallback onReorder;

  @override
  CachedReorderableListViewState<T> createState() =>
      CachedReorderableListViewState<T>();
}

class CachedReorderableListViewState<T>
    extends State<CachedReorderableListView<T>> {
  late List<T> list;

  @override
  void initState() {
    list = [...widget.list];
    super.initState();
  }

  @override
  void didUpdateWidget(covariant CachedReorderableListView<T> oldWidget) {
    if (widget.list != oldWidget.list) {
      list = [...widget.list];
    }
    super.didUpdateWidget(oldWidget);
  }

  void updateList(List<T> value) {
    setState(() {
      list = value;
    });
  }

  @override
  Widget build(BuildContext context) {
    return ReorderableListView.builder(
      itemCount: list.length,
      itemBuilder: (context, index) => widget.itemBuilder(context, list[index]),
      onReorder: (int oldIndex, int newIndex) {
        setState(() {
          if (oldIndex < newIndex) {
            newIndex -= 1;
          }
          final item = list.removeAt(oldIndex);
          list.insert(newIndex, item);
        });
        widget.onReorder(oldIndex, newIndex);
      },
    );
  }
}

// Then in your widget
CachedReorderableListView(
  onReorder: (int oldIndex, int newIndex) async {
    // Trigger your Riverpod update
  },
  list: yourList,
  itemBuilder: (context, item) {
    // render a list item
  },
);

[mcmah309]

This worked for me. Wish there was a better solution though. It would be nice to have this in a package on pub.dev as a wrapper around the entire ReorderableListView widget api

[mcmah309]

widget.onReorder(oldIndex, newIndex); should be called before setState. Since the callback in setState is executed immediately and may modify newIndex before widget.onReorder is called.