From bdbb5cb27e4a2224e73b915e0d066d22091fdedd Mon Sep 17 00:00:00 2001 From: onli Date: Wed, 19 Jul 2023 09:02:58 +0200 Subject: [PATCH 1/6] Animate lightbox bars Wraps the AppBar and the BottomAppBar used on the lightbox screen with AnimatedContainers. Now, when toggling their visibility, they should not removed from the widget tree completely, but instead their height be changed. This achieves a slide-in animation. --- lib/widgets/lightbox.dart | 88 ++++++++++++++++++++++----------------- 1 file changed, 50 insertions(+), 38 deletions(-) diff --git a/lib/widgets/lightbox.dart b/lib/widgets/lightbox.dart index e68e15a64d..13c00c1b78 100644 --- a/lib/widgets/lightbox.dart +++ b/lib/widgets/lightbox.dart @@ -132,45 +132,57 @@ class _LightboxPageState extends State<_LightboxPage> { final appBarBackgroundColor = Colors.grey.shade900.withOpacity(0.87); const appBarForegroundColor = Colors.white; - PreferredSizeWidget? appBar; - if (_headerFooterVisible) { - // TODO(#45): Format with e.g. "Yesterday at 4:47 PM" - final timestampText = DateFormat - .yMMMd(/* TODO(i18n): Pass selected language here, I think? */) + // TODO(#45): Format with e.g. "Yesterday at 4:47 PM" + final timestampText = DateFormat.yMMMd( + /* TODO(i18n): Pass selected language here, I think? */) .add_Hms() - .format(DateTime.fromMillisecondsSinceEpoch(widget.message.timestamp * 1000)); - - appBar = AppBar( - centerTitle: false, - foregroundColor: appBarForegroundColor, - backgroundColor: appBarBackgroundColor, - - // TODO(#41): Show message author's avatar - title: RichText( - text: TextSpan(children: [ - TextSpan( - text: '${widget.message.senderFullName}\n', - - // Restate default - style: themeData.textTheme.titleLarge!.copyWith(color: appBarForegroundColor)), - TextSpan( - text: timestampText, - - // Make smaller, like a subtitle - style: themeData.textTheme.titleSmall!.copyWith(color: appBarForegroundColor)), - ]))); - } - - Widget? bottomAppBar; - if (_headerFooterVisible) { - bottomAppBar = BottomAppBar( - color: appBarBackgroundColor, - child: Row(children: [ - _CopyLinkButton(url: widget.src), - // TODO(#43): Share image - // TODO(#42): Download image - ])); - } + .format(DateTime.fromMillisecondsSinceEpoch( + widget.message.timestamp * 1000)); + + final appBar = PreferredSize( + preferredSize: Size(MediaQuery.of(context).size.width, kToolbarHeight), + child: AnimatedContainer( + duration: const Duration(milliseconds: 100), + curve: Curves.easeIn, + height: _headerFooterVisible ? kToolbarHeight : 0, + child: AppBar( + centerTitle: false, + foregroundColor: appBarForegroundColor, + backgroundColor: appBarBackgroundColor, + + // TODO(#41): Show message author's avatar + title: RichText( + text: TextSpan(children: [ + TextSpan( + text: '${widget.message.senderFullName}\n', + + // Restate default + style: themeData.textTheme.titleLarge! + .copyWith(color: appBarForegroundColor)), + TextSpan( + text: timestampText, + + // Make smaller, like a subtitle + style: themeData.textTheme.titleSmall! + .copyWith(color: appBarForegroundColor)), + ]))))); + + + final bottomAppBar = AnimatedContainer( + duration: const Duration(milliseconds: 100), + curve: Curves.easeIn, + // 80 is the default in M3, we need to set a value for the animation + // to work + height: _headerFooterVisible + ? BottomAppBarTheme.of(context).height ?? 80 + : 0, + child: BottomAppBar( + color: appBarBackgroundColor, + child: Row(children: [ + _CopyLinkButton(url: widget.src), + // TODO(#43): Share image + // TODO(#42): Download image + ]))); return Theme( data: themeData.copyWith( From b40db5335d9adea93545e1900e59c492d7643807 Mon Sep 17 00:00:00 2001 From: onli Date: Thu, 20 Jul 2023 00:41:44 +0200 Subject: [PATCH 2/6] Fix image appbar being cut off on Android/iOS --- lib/widgets/lightbox.dart | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/widgets/lightbox.dart b/lib/widgets/lightbox.dart index 13c00c1b78..c4dd65bdf9 100644 --- a/lib/widgets/lightbox.dart +++ b/lib/widgets/lightbox.dart @@ -144,7 +144,9 @@ class _LightboxPageState extends State<_LightboxPage> { child: AnimatedContainer( duration: const Duration(milliseconds: 100), curve: Curves.easeIn, - height: _headerFooterVisible ? kToolbarHeight : 0, + height: _headerFooterVisible ? AppBar.preferredHeightFor(context, + MediaQuery.of(context).size) + : 0, child: AppBar( centerTitle: false, foregroundColor: appBarForegroundColor, From 4dbaad65122f4df4056f3d16e14089e9779689ea Mon Sep 17 00:00:00 2001 From: onli Date: Sun, 23 Jul 2023 20:58:34 +0200 Subject: [PATCH 3/6] Tests for lightbox.dart Add tests that check existence of an image widget and for the animations --- lib/widgets/lightbox.dart | 12 +-- test/widgets/lightbox_test.dart | 149 ++++++++++++++++++++++++++++++++ 2 files changed, 155 insertions(+), 6 deletions(-) create mode 100644 test/widgets/lightbox_test.dart diff --git a/lib/widgets/lightbox.dart b/lib/widgets/lightbox.dart index c4dd65bdf9..655c13e7ce 100644 --- a/lib/widgets/lightbox.dart +++ b/lib/widgets/lightbox.dart @@ -81,8 +81,9 @@ class _CopyLinkButton extends StatelessWidget { } } -class _LightboxPage extends StatefulWidget { - const _LightboxPage({ +@visibleForTesting +class LightboxPage extends StatefulWidget { + const LightboxPage({ required this.routeEntranceAnimation, required this.message, required this.src, @@ -93,10 +94,10 @@ class _LightboxPage extends StatefulWidget { final String src; @override - State<_LightboxPage> createState() => _LightboxPageState(); + State createState() => _LightboxPageState(); } -class _LightboxPageState extends State<_LightboxPage> { +class _LightboxPageState extends State { // TODO(#38): Animate entrance/exit of header and footer bool _headerFooterVisible = false; @@ -128,7 +129,6 @@ class _LightboxPageState extends State<_LightboxPage> { @override Widget build(BuildContext context) { final themeData = Theme.of(context); - final appBarBackgroundColor = Colors.grey.shade900.withOpacity(0.87); const appBarForegroundColor = Colors.white; @@ -230,7 +230,7 @@ Route getLightboxRoute({ Animation secondaryAnimation, ) { // TODO(#40): Drag down to close? - return _LightboxPage(routeEntranceAnimation: animation, message: message, src: src); + return LightboxPage(routeEntranceAnimation: animation, message: message, src: src); }, transitionsBuilder: ( BuildContext context, diff --git a/test/widgets/lightbox_test.dart b/test/widgets/lightbox_test.dart new file mode 100644 index 0000000000..2e90fb0d95 --- /dev/null +++ b/test/widgets/lightbox_test.dart @@ -0,0 +1,149 @@ +import 'dart:async'; +import 'dart:io'; + +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:zulip/api/model/model.dart'; +import 'package:zulip/model/narrow.dart'; +import 'package:zulip/widgets/content.dart'; +import 'package:zulip/widgets/lightbox.dart'; +import 'package:zulip/widgets/store.dart'; + +import '../example_data.dart' as eg; +import '../model/binding.dart'; +import '../model/test_store.dart'; +import 'content_test.dart'; + +Future setupToMessageActionSheet(WidgetTester tester, { + required Message message, + required Narrow narrow, +}) async { + addTearDown(() { + TestZulipBinding.instance.reset(); + }); + + await TestZulipBinding.instance.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + final store = await TestZulipBinding.instance.globalStore.perAccount(eg.selfAccount.id); + store.addUser(eg.user(userId: message.senderId)); + + await tester.pumpWidget( + MaterialApp( + home: GlobalStoreWidget( + child: PerAccountStoreWidget( + accountId: eg.selfAccount.id, + child: LightboxPage( + message: message, + routeEntranceAnimation: const AlwaysStoppedAnimation(1), + src: "https://zulip.com/", + ))))); + + // global store, per-account store, and message list get loaded + await tester.pumpAndSettle(); +} + +void main() { + TestZulipBinding.ensureInitialized(); + + group('lightbox', () { + setUp(() { + final httpClient = _FakeHttpClient(); + debugNetworkImageHttpClientProvider = () => httpClient; + httpClient.request.response + ..statusCode = HttpStatus.ok + ..content = kSolidBlueAvatar; + }); + + testWidgets('tries to render an image', (WidgetTester tester) async { + await setupToMessageActionSheet(tester, message: eg.streamMessage(), narrow: StreamNarrow(eg.streamMessage().streamId)); + + expect(find.byType(RealmContentNetworkImage), findsOneWidget); + // unset the client here, otherwise the test will always fail + debugNetworkImageHttpClientProvider = null; + }); + + testWidgets('appbar is invisible at first', (WidgetTester tester) async { + await setupToMessageActionSheet(tester, message: eg.streamMessage(), narrow: StreamNarrow(eg.streamMessage().streamId)); + + final appBarFinder = find.byType(AppBar); + expect(appBarFinder, findsOneWidget); + expect(tester.getSize(appBarFinder).height, 0); + + // unset the client here, otherwise the test will always fail + debugNetworkImageHttpClientProvider = null; + }); + + testWidgets('appbar is visible after a time', (WidgetTester tester) async { + await setupToMessageActionSheet(tester, message: eg.streamMessage(), narrow: StreamNarrow(eg.streamMessage().streamId)); + + expect(find.byType(AppBar), findsOneWidget); + await tester.tap(find.byType(RealmContentNetworkImage)); + + await tester.pumpAndSettle(const Duration(milliseconds: 3000)); + expect(tester.getSize(find.byType(AppBar)).height, greaterThan(20)); + + // unset the client here, otherwise the test will always fail + debugNetworkImageHttpClientProvider = null; + }); + + testWidgets('appbar hides again after a time', (WidgetTester tester) async { + await setupToMessageActionSheet(tester, message: eg.streamMessage(), narrow: StreamNarrow(eg.streamMessage().streamId)); + + expect(find.byType(AppBar), findsOneWidget); + await tester.tap(find.byType(RealmContentNetworkImage)); + await tester.pumpAndSettle(const Duration(milliseconds: 3000)); + + await tester.tap(find.byType(RealmContentNetworkImage)); + await tester.pumpAndSettle(const Duration(milliseconds: 3000)); + + expect(tester.getSize(find.byType(AppBar)).height, 0); + + // unset the client here, otherwise the test will always fail + debugNetworkImageHttpClientProvider = null; + }); + }); +} + +class _FakeHttpClient extends Fake implements HttpClient { + final _FakeHttpClientRequest request = _FakeHttpClientRequest(); + + @override + Future getUrl(Uri url) async => request; +} + +class _FakeHttpClientRequest extends Fake implements HttpClientRequest { + final _FakeHttpClientResponse response = _FakeHttpClientResponse(); + + @override + final _FakeHttpHeaders headers = _FakeHttpHeaders(); + + @override + Future close() async => response; +} + +class _FakeHttpHeaders extends Fake implements HttpHeaders { + final Map> values = {}; + + @override + void add(String name, Object value, {bool preserveHeaderCase = false}) { + (values[name] ??= []).add(value.toString()); + } +} + +class _FakeHttpClientResponse extends Fake implements HttpClientResponse { + @override + int statusCode = HttpStatus.ok; + + late List content; + + @override + int get contentLength => content.length; + + @override + HttpClientResponseCompressionState get compressionState => HttpClientResponseCompressionState.notCompressed; + + @override + StreamSubscription> listen(void Function(List event)? onData, {Function? onError, void Function()? onDone, bool? cancelOnError}) { + return Stream.value(content).listen( + onData, onDone: onDone, onError: onError, cancelOnError: cancelOnError); + } +} From 853a69beafde7a32f2dcb9bbb6cc4e75273ce236 Mon Sep 17 00:00:00 2001 From: onli Date: Wed, 26 Jul 2023 20:55:30 +0200 Subject: [PATCH 4/6] Add test against notch intrusion into lighbox appbar --- test/widgets/lightbox_test.dart | 37 ++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/test/widgets/lightbox_test.dart b/test/widgets/lightbox_test.dart index 2e90fb0d95..31ae7610a9 100644 --- a/test/widgets/lightbox_test.dart +++ b/test/widgets/lightbox_test.dart @@ -31,11 +31,17 @@ Future setupToMessageActionSheet(WidgetTester tester, { home: GlobalStoreWidget( child: PerAccountStoreWidget( accountId: eg.selfAccount.id, - child: LightboxPage( - message: message, - routeEntranceAnimation: const AlwaysStoppedAnimation(1), - src: "https://zulip.com/", - ))))); + child: MediaQuery( + // This simulates the effect of a notch + data: const MediaQueryData(padding: EdgeInsets.only(top: 60), + size: Size(800, 600), + ), + child: LightboxPage( + message: message, + routeEntranceAnimation: const AlwaysStoppedAnimation(1), + src: "https://zulip.com/", + ), + ))))); // global store, per-account store, and message list get loaded await tester.pumpAndSettle(); @@ -100,6 +106,27 @@ void main() { // unset the client here, otherwise the test will always fail debugNetworkImageHttpClientProvider = null; }); + + testWidgets('appbar is visible despite notch', (WidgetTester tester) async { + await setupToMessageActionSheet(tester, message: eg.streamMessage(), narrow: StreamNarrow(eg.streamMessage().streamId)); + + expect(find.byType(AppBar), findsOneWidget); + await tester.tap(find.byType(RealmContentNetworkImage)); + await tester.pumpAndSettle(const Duration(milliseconds: 3000)); + + expect(tester.getSize(find.byType(AppBar)).height, greaterThan(20)); + + // This will fail if the appBar is obstructed by the notch + expect( + find.byWidgetPredicate((widget) => + widget is RichText && + widget.text.toPlainText().contains('A Person')).hitTestable(), + findsOneWidget + ); + + // unset the client here, otherwise the test will always fail + debugNetworkImageHttpClientProvider = null; + }); }); } From 79b6647f455a2918538d46433ee4904c4350e9fd Mon Sep 17 00:00:00 2001 From: onli Date: Wed, 26 Jul 2023 21:29:25 +0200 Subject: [PATCH 5/6] minor: formatting --- lib/widgets/lightbox.dart | 92 +++++++++++++++++++-------------------- 1 file changed, 44 insertions(+), 48 deletions(-) diff --git a/lib/widgets/lightbox.dart b/lib/widgets/lightbox.dart index 655c13e7ce..800d36c799 100644 --- a/lib/widgets/lightbox.dart +++ b/lib/widgets/lightbox.dart @@ -133,58 +133,54 @@ class _LightboxPageState extends State { const appBarForegroundColor = Colors.white; // TODO(#45): Format with e.g. "Yesterday at 4:47 PM" - final timestampText = DateFormat.yMMMd( - /* TODO(i18n): Pass selected language here, I think? */) - .add_Hms() - .format(DateTime.fromMillisecondsSinceEpoch( - widget.message.timestamp * 1000)); + final timestampText = DateFormat + .yMMMd(/* TODO(i18n): Pass selected language here, I think? */) + .add_Hms() + .format(DateTime.fromMillisecondsSinceEpoch(widget.message.timestamp * 1000)); final appBar = PreferredSize( - preferredSize: Size(MediaQuery.of(context).size.width, kToolbarHeight), - child: AnimatedContainer( - duration: const Duration(milliseconds: 100), - curve: Curves.easeIn, - height: _headerFooterVisible ? AppBar.preferredHeightFor(context, - MediaQuery.of(context).size) - : 0, - child: AppBar( - centerTitle: false, - foregroundColor: appBarForegroundColor, - backgroundColor: appBarBackgroundColor, - - // TODO(#41): Show message author's avatar - title: RichText( - text: TextSpan(children: [ - TextSpan( - text: '${widget.message.senderFullName}\n', - - // Restate default - style: themeData.textTheme.titleLarge! - .copyWith(color: appBarForegroundColor)), - TextSpan( - text: timestampText, - - // Make smaller, like a subtitle - style: themeData.textTheme.titleSmall! - .copyWith(color: appBarForegroundColor)), - ]))))); - - - final bottomAppBar = AnimatedContainer( + preferredSize: Size(MediaQuery.of(context).size.width, kToolbarHeight), + child: AnimatedContainer( duration: const Duration(milliseconds: 100), curve: Curves.easeIn, - // 80 is the default in M3, we need to set a value for the animation - // to work - height: _headerFooterVisible - ? BottomAppBarTheme.of(context).height ?? 80 - : 0, - child: BottomAppBar( - color: appBarBackgroundColor, - child: Row(children: [ - _CopyLinkButton(url: widget.src), - // TODO(#43): Share image - // TODO(#42): Download image - ]))); + height: _headerFooterVisible ? AppBar.preferredHeightFor(context, + MediaQuery.of(context).size) + : 0, + child: AppBar( + centerTitle: false, + foregroundColor: appBarForegroundColor, + backgroundColor: appBarBackgroundColor, + + // TODO(#41): Show message author's avatar + title: RichText( + text: TextSpan(children: [ + TextSpan( + text: '${widget.message.senderFullName}\n', + + // Restate default + style: themeData.textTheme.titleLarge!.copyWith(color: appBarForegroundColor)), + TextSpan( + text: timestampText, + + // Make smaller, like a subtitle + style: themeData.textTheme.titleSmall!.copyWith(color: appBarForegroundColor)), + ]))))); + + final bottomAppBar = AnimatedContainer( + duration: const Duration(milliseconds: 100), + curve: Curves.easeIn, + // 80 is the default in M3, we need to set a value for the animation + // to work + height: _headerFooterVisible + ? BottomAppBarTheme.of(context).height ?? 80 + : 0, + child: BottomAppBar( + color: appBarBackgroundColor, + child: Row(children: [ + _CopyLinkButton(url: widget.src), + // TODO(#43): Share image + // TODO(#42): Download image + ]))); return Theme( data: themeData.copyWith( From c7c3b1f434c74089a387f046976378dd9ada560e Mon Sep 17 00:00:00 2001 From: onli Date: Wed, 26 Jul 2023 23:00:05 +0200 Subject: [PATCH 6/6] Sync lightbox AppBar and BottomAppBar animation --- lib/widgets/lightbox.dart | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/widgets/lightbox.dart b/lib/widgets/lightbox.dart index 800d36c799..ea12f33872 100644 --- a/lib/widgets/lightbox.dart +++ b/lib/widgets/lightbox.dart @@ -143,14 +143,16 @@ class _LightboxPageState extends State { child: AnimatedContainer( duration: const Duration(milliseconds: 100), curve: Curves.easeIn, - height: _headerFooterVisible ? AppBar.preferredHeightFor(context, - MediaQuery.of(context).size) + height: _headerFooterVisible ? AppBar.preferredHeightFor( + context, + Size(0, MediaQuery.of(context).padding.top + kToolbarHeight) + ) : 0, child: AppBar( centerTitle: false, foregroundColor: appBarForegroundColor, backgroundColor: appBarBackgroundColor, - + // TODO(#41): Show message author's avatar title: RichText( text: TextSpan(children: [