From 15be3437bf8e889589331a09938e3b477d0394a0 Mon Sep 17 00:00:00 2001 From: shenlong <139912620+shenlong-tanwen@users.noreply.github.com> Date: Tue, 1 Jul 2025 20:53:20 +0530 Subject: [PATCH] fix: timeline service uninitialised across routes (#19544) --- mobile/lib/pages/common/tab_shell.page.dart | 9 +- .../pages/dev/local_timeline.page.dart | 4 +- .../pages/dev/main_timeline.page.dart | 22 +-- .../pages/dev/remote_timeline.page.dart | 4 +- .../home_bottom_app_bar.widget.dart | 3 +- .../widgets/timeline/timeline.widget.dart | 164 ++++++++---------- .../infrastructure/action.provider.dart | 8 +- .../infrastructure/timeline.provider.dart | 14 +- .../timeline/multiselect.provider.dart | 6 +- 9 files changed, 95 insertions(+), 139 deletions(-) diff --git a/mobile/lib/pages/common/tab_shell.page.dart b/mobile/lib/pages/common/tab_shell.page.dart index aa5b695999..31ccb12392 100644 --- a/mobile/lib/pages/common/tab_shell.page.dart +++ b/mobile/lib/pages/common/tab_shell.page.dart @@ -4,11 +4,11 @@ import 'package:flutter/material.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:immich_mobile/extensions/build_context_extensions.dart'; import 'package:immich_mobile/providers/asset_viewer/scroll_notifier.provider.dart'; -import 'package:immich_mobile/providers/multiselect.provider.dart'; -import 'package:immich_mobile/providers/search/search_input_focus.provider.dart'; -import 'package:immich_mobile/routing/router.dart'; import 'package:immich_mobile/providers/haptic_feedback.provider.dart'; +import 'package:immich_mobile/providers/search/search_input_focus.provider.dart'; import 'package:immich_mobile/providers/tab.provider.dart'; +import 'package:immich_mobile/providers/timeline/multiselect.provider.dart'; +import 'package:immich_mobile/routing/router.dart'; @RoutePage() class TabShellPage extends ConsumerWidget { @@ -138,7 +138,8 @@ class TabShellPage extends ConsumerWidget { ); } - final multiselectEnabled = ref.watch(multiselectProvider); + final multiselectEnabled = + ref.watch(multiSelectProvider.select((s) => s.isEnabled)); return AutoTabsRouter( routes: [ const MainTimelineRoute(), diff --git a/mobile/lib/presentation/pages/dev/local_timeline.page.dart b/mobile/lib/presentation/pages/dev/local_timeline.page.dart index 8c06a6b62a..3a98a81e9e 100644 --- a/mobile/lib/presentation/pages/dev/local_timeline.page.dart +++ b/mobile/lib/presentation/pages/dev/local_timeline.page.dart @@ -1,5 +1,3 @@ -import 'dart:async'; - import 'package:auto_route/auto_route.dart'; import 'package:flutter/widgets.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; @@ -20,7 +18,7 @@ class LocalTimelinePage extends StatelessWidget { (ref) { final timelineService = ref.watch(timelineFactoryProvider).localAlbum(albumId: albumId); - ref.onDispose(() => unawaited(timelineService.dispose())); + ref.onDispose(timelineService.dispose); return timelineService; }, ), diff --git a/mobile/lib/presentation/pages/dev/main_timeline.page.dart b/mobile/lib/presentation/pages/dev/main_timeline.page.dart index cb2fdacf39..090db4f6ba 100644 --- a/mobile/lib/presentation/pages/dev/main_timeline.page.dart +++ b/mobile/lib/presentation/pages/dev/main_timeline.page.dart @@ -1,10 +1,7 @@ -import 'dart:async'; - import 'package:auto_route/auto_route.dart'; import 'package:flutter/material.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:immich_mobile/presentation/widgets/timeline/timeline.widget.dart'; -import 'package:immich_mobile/providers/infrastructure/timeline.provider.dart'; @RoutePage() class MainTimelinePage extends ConsumerWidget { @@ -12,23 +9,6 @@ class MainTimelinePage extends ConsumerWidget { @override Widget build(BuildContext context, WidgetRef ref) { - return ProviderScope( - key: ref.watch(timelineUsersProvider).value != null - ? ValueKey(ref.watch(timelineUsersProvider).value) - : const ValueKey("main-timeline"), - overrides: [ - timelineServiceProvider.overrideWith( - (ref) { - final timelineUsers = - ref.watch(timelineUsersProvider).valueOrNull ?? []; - final timelineService = - ref.watch(timelineFactoryProvider).main(timelineUsers); - ref.onDispose(() => unawaited(timelineService.dispose())); - return timelineService; - }, - ), - ], - child: const Timeline(), - ); + return const Timeline(); } } diff --git a/mobile/lib/presentation/pages/dev/remote_timeline.page.dart b/mobile/lib/presentation/pages/dev/remote_timeline.page.dart index 4965359ad5..6568f0f74f 100644 --- a/mobile/lib/presentation/pages/dev/remote_timeline.page.dart +++ b/mobile/lib/presentation/pages/dev/remote_timeline.page.dart @@ -1,5 +1,3 @@ -import 'dart:async'; - import 'package:auto_route/auto_route.dart'; import 'package:flutter/widgets.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; @@ -21,7 +19,7 @@ class RemoteTimelinePage extends StatelessWidget { final timelineService = ref .watch(timelineFactoryProvider) .remoteAlbum(albumId: albumId); - ref.onDispose(() => unawaited(timelineService.dispose())); + ref.onDispose(timelineService.dispose); return timelineService; }, ), diff --git a/mobile/lib/presentation/widgets/bottom_app_bar/home_bottom_app_bar.widget.dart b/mobile/lib/presentation/widgets/bottom_app_bar/home_bottom_app_bar.widget.dart index 8559cc99ad..4e55dbb3db 100644 --- a/mobile/lib/presentation/widgets/bottom_app_bar/home_bottom_app_bar.widget.dart +++ b/mobile/lib/presentation/widgets/bottom_app_bar/home_bottom_app_bar.widget.dart @@ -30,8 +30,7 @@ class HomeBottomAppBar extends ConsumerWidget { return BaseBottomSheet( initialChildSize: 0.25, - minChildSize: 0.22, - expand: false, + shouldCloseOnMinExtent: false, actions: [ if (multiselect.isEnabled) const ShareActionButton(), if (multiselect.hasRemote) ...[ diff --git a/mobile/lib/presentation/widgets/timeline/timeline.widget.dart b/mobile/lib/presentation/widgets/timeline/timeline.widget.dart index 59c4c46135..fd0806cff0 100644 --- a/mobile/lib/presentation/widgets/timeline/timeline.widget.dart +++ b/mobile/lib/presentation/widgets/timeline/timeline.widget.dart @@ -43,26 +43,15 @@ class Timeline extends StatelessWidget { } } -class _SliverTimeline extends ConsumerStatefulWidget { +class _SliverTimeline extends StatefulWidget { const _SliverTimeline(); @override - ConsumerState createState() => _SliverTimelineState(); + State createState() => _SliverTimelineState(); } -class _SliverTimelineState extends ConsumerState<_SliverTimeline> { +class _SliverTimelineState extends State<_SliverTimeline> { final _scrollController = ScrollController(); - PersistentBottomSheetController? _bottomSheetController; - bool _isMultiSelectEnabled = false; - - @override - void initState() { - super.initState(); - ref.listenManual( - multiSelectProvider.select((s) => s.isEnabled), - _onMultiSelectChanged, - ); - } @override void dispose() { @@ -70,93 +59,80 @@ class _SliverTimelineState extends ConsumerState<_SliverTimeline> { super.dispose(); } - void _onMultiSelectChanged(bool? previous, bool current) { - if (context.mounted && current != _isMultiSelectEnabled) { - setState(() { - _isMultiSelectEnabled = current; - if (current) { - _bottomSheetController = showBottomSheet( - context: context, - builder: (_) => const HomeBottomAppBar(), - ); - _bottomSheetController?.closed.then((_) { - _bottomSheetController = null; - // Reset the multi-select state when the bottom sheet is closed - ref.read(multiSelectProvider.notifier).reset(); - }); - } else { - _bottomSheetController?.close(); - } - }); - } - } - @override Widget build(BuildContext _) { - final asyncSegments = ref.watch(timelineSegmentProvider); - final maxHeight = - ref.watch(timelineArgsProvider.select((args) => args.maxHeight)); + return Consumer( + builder: (context, ref, child) { + final asyncSegments = ref.watch(timelineSegmentProvider); + final maxHeight = + ref.watch(timelineArgsProvider.select((args) => args.maxHeight)); + final isMultiSelectEnabled = + ref.watch(multiSelectProvider.select((s) => s.isEnabled)); + return asyncSegments.widgetWhen( + onData: (segments) { + final childCount = (segments.lastOrNull?.lastIndex ?? -1) + 1; + final statusBarHeight = context.padding.top; + final totalAppBarHeight = statusBarHeight + kToolbarHeight; + const scrubberBottomPadding = 100.0; - return asyncSegments.widgetWhen( - onData: (segments) { - final childCount = (segments.lastOrNull?.lastIndex ?? -1) + 1; - final statusBarHeight = context.padding.top; - final totalAppBarHeight = statusBarHeight + kToolbarHeight; - const scrubberBottomPadding = 100.0; - - return PrimaryScrollController( - controller: _scrollController, - child: Stack( - children: [ - Scrubber( - layoutSegments: segments, - timelineHeight: maxHeight, - topPadding: totalAppBarHeight + 10, - bottomPadding: context.padding.bottom + scrubberBottomPadding, - child: CustomScrollView( - primary: true, - cacheExtent: maxHeight * 2, - slivers: [ - SliverAnimatedOpacity( - duration: Durations.medium1, - opacity: _isMultiSelectEnabled ? 0 : 1, - sliver: const ImmichSliverAppBar( - floating: true, - pinned: false, - snap: false, - ), + return PrimaryScrollController( + controller: _scrollController, + child: Stack( + children: [ + Scrubber( + layoutSegments: segments, + timelineHeight: maxHeight, + topPadding: totalAppBarHeight + 10, + bottomPadding: + context.padding.bottom + scrubberBottomPadding, + child: CustomScrollView( + primary: true, + cacheExtent: maxHeight * 2, + slivers: [ + SliverAnimatedOpacity( + duration: Durations.medium1, + opacity: isMultiSelectEnabled ? 0 : 1, + sliver: const ImmichSliverAppBar( + floating: true, + pinned: false, + snap: false, + ), + ), + _SliverSegmentedList( + segments: segments, + delegate: SliverChildBuilderDelegate( + (ctx, index) { + if (index >= childCount) return null; + final segment = segments.findByIndex(index); + return segment?.builder(ctx, index) ?? + const SizedBox.shrink(); + }, + childCount: childCount, + addAutomaticKeepAlives: false, + // We add repaint boundary around tiles, so skip the auto boundaries + addRepaintBoundaries: false, + ), + ), + const SliverPadding( + padding: EdgeInsets.only( + bottom: scrubberBottomPadding, + ), + ), + ], ), - _SliverSegmentedList( - segments: segments, - delegate: SliverChildBuilderDelegate( - (ctx, index) { - if (index >= childCount) return null; - final segment = segments.findByIndex(index); - return segment?.builder(ctx, index) ?? - const SizedBox.shrink(); - }, - childCount: childCount, - addAutomaticKeepAlives: false, - // We add repaint boundary around tiles, so skip the auto boundaries - addRepaintBoundaries: false, - ), - ), - const SliverPadding( - padding: EdgeInsets.only( - bottom: scrubberBottomPadding, - ), + ), + if (isMultiSelectEnabled) ...[ + const Positioned( + top: 60, + left: 25, + child: _MultiSelectStatusButton(), ), + const HomeBottomAppBar(), ], - ), + ], ), - if (_isMultiSelectEnabled) - const Positioned( - top: 60, - left: 25, - child: _MultiSelectStatusButton(), - ), - ], - ), + ); + }, ); }, ); diff --git a/mobile/lib/providers/infrastructure/action.provider.dart b/mobile/lib/providers/infrastructure/action.provider.dart index e18a2aeb1b..5d0db7b382 100644 --- a/mobile/lib/providers/infrastructure/action.provider.dart +++ b/mobile/lib/providers/infrastructure/action.provider.dart @@ -1,20 +1,18 @@ import 'package:flutter/material.dart'; import 'package:immich_mobile/constants/enums.dart'; import 'package:immich_mobile/domain/models/asset/base_asset.model.dart'; -import 'package:immich_mobile/providers/infrastructure/timeline.provider.dart'; -import 'package:immich_mobile/providers/multiselect.provider.dart'; import 'package:immich_mobile/providers/timeline/multiselect.provider.dart'; import 'package:immich_mobile/providers/user.provider.dart'; import 'package:immich_mobile/services/action.service.dart'; +import 'package:immich_mobile/services/timeline.service.dart'; import 'package:logging/logging.dart'; import 'package:riverpod_annotation/riverpod_annotation.dart'; final actionProvider = NotifierProvider( ActionNotifier.new, dependencies: [ - actionServiceProvider, + multiSelectProvider, timelineServiceProvider, - multiselectProvider, ], ); @@ -32,7 +30,7 @@ class ActionResult { class ActionNotifier extends Notifier { final Logger _logger = Logger('ActionNotifier'); - late final ActionService _service; + late ActionService _service; ActionNotifier() : super(); diff --git a/mobile/lib/providers/infrastructure/timeline.provider.dart b/mobile/lib/providers/infrastructure/timeline.provider.dart index 0bf8b1b143..bbc04349de 100644 --- a/mobile/lib/providers/infrastructure/timeline.provider.dart +++ b/mobile/lib/providers/infrastructure/timeline.provider.dart @@ -15,9 +15,17 @@ final timelineArgsProvider = Provider.autoDispose( throw UnimplementedError('Will be overridden through a ProviderScope.'), ); -final timelineServiceProvider = Provider.autoDispose( - (ref) => - throw UnimplementedError('Will be overridden through a ProviderScope.'), +final timelineServiceProvider = Provider( + (ref) { + final timelineUsers = ref.watch(timelineUsersProvider).valueOrNull ?? []; + final timelineService = + ref.watch(timelineFactoryProvider).main(timelineUsers); + ref.onDispose(timelineService.dispose); + return timelineService; + }, + // Empty dependencies to inform the framework that this provider + // might be used in a ProviderScope + dependencies: [], ); final timelineFactoryProvider = Provider( diff --git a/mobile/lib/providers/timeline/multiselect.provider.dart b/mobile/lib/providers/timeline/multiselect.provider.dart index 2f2d76c68c..2b24a272bf 100644 --- a/mobile/lib/providers/timeline/multiselect.provider.dart +++ b/mobile/lib/providers/timeline/multiselect.provider.dart @@ -47,12 +47,10 @@ class MultiSelectState { } class MultiSelectNotifier extends Notifier { - late final TimelineService _timelineService; + TimelineService get _timelineService => ref.read(timelineServiceProvider); @override MultiSelectState build() { - _timelineService = ref.read(timelineServiceProvider); - return const MultiSelectState(selectedAssets: {}); } @@ -145,5 +143,5 @@ final bucketSelectionProvider = Provider.family>( // Check if all assets in the bucket are selected return bucketAssets.every((asset) => selectedAssets.contains(asset)); }, - dependencies: [multiSelectProvider], + dependencies: [multiSelectProvider, timelineServiceProvider], );