Avoiding jerky scrolling in ListView caused by resizing... is my approach bad?

Help Widgets
1

I have a ListView of widgets that depend on asynchronous Riverpod providers for their content. These widgets show a CircularProgressIndicator while the AsyncValue is loading, and renders the full widget when it has a value. I don’t know the final height of the widget ahead of time.

The ListView uses a builder, so when items are scrolled off screen, they are unloaded, and rebuilt when scrolled back into view. This triggers the widget to go through the process of waiting on the AsyncValue again, showing the CircularProgressIndicator, which temporarily makes the widget shorter. When this happens, the ListView “jerks”.

In order to prevent this, I have created a widget that pays attention to its own size, caches the tallest that it’s ever been, and wraps its child in a Container with a minHeight constraint. This seems to work very well!

There are some potential gotchas:

  • Doesn’t work if the CircularProgressIndicator is taller than the widget’s actual contents (not a problem for my use case).
  • The height cache value needs to be evicted if the widget contents change in a way that would make it smaller (pretty straightforward but I haven’t done this yet).
  • The widget needs a GlobalKey to work, and I have to make sure I keep using the same GlobalKey after widgets are rebuilt (also pretty straightforward).

I’m an experienced developer but relatively new to Flutter and Dart. Are there any other reasons why this might be a bad idea that I don’t know about? Or any different approaches that might be more idiomatically correct?

Thanks for looking! Here’s the code for the widget:

import 'package:flutter/material.dart';

/// A widget that ensures its child will always be at least as tall as it
/// ever was.
class NeverShrinkWidget extends StatelessWidget {
  const NeverShrinkWidget({super.key, required this.child});

  final Widget child;

  static final Map<GlobalKey, double> _maxHeights = {};

  void _recordMaxHeight() {
    if (key is! GlobalKey) return;
    final globalKey = key as GlobalKey;

    final keyContext = globalKey.currentContext;
    if (keyContext == null || !keyContext.mounted) return;
    final box = keyContext.findRenderObject() as RenderBox;

    if (!_maxHeights.containsKey(globalKey) ||
        _maxHeights[globalKey]! < box.size.height) {
      _maxHeights[globalKey] = box.size.height;
    }
  }

  double? _getMaxHeight() {
    if (key is! GlobalKey) return null;
    final globalKey = key as GlobalKey;
    return _maxHeights[globalKey];
  }

  @override
  Widget build(BuildContext context) {
    return NotificationListener<SizeChangedLayoutNotification>(
      onNotification: (notification) {
        WidgetsBinding.instance.addPostFrameCallback((_) => _recordMaxHeight());
        return false;
      },
      child: Container(
        constraints: BoxConstraints(minHeight: _getMaxHeight() ?? 0),
        child: SizeChangedLayoutNotifier(
          child: child,
        ),
      ),
    );
  }
}
3 Likes
2

It’s a fascinating problem and an ingenious solution. At a glance, I don’t see anything in particular wrong with the code (Aside from the fact that I would prefer a ConstrainedBox over a Container.)

Are there any specific issues you’re concerned about with this implementation, or simply looking for a “best practices”?

3

Thanks for the response!

Good call on ConstrainedBox - I also just noticed that I ought to be returning true from the onNotification callback.

But yeah, I suppose I want to make sure I’m adhering to best practices and not creating any antipatterns, like:

  • is storing the cache as a static Map on this class reasonable, or should that be state stored somewhere else for Reasons™? This felt right because I don’t need it anywhere else for the moment.
  • what about the way I’m using addPostFrameCallback to record the max height? I’ve seen others use Future.delayed(Duration.zero, () {...}) in essentially the same way.

More generally, it feels like one of those times where I’m trying to be more clever than is healthy, and I’ve learned the hard way to take a closer look when that happens. :wink:

4

Since you’re not modifying the state (on account of it being a stateless widget), storing your keys in a map isn’t likely to cause any undesirable outcomes. I think that’s fine. The postframe callback should be preferred over the Future call. (You likely see the future call from less experienced developers.)

Nothing about your solution immediately gives me red flags, so I’d proceed with it. If you’re concerned about unnecessary rebuilds and performance, I would recommend using the performance analyzer in the DevTools. I don’t anticipate you’ll find anything too crazy with this solution but you never know.

1 Like
5

Great! Thank you so much for lending your experience!

1 Like
6

It was my distinct pleasure! Let me know if you have any other questions.

1 Like