fix(mobile): Timeline grid not resizing properly on device rotation #24573
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,6 +29,24 @@ import 'package:immich_mobile/widgets/common/immich_sliver_app_bar.dart'; | |
| import 'package:immich_mobile/widgets/common/mesmerizing_sliver_app_bar.dart'; | ||
| import 'package:immich_mobile/widgets/common/selection_sliver_app_bar.dart'; | ||
|
|
||
| final _runtimeTimelineArgsProvider = StateProvider<TimelineArgs>((ref) { | ||
| return const TimelineArgs(maxWidth: 0, maxHeight: 0); | ||
| }); | ||
|
|
||
| class _TimelineRowAnchor { | ||
| final int rowIndex; | ||
| final double deltaPx; | ||
|
|
||
| const _TimelineRowAnchor({required this.rowIndex, required this.deltaPx}); | ||
|
|
||
| @override | ||
| String toString() => '_TimelineRowAnchor(rowIndex: $rowIndex, deltaPx: $deltaPx)'; | ||
| } | ||
|
|
||
| final _timelineAnchorRowProvider = StateProvider<_TimelineRowAnchor?>((ref) => null); | ||
|
|
||
| final _timelinePendingRestoreRowAnchorProvider = StateProvider<_TimelineRowAnchor?>((ref) => null); | ||
|
|
||
| class Timeline extends StatelessWidget { | ||
| const Timeline({ | ||
| super.key, | ||
|
|
@@ -61,29 +79,48 @@ class Timeline extends StatelessWidget { | |
| resizeToAvoidBottomInset: false, | ||
| floatingActionButton: const DownloadStatusFloatingButton(), | ||
| body: LayoutBuilder( | ||
| builder: (_, constraints) { | ||
| return ProviderScope( | ||
| overrides: [timelineArgsProvider.overrideWith((ref) => ref.watch(_runtimeTimelineArgsProvider))], | ||
| child: Consumer( | ||
| builder: (context, ref, _) { | ||
| final columnCount = ref.watch(settingsProvider.select((s) => s.get(Setting.tilesPerRow))); | ||
| final desired = TimelineArgs( | ||
| maxWidth: constraints.maxWidth, | ||
| maxHeight: constraints.maxHeight, | ||
| columnCount: columnCount, | ||
| showStorageIndicator: showStorageIndicator, | ||
| withStack: withStack, | ||
| groupBy: groupBy, | ||
| ); | ||
| final current = ref.watch(_runtimeTimelineArgsProvider); | ||
|
|
||
| if (current != desired) { | ||
| final rowAnchor = ref.read(_timelineAnchorRowProvider); | ||
| WidgetsBinding.instance.addPostFrameCallback((_) { | ||
| final latest = ref.read(_runtimeTimelineArgsProvider); | ||
| if (latest != desired) { | ||
| ref.read(_runtimeTimelineArgsProvider.notifier).state = desired; | ||
| } | ||
| if (rowAnchor != null) { | ||
| ref.read(_timelinePendingRestoreRowAnchorProvider.notifier).state = rowAnchor; | ||
| } | ||
| }); | ||
| } | ||
|
Comment on lines
+85
to
+109
Member
There was a problem hiding this comment. Why'd we need all these complex rebuild flows? All we need to do is to rebuild the widget on orientation state while retaining the anchor. It should actually be possible with just a single provider that stores the required anchor before rebuild and restores it after the change. Can you look into simplifying all the provider logic? Or can you explain why we need each of the providers? |
||
|
|
||
| return _SliverTimeline( | ||
| topSliverWidget: topSliverWidget, | ||
| topSliverWidgetHeight: topSliverWidgetHeight, | ||
| appBar: appBar, | ||
| bottomSheet: bottomSheet, | ||
| withScrubber: withScrubber, | ||
| snapToMonth: snapToMonth, | ||
| initialScrollOffset: initialScrollOffset, | ||
| ); | ||
| }, | ||
| ), | ||
| ); | ||
| }, | ||
| ), | ||
| ); | ||
| } | ||
|
|
@@ -126,13 +163,22 @@ class _SliverTimelineState extends ConsumerState<_SliverTimeline> { | |
| double _scaleFactor = 3.0; | ||
| double _baseScaleFactor = 3.0; | ||
| int? _scaleRestoreAssetIndex; | ||
| _TimelineRowAnchor? _pendingRestoreRowAnchor; | ||
| bool _hasPendingRowAnchorRestore = false; | ||
|
|
||
| @override | ||
| void initState() { | ||
| super.initState(); | ||
| _scrollController = ScrollController( | ||
| initialScrollOffset: widget.initialScrollOffset ?? 0.0, | ||
| onAttach: (position) { | ||
| _scrollController.addListener(_onScroll); | ||
| _restoreScalePosition(position); | ||
| _restoreRowAnchor(); | ||
| }, | ||
| onDetach: (position) { | ||
| _scrollController.removeListener(_onScroll); | ||
| }, | ||
| ); | ||
| _eventSubscription = EventStream.shared.listen(_onEvent); | ||
|
|
||
|
|
@@ -142,6 +188,75 @@ class _SliverTimelineState extends ConsumerState<_SliverTimeline> { | |
| _baseScaleFactor = _scaleFactor; | ||
|
|
||
| ref.listenManual(multiSelectProvider.select((s) => s.isEnabled), _onMultiSelectionToggled); | ||
|
|
||
| ref.listenManual(_timelinePendingRestoreRowAnchorProvider, (_, next) { | ||
| if (next == null) return; | ||
| _pendingRestoreRowAnchor = next; | ||
| _hasPendingRowAnchorRestore = true; | ||
| _restoreRowAnchor(); | ||
| ref.read(_timelinePendingRestoreRowAnchorProvider.notifier).state = null; | ||
| }); | ||
|
|
||
| ref.listenManual(timelineSegmentProvider.select((async) => async.valueOrNull), (previous, next) { | ||
| if (previous == null || next == null) return; | ||
|
|
||
| if (previous.equals(next)) return; | ||
|
|
||
| final currentAnchor = ref.read(_timelineAnchorRowProvider); | ||
| if (currentAnchor == null) return; | ||
|
|
||
| if (next.isEmpty) return; | ||
|
|
||
| final targetSegment = next.findByIndex(currentAnchor.rowIndex); | ||
| if (targetSegment == null) { | ||
| final lastSegment = next.lastOrNull; | ||
| if (lastSegment == null) return; | ||
| final clampedRowIndex = currentAnchor.rowIndex.clamp(0, lastSegment.lastIndex); | ||
| final fallbackSegment = next.findByIndex(clampedRowIndex); | ||
| if (fallbackSegment == null) return; | ||
| final targetOffset = fallbackSegment.indexToLayoutOffset(clampedRowIndex) + currentAnchor.deltaPx; | ||
| WidgetsBinding.instance.addPostFrameCallback((_) { | ||
| if (!mounted || !_scrollController.hasClients) return; | ||
| final max = _scrollController.position.maxScrollExtent; | ||
| final clamped = targetOffset.clamp(0.0, max); | ||
| _scrollController.jumpTo(clamped); | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| final targetOffset = targetSegment.indexToLayoutOffset(currentAnchor.rowIndex) + currentAnchor.deltaPx; | ||
| WidgetsBinding.instance.addPostFrameCallback((_) { | ||
| if (!mounted || !_scrollController.hasClients) return; | ||
| final max = _scrollController.position.maxScrollExtent; | ||
| final clamped = targetOffset.clamp(0.0, max); | ||
| final currentOffset = _scrollController.offset; | ||
| if ((clamped - currentOffset).abs() > 1.0) { | ||
| _scrollController.jumpTo(clamped); | ||
| } | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| _TimelineRowAnchor? _computeRowAnchor(List<Segment> segments, double scrollOffset) { | ||
| final segment = segments.findByOffset(scrollOffset) ?? segments.lastOrNull; | ||
| if (segment == null) return null; | ||
| final rowIndex = segment.getMinChildIndexForScrollOffset(scrollOffset); | ||
| final rowOffset = segment.indexToLayoutOffset(rowIndex); | ||
| final deltaPx = (scrollOffset - rowOffset).clamp(0.0, double.infinity); | ||
| return _TimelineRowAnchor(rowIndex: rowIndex, deltaPx: deltaPx); | ||
| } | ||
|
|
||
| void _onScroll() { | ||
| if (!_scrollController.hasClients) return; | ||
| final scrollOffset = _scrollController.offset; | ||
| final asyncSegments = ref.read(timelineSegmentProvider); | ||
| asyncSegments.whenData((segments) { | ||
| if (segments.isEmpty) return; | ||
| final rowAnchor = _computeRowAnchor(segments, scrollOffset); | ||
| if (rowAnchor != null) { | ||
| ref.read(_timelineAnchorRowProvider.notifier).state = rowAnchor; | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| void _onEvent(Event event) { | ||
|
|
@@ -187,6 +302,45 @@ class _SliverTimelineState extends ConsumerState<_SliverTimeline> { | |
| _scaleRestoreAssetIndex = null; | ||
| } | ||
|
|
||
| void _restoreRowAnchor() { | ||
| if (_pendingRestoreRowAnchor == null || !_hasPendingRowAnchorRestore) return; | ||
|
|
||
| final asyncSegments = ref.read(timelineSegmentProvider); | ||
| asyncSegments.whenData((segments) { | ||
| if (segments.isEmpty) return; | ||
|
|
||
| final rowAnchor = _pendingRestoreRowAnchor!; | ||
| final targetSegment = segments.findByIndex(rowAnchor.rowIndex); | ||
| if (targetSegment == null) { | ||
| final lastSegment = segments.lastOrNull; | ||
| if (lastSegment == null) return; | ||
| final clampedRowIndex = rowAnchor.rowIndex.clamp(0, lastSegment.lastIndex); | ||
| final fallbackSegment = segments.findByIndex(clampedRowIndex); | ||
| if (fallbackSegment == null) return; | ||
| final targetOffset = fallbackSegment.indexToLayoutOffset(clampedRowIndex) + rowAnchor.deltaPx; | ||
| WidgetsBinding.instance.addPostFrameCallback((_) { | ||
| if (!mounted || !_scrollController.hasClients) return; | ||
| final max = _scrollController.position.maxScrollExtent; | ||
| final clamped = targetOffset.clamp(0.0, max); | ||
| _scrollController.jumpTo(clamped); | ||
| _hasPendingRowAnchorRestore = false; | ||
| _pendingRestoreRowAnchor = null; | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| final targetOffset = targetSegment.indexToLayoutOffset(rowAnchor.rowIndex) + rowAnchor.deltaPx; | ||
| WidgetsBinding.instance.addPostFrameCallback((_) { | ||
| if (!mounted || !_scrollController.hasClients) return; | ||
| final max = _scrollController.position.maxScrollExtent; | ||
| final clamped = targetOffset.clamp(0.0, max); | ||
| _scrollController.jumpTo(clamped); | ||
| _hasPendingRowAnchorRestore = false; | ||
| _pendingRestoreRowAnchor = null; | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| @override | ||
| void dispose() { | ||
| _scrollController.dispose(); | ||
|
|
@@ -331,6 +485,7 @@ class _SliverTimelineState extends ConsumerState<_SliverTimeline> { | |
| (isMultiSelectEnabled ? bottomSheetOpenModifier : 0); | ||
|
|
||
| final grid = CustomScrollView( | ||
| key: const PageStorageKey<String>('timeline-grid-scroll'), | ||
| primary: true, | ||
| physics: _scrollPhysics, | ||
| cacheExtent: maxHeight * 2, | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,4 @@ | ||
| import 'package:auto_route/auto_route.dart'; | ||
| import 'package:flutter/material.dart'; | ||
| import 'package:hooks_riverpod/hooks_riverpod.dart'; | ||
| import 'package:immich_mobile/models/memories/memory.model.dart'; | ||
|
|
@@ -23,26 +22,39 @@ class MemoryLane extends HookConsumerWidget { | |
| (memories) => memories != null | ||
| ? ConstrainedBox( | ||
| constraints: const BoxConstraints(maxHeight: 200), | ||
| child: ListView.builder( | ||
| key: const PageStorageKey<String>('memory-lane-scroll'), | ||
| scrollDirection: Axis.horizontal, | ||
| physics: const BouncingScrollPhysics(), | ||
| itemCount: memories.length, | ||
| itemBuilder: (ctx, memoryIndex) { | ||
| return Padding( | ||
| padding: const EdgeInsets.only(right: 8), | ||
| child: InkWell( | ||
| onTap: () { | ||
| ref.read(hapticFeedbackProvider.notifier).heavyImpact(); | ||
| if (memories[memoryIndex].assets.isNotEmpty) { | ||
| final asset = memories[memoryIndex].assets[0]; | ||
| ref.read(currentAssetProvider.notifier).set(asset); | ||
| if (asset.isVideo || asset.isMotionPhoto) { | ||
| ref.read(videoPlaybackValueProvider.notifier).reset(); | ||
| } | ||
| } | ||
| context.pushRoute(MemoryRoute(memories: memories, memoryIndex: memoryIndex)); | ||
| }, | ||
| child: Material( | ||
| elevation: 2, | ||
| color: Colors.black, | ||
| borderRadius: const BorderRadius.all(Radius.circular(12)), | ||
| child: SizedBox( | ||
| width: 205, | ||
| height: 200, | ||
| child: MemoryCard(index: memoryIndex, memory: memories[memoryIndex]), | ||
| ), | ||
| ), | ||
| ), | ||
| ); | ||
| }, | ||
| ), | ||
| ) | ||
| : const SizedBox(), | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self-review: CarouselView triggered a framework assertion during rotation ('haveDimensions == (_lastMetrics != null)')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A
ListViewis different fromCarouselView. We should be fixing the issue and not replace it with a different widget