[State Restoration] Fortnightly app #390
Conversation
lib/layout/image_placeholder.dart
Outdated
| super.initState(); | ||
| // Once the image is loaded, we no longer need to show a placeholder | ||
| widget.image | ||
| .resolve(ImageConfiguration.empty) |
There was a problem hiding this comment.
This will wait for the asset with the given configuration, which is not necessarily the one that ends up being shown on screen?
lib/layout/image_placeholder.dart
Outdated
| if (wasSynchronouslyLoaded || !kIsWeb) { | ||
| return this.child ?? child; | ||
| if (!_isLoaded) { |
There was a problem hiding this comment.
Why do we need the extra _isLoaded flag? How does it differ from wasSynchronouslyLoaded?
lib/layout/image_placeholder.dart
Outdated
| frameBuilder: (context, child, frame, wasSynchronouslyLoaded) { | ||
| // We only need to animate the loading of images on the web. We can | ||
| // also return early if the images are already in the cache. | ||
| if (wasSynchronouslyLoaded || !kIsWeb) { | ||
| return this.child ?? child; | ||
| if (!_isLoaded) { | ||
| return widget.placeholder; |
There was a problem hiding this comment.
Wouldn't we also want to switch from the placeholder to the actual image in an animated way here as well?
Is that because of the assets? Or is this a separate problem? |
lib/layout/image_placeholder.dart
Outdated
| if (!_isLoaded) { | ||
| return widget.placeholder; | ||
| } | ||
| return widget.child ?? child; |
There was a problem hiding this comment.
I think we can simplify this a lot and avoid the bug mentioned above where we resolve it for an incorrect configuration by removing the _isLoaded flag altogether and just changing this code to:
| if (!_isLoaded) { | |
| return widget.placeholder; | |
| } | |
| return widget.child ?? child; | |
| if (frame == null) { | |
| return widget.placeholder; | |
| } | |
| return widget.child ?? child; |
FadeInImagePlaceholder can be a StatelessWidget again after that.
Not sure if we want the transition from placeholder to real image to be animated here as well or not (I am leaning towards animation).
There was a problem hiding this comment.
If we want it animated, we could simply fo this:
if (wasSynchronouslyLoaded) {
return this.child ?? child;
} else {
return AnimatedSwitcher(
duration: duration,
child: frame != null ? this.child ?? child : placeholder,
);
}(And refactor the widget to be stateless again, of course).
There was a problem hiding this comment.
I originally tried what you suggested in the most recent comment, and when I did, it thought that it didn't work (it works perfectly with what you described).
I think what was happening is I was testing and adding state restoration while testing this part of the app and was getting confused -- Every time the app is killed and restored, the app ends up in the state at which flutter run was called, not including any of the changes I made to the gallery code afterwards.
shihaohong
changed the title
Just a few
ListViews. The horizontal ListView is fine, however, I noticed that the vertical ListView's scroll offset isn't properly restored. Also, the asset images flicker upon state restoration. A similar issue is happening for the Reply mini app as well.My guess is that the images need to be loaded when the app is restored, but at the first layout, they're not available and result in the offset being restored incorrectly.
If this is really the case, is there a way to wait for the images to load before building the ListView? I stumbled upon this flutter issue while looking for clues.Another solution could be to put a placeholder in place before the asset image is available, but it's weird because FadeInImagePlaceholder should already be doing this? I'm not too familiar with images so I'm not exactly sure what's going on here.
Edit: I ended up modifying the code so that it uses a placeholder when the asset image is not loaded yet. PTAL