lightbox: Prevent hero animation between message lists

Fixes: #930

Author: lakshya1goel

Diff for: lib/widgets/content.dart

@@ -20,6 +20,7 @@ import 'dialog.dart';
 import 'icons.dart';
 import 'lightbox.dart';
 import 'message_list.dart';
+import 'page.dart';
 import 'poll.dart';
 import 'store.dart';
 import 'text.dart';
@@ -638,6 +639,7 @@ class MessageImage extends StatelessWidget {
     // TODO image hover animation
     final srcUrl = node.srcUrl;
     final thumbnailUrl = node.thumbnailUrl;
+    final pageContext = PageRoot.contextOf(context);
     final store = PerAccountStoreWidget.of(context);
     final resolvedSrcUrl = store.tryResolveUrl(srcUrl);
     final resolvedThumbnailUrl = thumbnailUrl == null
@@ -653,12 +655,14 @@ class MessageImage extends StatelessWidget {
           src: resolvedSrcUrl,
           thumbnailUrl: resolvedThumbnailUrl,
           originalWidth: node.originalWidth,
-          originalHeight: node.originalHeight));
+          originalHeight: node.originalHeight,
+          pageContext: pageContext));
       },
       child: node.loading
         ? const CupertinoActivityIndicator()
         : resolvedSrcUrl == null ? null : LightboxHero(
             message: message,
+            pageContext: pageContext,
             src: resolvedSrcUrl,
             child: RealmContentNetworkImage(
               resolvedThumbnailUrl ?? resolvedSrcUrl,

Diff for: lib/widgets/lightbox.dart

@@ -21,20 +21,22 @@ import 'store.dart';
 //   fly to an image preview with a different URL, following a message edit
 //   while the lightbox was open.
 class _LightboxHeroTag {
-  _LightboxHeroTag({required this.messageId, required this.src});
+  _LightboxHeroTag({required this.messageId, required this.src, required this.pageContext});
 
   final int messageId;
   final Uri src;
+  final BuildContext pageContext;
 
   @override
   bool operator ==(Object other) {
     return other is _LightboxHeroTag &&
       other.messageId == messageId &&
-      other.src == src;
+      other.src == src &&
+      other.pageContext == pageContext;
   }
 
   @override
-  int get hashCode => Object.hash('_LightboxHeroTag', messageId, src);
+  int get hashCode => Object.hash('_LightboxHeroTag', messageId, src, pageContext);
 }
 
 /// Builds a [Hero] from an image in the message list to the lightbox page.
@@ -44,16 +46,18 @@ class LightboxHero extends StatelessWidget {
     required this.message,
     required this.src,
     required this.child,
+    required this.pageContext,
   });
 
   final Message message;
   final Uri src;
   final Widget child;
+  final BuildContext pageContext;
 
   @override
   Widget build(BuildContext context) {
     return Hero(
-      tag: _LightboxHeroTag(messageId: message.id, src: src),
+      tag: _LightboxHeroTag(messageId: message.id, src: src, pageContext: pageContext),
       flightShuttleBuilder: (
         BuildContext flightContext,
         Animation<double> animation,
@@ -230,6 +234,7 @@ class _ImageLightboxPage extends StatefulWidget {
     required this.thumbnailUrl,
     required this.originalWidth,
     required this.originalHeight,
+    required this.pageContext,
   });
 
   final Animation<double> routeEntranceAnimation;
@@ -238,6 +243,7 @@ class _ImageLightboxPage extends StatefulWidget {
   final Uri? thumbnailUrl;
   final double? originalWidth;
   final double? originalHeight;
+  final BuildContext pageContext;
 
   @override
   State<_ImageLightboxPage> createState() => _ImageLightboxPageState();
@@ -319,6 +325,7 @@ class _ImageLightboxPageState extends State<_ImageLightboxPage> {
             child: LightboxHero(
               message: widget.message,
               src: widget.src,
+              pageContext: widget.pageContext,
               child: RealmContentNetworkImage(widget.src,
                 filterQuality: FilterQuality.medium,
                 frameBuilder: _frameBuilder,
@@ -603,6 +610,7 @@ Route<void> getImageLightboxRoute({
   required Uri? thumbnailUrl,
   required double? originalWidth,
   required double? originalHeight,
+  required BuildContext pageContext,
 }) {
   return _getLightboxRoute(
     accountId: accountId,
@@ -614,7 +622,8 @@ Route<void> getImageLightboxRoute({
         src: src,
         thumbnailUrl: thumbnailUrl,
         originalWidth: originalWidth,
-        originalHeight: originalHeight);
+        originalHeight: originalHeight,
+        pageContext: pageContext);
     });
 }
 

Diff for: test/widgets/lightbox_test.dart

@@ -13,6 +13,7 @@ import 'package:zulip/model/localizations.dart';
 import 'package:zulip/widgets/app.dart';
 import 'package:zulip/widgets/content.dart';
 import 'package:zulip/widgets/lightbox.dart';
+import 'package:zulip/widgets/page.dart';
 
 import '../example_data.dart' as eg;
 import '../model/binding.dart';
@@ -207,14 +208,19 @@ void main() {
       addTearDown(testBinding.reset);
       await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
 
-      // ZulipApp instead of TestZulipApp because we need the navigator to push
-      // the lightbox route. The lightbox page works together with the route;
-      // it takes the route's entrance animation.
-      await tester.pumpWidget(const ZulipApp());
+      // ZulipApp instead of TestZulipApp because we need:
+      // 1. The navigator to push the lightbox route. The lightbox page works
+      //    together with the route; it takes the route's entrance animation.
+      // 2. The PageRoot widget to provide context for Hero animations between
+      //    the message list and lightbox.
+      await tester.pumpWidget(PageRoot(
+        child: const ZulipApp()
+      ));
       await tester.pump();
       final navigator = await ZulipApp.navigator;
       unawaited(navigator.push(getImageLightboxRoute(
         accountId: eg.selfAccount.id,
+        pageContext: PageRoot.contextOf(navigator.context),
         message: message ?? eg.streamMessage(),
         src: src,
         thumbnailUrl: thumbnailUrl,
@@ -286,6 +292,48 @@ void main() {
       debugNetworkImageHttpClientProvider = null;
     });
 
+    testWidgets('LightboxHero with different pageContext does not share hero animation', (tester) async {
+      final message = eg.streamMessage();
+      final src = Uri.parse('https://chat.example/image.png');
+      await tester.pumpWidget(MaterialApp(
+        home: Scaffold(
+          body: Row(
+            children: [
+              Builder(builder: (context1) {
+                return LightboxHero(
+                  message: message,
+                  src: src,
+                  pageContext: context1,
+                  child: const SizedBox(width: 50, height: 50),
+                );
+              }),
+              Builder(builder: (context2) {
+                return LightboxHero(
+                  message: message,
+                  src: src,
+                  pageContext: context2,
+                  child: const SizedBox(width: 50, height: 50),
+                );
+              }),
+            ],
+          ),
+        ),
+      ));
+
+      final heroes = tester.widgetList<Hero>(find.byType(Hero)).toList();
+      expect(heroes.length, 2);
+
+      final heroTag1 = heroes[0].tag;
+      final heroTag2 = heroes[1].tag;
+      expect(heroTag1, isNot(equals(heroTag2)));
+
+      final tag1 = heroTag1 as dynamic;
+      final tag2 = heroTag2 as dynamic;
+      expect(tag1.messageId, equals(tag2.messageId));
+      expect(tag1.src, equals(tag2.src));
+      expect(tag1.pageContext, isNot(equals(tag2.pageContext)));
+    });
+
     // TODO test _CopyLinkButton
     // TODO test thumbnail gets shown, then gets replaced when main image loads
     // TODO test image is scaled down to fit, but not up