From 9640d9f5d2b3cc2e0fab441401be54c0c03de4f5 Mon Sep 17 00:00:00 2001 From: Adam Gastineau Date: Thu, 8 Oct 2026 11:01:00 -0700 Subject: [PATCH] fix(mobile): prevent rapid toggling and rerendering of cells (#32200) * fix(mobile): prevent rapid toggling and rerendering of cells * Apply Shakil's fix --- .../widgets/timeline/fixed/segment.model.dart | 53 +++++++++--- .../timeline/fixed_segment_row_test.dart | 85 +++++++++++++++++++ 2 files changed, 125 insertions(+), 13 deletions(-) create mode 100644 mobile/test/presentation/widgets/timeline/fixed_segment_row_test.dart diff --git a/mobile/lib/presentation/widgets/timeline/fixed/segment.model.dart b/mobile/lib/presentation/widgets/timeline/fixed/segment.model.dart index 13abd664f3..b65684f2c9 100644 --- a/mobile/lib/presentation/widgets/timeline/fixed/segment.model.dart +++ b/mobile/lib/presentation/widgets/timeline/fixed/segment.model.dart @@ -106,7 +106,6 @@ class _FixedSegmentRow extends ConsumerWidget { @override Widget build(BuildContext context, WidgetRef ref) { - final recommendDeferredLoading = ref.watch(timelineStateProvider.select((s) => s.recommendDeferredLoading)); final timelineService = ref.watch(timelineServiceProvider); final isDynamicLayout = columnCount <= (context.isMobile ? 2 : 3); @@ -119,18 +118,20 @@ class _FixedSegmentRow extends ConsumerWidget { ); } - if (recommendDeferredLoading) { - return _buildPlaceholder(context); - } - - return FutureBuilder>( - future: timelineService.loadAssets(assetIndex, assetCount), - builder: (context, snapshot) { - if (snapshot.connectionState != ConnectionState.done) { - return _buildPlaceholder(context); - } - return _buildAssetRow(context, snapshot.requireData, timelineService, isDynamicLayout); - }, + // Purposefully created outside of the FutureBuilder so it's created only once + late final assets = timelineService.loadAssets(assetIndex, assetCount); + return _DeferredRowLoader( + key: ValueKey(assetIndex), + placeholder: _buildPlaceholder, + builder: (context) => FutureBuilder>( + future: assets, + builder: (context, snapshot) { + if (snapshot.connectionState != ConnectionState.done) { + return _buildPlaceholder(context); + } + return _buildAssetRow(context, snapshot.requireData, timelineService, isDynamicLayout); + }, + ), ); } @@ -199,6 +200,32 @@ class _FixedSegmentRow extends ConsumerWidget { } } +/// Lock to displaying placeholders while the timeline is quickly scrolling +/// Automatically stops watching the deferred loading flag once the row has started loading, preventing ever rebuilding the view and losing state +class _DeferredRowLoader extends ConsumerStatefulWidget { + final WidgetBuilder placeholder; + final WidgetBuilder builder; + + const _DeferredRowLoader({super.key, required this.placeholder, required this.builder}); + + @override + ConsumerState<_DeferredRowLoader> createState() => _DeferredRowLoaderState(); +} + +class _DeferredRowLoaderState extends ConsumerState<_DeferredRowLoader> { + bool _isLoading = false; + + @override + Widget build(BuildContext context) { + if (!_isLoading && ref.watch(timelineStateProvider.select((state) => state.recommendDeferredLoading))) { + return widget.placeholder(context); + } + + _isLoading = true; + return widget.builder(context); + } +} + class _AssetTileWidget extends ConsumerWidget { final BaseAsset asset; final int assetIndex; diff --git a/mobile/test/presentation/widgets/timeline/fixed_segment_row_test.dart b/mobile/test/presentation/widgets/timeline/fixed_segment_row_test.dart new file mode 100644 index 0000000000..df4cd73da6 --- /dev/null +++ b/mobile/test/presentation/widgets/timeline/fixed_segment_row_test.dart @@ -0,0 +1,85 @@ +import 'dart:math' as math; + +import 'package:auto_route/auto_route.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_mobile/domain/models/asset/base_asset.model.dart'; +import 'package:immich_mobile/domain/models/config/app_config.dart'; +import 'package:immich_mobile/domain/models/timeline.model.dart'; +import 'package:immich_mobile/domain/services/timeline.service.dart'; +import 'package:immich_mobile/presentation/widgets/images/thumbnail_tile.widget.dart'; +import 'package:immich_mobile/presentation/widgets/timeline/timeline.state.dart'; +import 'package:immich_mobile/presentation/widgets/timeline/timeline.widget.dart'; +import 'package:immich_mobile/providers/infrastructure/settings.provider.dart'; +import 'package:immich_mobile/providers/infrastructure/timeline.provider.dart'; + +import '../../../fixtures/asset.stub.dart'; + +void main() { + testWidgets('should not reload row data when toggling recommendDeferredLoading', (tester) async { + final assets = List.generate(2000, (i) => LocalAssetStub.image1.copyWith(id: 'a$i')); + final service = TimelineService(( + assetSource: (i, n) async => assets.sublist(i, math.min(i + n, assets.length)), + bucketSource: () => Stream.value([TimeBucket(date: DateTime(2025), assetCount: assets.length)]), + origin: TimelineOrigin.main, + )); + addTearDown(service.dispose); + + final container = ProviderContainer( + overrides: [ + timelineServiceProvider.overrideWithValue(service), + appConfigProvider.overrideWithValue(const AppConfig()), + ], + ); + addTearDown(container.dispose); + + final router = RootStackRouter.build( + routes: [ + AutoRoute( + initial: true, + page: PageInfo( + 'Timeline', + builder: (_) => const Timeline( + withScrubber: false, + readOnly: true, + groupBy: GroupAssetsBy.none, + appBar: SliverToBoxAdapter(child: SizedBox.shrink()), + ), + ), + ), + ], + ); + + await tester.pumpWidget( + UncontrolledProviderScope( + container: container, + child: MaterialApp.router(routerConfig: router.config()), + ), + ); + + // Segment stream resolves, then the first buffer + await tester.pump(); + await tester.pump(); + + // Start past the first buffer + tester + .state(find.descendant(of: find.byType(Timeline), matching: find.byType(Scrollable)).first) + .position + .jumpTo(90000); + + await tester.pump(const Duration(milliseconds: 200)); + await tester.pump(); + + // Thumbnails will fail to load + tester.takeException(); + + final drawn = find.byType(ThumbnailTile).evaluate().length; + expect(drawn, greaterThan(0)); + + container.read(timelineStateProvider.notifier).setRecommendDeferredLoading(true); + await tester.pump(); + + expect(find.byType(ThumbnailTile), findsNWidgets(drawn)); + }); +}