From ba0d91a1ff26326e4a6ba7ed4dbfcab2f5b915b4 Mon Sep 17 00:00:00 2001 From: Thibault Deckers Date: Sat, 14 Sep 2024 00:47:15 +0200 Subject: [PATCH] entry hero review --- lib/widgets/collection/grid/tile.dart | 6 ++---- lib/widgets/common/thumbnail/image.dart | 10 ++++++---- lib/widgets/map/scroller.dart | 3 ++- lib/widgets/viewer/entry_viewer_stack.dart | 8 ++++---- lib/widgets/viewer/hero.dart | 11 +++++++---- lib/widgets/viewer/visual/entry_page_view.dart | 4 ++-- 6 files changed, 23 insertions(+), 19 deletions(-) diff --git a/lib/widgets/collection/grid/tile.dart b/lib/widgets/collection/grid/tile.dart index 4e8824d0a..d6b31b519 100644 --- a/lib/widgets/collection/grid/tile.dart +++ b/lib/widgets/collection/grid/tile.dart @@ -8,6 +8,7 @@ import 'package:aves/widgets/collection/grid/list_details_theme.dart'; import 'package:aves/widgets/common/grid/scaling.dart'; import 'package:aves/widgets/common/thumbnail/decorated.dart'; import 'package:aves/widgets/common/thumbnail/notifications.dart'; +import 'package:aves/widgets/viewer/hero.dart'; import 'package:aves_model/aves_model.dart'; import 'package:flutter/material.dart'; import 'package:provider/provider.dart'; @@ -62,10 +63,7 @@ class InteractiveTile extends StatelessWidget { selectable: true, highlightable: true, isScrollingNotifier: isScrollingNotifier, - // hero tag should include a collection identifier, so that it animates - // between different views of the entry in the same collection (e.g. thumbnails <-> viewer) - // but not between different collection instances, even with the same attributes (e.g. reloading collection page via drawer) - heroTagger: () => Object.hashAll([collection.id, entry.id]), + heroTagger: () => EntryHeroInfo(collection, entry).tag, ), ), ); diff --git a/lib/widgets/common/thumbnail/image.dart b/lib/widgets/common/thumbnail/image.dart index 25f6c7f68..0760c109e 100644 --- a/lib/widgets/common/thumbnail/image.dart +++ b/lib/widgets/common/thumbnail/image.dart @@ -261,11 +261,12 @@ class _ThumbnailImageState extends State { }, ); - if (animate && widget.heroTag != null) { + final heroTag = widget.heroTag; + if (animate && heroTag != null) { final background = settings.imageBackground; final backgroundColor = background.isColor ? background.color : null; image = Hero( - tag: widget.heroTag!, + tag: heroTag, flightShuttleBuilder: (flight, animation, direction, fromHero, toHero) { Widget child = TransitionImage( image: entry.bestCachedThumbnail, @@ -296,9 +297,10 @@ class _ThumbnailImageState extends State { extent: extent, ); - if (animate && widget.heroTag != null) { + final heroTag = widget.heroTag; + if (animate && heroTag != null) { child = Hero( - tag: widget.heroTag!, + tag: heroTag, flightShuttleBuilder: (flight, animation, direction, fromHero, toHero) { return MediaQueryDataProvider( child: DefaultTextStyle( diff --git a/lib/widgets/map/scroller.dart b/lib/widgets/map/scroller.dart index 5bb89661f..9e8befb6b 100644 --- a/lib/widgets/map/scroller.dart +++ b/lib/widgets/map/scroller.dart @@ -6,6 +6,7 @@ import 'package:aves/widgets/common/extensions/build_context.dart'; import 'package:aves/widgets/common/identity/empty.dart'; import 'package:aves/widgets/common/thumbnail/scroller.dart'; import 'package:aves/widgets/map/info_row.dart'; +import 'package:aves/widgets/viewer/hero.dart'; import 'package:flutter/material.dart'; class MapEntryScroller extends StatefulWidget { @@ -85,7 +86,7 @@ class _MapEntryScrollerState extends State { entryBuilder: (index) => index < regionEntries.length ? regionEntries[index] : null, indexNotifier: widget.selectedIndexNotifier, onTap: widget.onTap, - heroTagger: (entry) => Object.hashAll([regionCollection?.id, entry.id]), + heroTagger: (entry) => EntryHeroInfo(regionCollection, entry).tag, highlightable: true, showLocation: false, ); diff --git a/lib/widgets/viewer/entry_viewer_stack.dart b/lib/widgets/viewer/entry_viewer_stack.dart index 3a09636a5..4d4e4ccfa 100644 --- a/lib/widgets/viewer/entry_viewer_stack.dart +++ b/lib/widgets/viewer/entry_viewer_stack.dart @@ -75,7 +75,7 @@ class _EntryViewerStackState extends State with EntryViewContr late Animation _overlayTopOffset; EdgeInsets? _frozenViewInsets, _frozenViewPadding; late VideoActionDelegate _videoActionDelegate; - final ValueNotifier _heroInfoNotifier = ValueNotifier(null); + final ValueNotifier _heroInfoNotifier = ValueNotifier(null); bool _isEntryTracked = true; Timer? _overlayHidingTimer, _appInactiveReactionTimer; @@ -116,7 +116,7 @@ class _EntryViewerStackState extends State with EntryViewContr final initialEntry = widget.initialEntry; final entry = entries.firstWhereOrNull((entry) => entry.id == initialEntry.id) ?? entries.firstOrNull; // opening hero, with viewer as target - _heroInfoNotifier.value = HeroInfo(collection?.id, entry); + _heroInfoNotifier.value = EntryHeroInfo(collection, entry); entryNotifier = viewerController.entryNotifier; entryNotifier.value = entry; _currentEntryIndex = max(0, entry != null ? entries.indexOf(entry) : -1); @@ -224,7 +224,7 @@ class _EntryViewerStackState extends State with EntryViewContr _onPopInvoked(); }, - child: ValueListenableProvider.value( + child: ValueListenableProvider.value( value: _heroInfoNotifier, child: NotificationListener( onNotification: _handleNotification, @@ -867,7 +867,7 @@ class _EntryViewerStackState extends State with EntryViewContr } // closing hero, with viewer as source - final heroInfo = HeroInfo(collection?.id, entryNotifier.value); + final heroInfo = EntryHeroInfo(collection, entryNotifier.value); if (_heroInfoNotifier.value != heroInfo) { _heroInfoNotifier.value = heroInfo; // we post closing the viewer page so that hero animation source is ready diff --git a/lib/widgets/viewer/hero.dart b/lib/widgets/viewer/hero.dart index 3e9c19f9f..d1432d3c7 100644 --- a/lib/widgets/viewer/hero.dart +++ b/lib/widgets/viewer/hero.dart @@ -1,17 +1,20 @@ import 'package:aves/model/entry/entry.dart'; +import 'package:aves/model/source/collection_lens.dart'; import 'package:equatable/equatable.dart'; import 'package:flutter/widgets.dart'; @immutable -class HeroInfo extends Equatable { +class EntryHeroInfo extends Equatable { // hero tag should include a collection identifier, so that it animates // between different views of the entry in the same collection (e.g. thumbnails <-> viewer) // but not between different collection instances, even with the same attributes (e.g. reloading collection page via drawer) - final int? collectionId; + final CollectionLens? collection; final AvesEntry? entry; @override - List get props => [collectionId, entry?.uri]; + List get props => [collection?.id, entry?.uri]; - const HeroInfo(this.collectionId, this.entry); + const EntryHeroInfo(this.collection, this.entry); + + int get tag => Object.hashAll([collection?.id, entry?.uri]); } diff --git a/lib/widgets/viewer/visual/entry_page_view.dart b/lib/widgets/viewer/visual/entry_page_view.dart index 5f6bf112e..6098b07ec 100644 --- a/lib/widgets/viewer/visual/entry_page_view.dart +++ b/lib/widgets/viewer/visual/entry_page_view.dart @@ -150,9 +150,9 @@ class _EntryPageViewState extends State with TickerProviderStateM final animate = context.select((v) => v.animate); if (animate) { - child = Consumer( + child = Consumer( builder: (context, info, child) => Hero( - tag: info != null && info.entry == mainEntry ? Object.hashAll([info.collectionId, mainEntry.id]) : hashCode, + tag: info != null && info.entry == mainEntry ? info.tag : hashCode, transitionOnUserGestures: true, child: child!, ),