From 9a846efdcf23c7f4360b637c6db0594558348034 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 11 Jul 2024 15:57:28 -0700 Subject: [PATCH 01/10] i18n: Add app_ar.arb file This will let us simplify how we simulate RTL in the emoji-reaction widget tests, coming up. Fixes: #803 --- assets/l10n/app_ar.arb | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 assets/l10n/app_ar.arb diff --git a/assets/l10n/app_ar.arb b/assets/l10n/app_ar.arb new file mode 100644 index 0000000000..0db3279e44 --- /dev/null +++ b/assets/l10n/app_ar.arb @@ -0,0 +1,3 @@ +{ + +} From 551c61e4b461a7854bdd6bd2e002185e0953ad19 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 11 Jul 2024 16:25:32 -0700 Subject: [PATCH 02/10] emoji_reaction test [nfc]: Refactor helper default params --- test/widgets/emoji_reaction_test.dart | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/widgets/emoji_reaction_test.dart b/test/widgets/emoji_reaction_test.dart index 7244a68f1d..9b58ef53a4 100644 --- a/test/widgets/emoji_reaction_test.dart +++ b/test/widgets/emoji_reaction_test.dart @@ -39,15 +39,15 @@ void main() { Future setupChipsInBox(WidgetTester tester, { required List reactions, - double? width, - TextDirection? textDirection, + double width = 245.0, // (seen in context on an iPhone 13 Pro) + TextDirection textDirection = TextDirection.ltr, }) async { final message = eg.streamMessage(reactions: reactions); await tester.pumpWidget( MaterialApp( home: Directionality( - textDirection: textDirection ?? TextDirection.ltr, + textDirection: textDirection, child: GlobalStoreWidget( child: PerAccountStoreWidget( accountId: eg.selfAccount.id, @@ -55,7 +55,7 @@ void main() { child: ColoredBox( color: Colors.white, child: SizedBox( - width: width ?? 245.0, // (seen in context on an iPhone 13 Pro) + width: width, child: ReactionChipsList( messageId: message.id, reactions: message.reactions!, From eaa2a68ed562e328775bc1f26b99fee7e865ddb0 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 11 Jul 2024 16:30:46 -0700 Subject: [PATCH 03/10] emoji_reaction test: Check that RTL layout takes effect in test setup We're about to refactor the way RTL gets applied. While we're at it, also check the width of the ReactionChipsList element. --- test/flutter_checks.dart | 10 ++++++++++ test/widgets/emoji_reaction_test.dart | 6 ++++++ 2 files changed, 16 insertions(+) diff --git a/test/flutter_checks.dart b/test/flutter_checks.dart index 0a3fbcba91..af66b031a5 100644 --- a/test/flutter_checks.dart +++ b/test/flutter_checks.dart @@ -111,3 +111,13 @@ extension TypographyChecks on Subject { extension InlineSpanChecks on Subject { Subject get style => has((x) => x.style, 'style'); } + +extension SizeChecks on Subject { + Subject get width => has((x) => x.width, 'width'); + Subject get height => has((x) => x.height, 'height'); +} + +extension ElementChecks on Subject { + Subject get size => has((t) => t.size, 'size'); + // TODO more +} diff --git a/test/widgets/emoji_reaction_test.dart b/test/widgets/emoji_reaction_test.dart index 9b58ef53a4..a9b8fb46c1 100644 --- a/test/widgets/emoji_reaction_test.dart +++ b/test/widgets/emoji_reaction_test.dart @@ -1,6 +1,7 @@ import 'dart:io' as io; import 'dart:io'; +import 'package:checks/checks.dart'; import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; @@ -13,6 +14,7 @@ import 'package:zulip/widgets/emoji_reaction.dart'; import 'package:zulip/widgets/store.dart'; import '../example_data.dart' as eg; +import '../flutter_checks.dart'; import '../model/binding.dart'; import '../model/test_store.dart'; import '../test_images.dart'; @@ -63,6 +65,10 @@ void main() { // global store, per-account store await tester.pumpAndSettle(); + + final reactionChipsList = tester.element(find.byType(ReactionChipsList)); + check(Directionality.of(reactionChipsList)).equals(textDirection); + check(reactionChipsList).size.isNotNull().width.equals(width); } // Smoke tests under various conditions. From a596789c58361698df03cf2328154729fc5b57d5 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 15 Jul 2024 15:52:35 -0700 Subject: [PATCH 04/10] test: Add TestZulipApp; replace uses of ZulipApp Greg noticed an instance where using ZulipApp directly was causing tests to slow down by some 75%: https://github.com/zulip/zulip-flutter/pull/805#discussion_r1675335632 Specifically, that was when we converted the emoji-reaction widget tests over to use ZulipApp instead of mimicking it with a MaterialApp, etc. It's helpful to use something approaching the real ZulipApp. But we'd like to avoid that slowdown, especially if it's happening in code that most tests don't need to exercise. So, here's a stripped-down version that does that. It takes roughly the same time as the MaterialApp approach in those emoji-reaction tests. (11s on my machine, vs. 16s with ZulipApp.) This commit just converts the tests that were using ZulipApp. Next, we'll sweep through and treat the ones using the ad hoc MaterialApp approach. --- test/notifications/display_test.dart | 2 ++ test/widgets/code_block_test.dart | 9 ++---- test/widgets/inbox_test.dart | 11 +++---- test/widgets/lightbox_test.dart | 3 ++ test/widgets/test_app.dart | 44 ++++++++++++++++++++++++++++ test/widgets/text_test.dart | 12 +++----- test/widgets/theme_test.dart | 10 ++----- 7 files changed, 63 insertions(+), 28 deletions(-) create mode 100644 test/widgets/test_app.dart diff --git a/test/notifications/display_test.dart b/test/notifications/display_test.dart index d14a81da41..33593b2744 100644 --- a/test/notifications/display_test.dart +++ b/test/notifications/display_test.dart @@ -340,6 +340,8 @@ void main() { pushedRoutes = []; final testNavObserver = TestNavigatorObserver() ..onPushed = (route, prevRoute) => pushedRoutes.add(route); + // This uses [ZulipApp] instead of [TestZulipApp] because notification + // logic uses `await ZulipApp.navigator`. await tester.pumpWidget(ZulipApp(navigatorObservers: [testNavObserver])); if (early) { check(pushedRoutes).isEmpty(); diff --git a/test/widgets/code_block_test.dart b/test/widgets/code_block_test.dart index 358fa185fa..5e7d657d1b 100644 --- a/test/widgets/code_block_test.dart +++ b/test/widgets/code_block_test.dart @@ -1,11 +1,10 @@ import 'package:checks/checks.dart'; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; -import 'package:zulip/widgets/app.dart'; import 'package:zulip/widgets/code_block.dart'; -import 'package:zulip/widgets/page.dart'; import '../model/binding.dart'; +import 'test_app.dart'; void main() { TestZulipBinding.ensureInitialized(); @@ -14,12 +13,8 @@ void main() { group('lerp', () { Future contextWithZulipTheme(WidgetTester tester) async { addTearDown(testBinding.reset); - await tester.pumpWidget(const ZulipApp()); + await tester.pumpWidget(const TestZulipApp()); await tester.pump(); - final navigator = await ZulipApp.navigator; - navigator.push(MaterialWidgetRoute(page: Builder( - builder: (context) => const Placeholder()))); - await tester.pumpAndSettle(); return tester.element(find.byType(Placeholder)); } diff --git a/test/widgets/inbox_test.dart b/test/widgets/inbox_test.dart index c8b658f96f..c9336efa32 100644 --- a/test/widgets/inbox_test.dart +++ b/test/widgets/inbox_test.dart @@ -4,7 +4,6 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:zulip/api/model/events.dart'; import 'package:zulip/api/model/model.dart'; import 'package:zulip/model/store.dart'; -import 'package:zulip/widgets/app.dart'; import 'package:zulip/widgets/icons.dart'; import 'package:zulip/widgets/inbox.dart'; import 'package:zulip/widgets/stream_colors.dart'; @@ -13,6 +12,7 @@ import '../example_data.dart' as eg; import '../flutter_checks.dart'; import '../model/binding.dart'; import '../model/test_store.dart'; +import 'test_app.dart'; /// Repeatedly drags `view` by `moveStep` until `finder` is invisible. /// @@ -71,11 +71,12 @@ void main() { await store.handleEvent(MessageEvent(id: 1, message: message)); } - await tester.pumpWidget( - ZulipApp(navigatorObservers: [if (navigatorObserver != null) navigatorObserver])); + await tester.pumpWidget(TestZulipApp( + accountId: eg.selfAccount.id, + navigatorObservers: [if (navigatorObserver != null) navigatorObserver], + child: const InboxPage(), + )); await tester.pump(); - final navigator = await ZulipApp.navigator; - navigator.push(InboxPage.buildRoute(accountId: eg.selfAccount.id)); // global store and per-account store get loaded await tester.pumpAndSettle(); diff --git a/test/widgets/lightbox_test.dart b/test/widgets/lightbox_test.dart index 419690f788..d3a62de14f 100644 --- a/test/widgets/lightbox_test.dart +++ b/test/widgets/lightbox_test.dart @@ -208,6 +208,9 @@ 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()); await tester.pump(); final navigator = await ZulipApp.navigator; diff --git a/test/widgets/test_app.dart b/test/widgets/test_app.dart new file mode 100644 index 0000000000..ebc1e209e4 --- /dev/null +++ b/test/widgets/test_app.dart @@ -0,0 +1,44 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_gen/gen_l10n/zulip_localizations.dart'; + +import 'package:zulip/widgets/store.dart'; +import 'package:zulip/widgets/theme.dart'; + +/// A lightweight mock of [ZulipApp], suitable for most widget tests. +class TestZulipApp extends StatelessWidget { + const TestZulipApp({ + super.key, + this.accountId, + this.navigatorObservers, + this.child = const Placeholder(), + }); + + final int? accountId; + + /// A list to pass through to [MaterialApp.navigatorObservers]. + final List? navigatorObservers; + + /// A [Widget] to render on the home page. + /// + /// If [accountId] is provided, this is wrapped in a [PerAccountStoreWidget]. + /// + /// Defaults to `Placeholder()`. + final Widget child; + + @override + Widget build(BuildContext context) { + return GlobalStoreWidget( + child: MaterialApp( + title: 'Zulip', + localizationsDelegates: ZulipLocalizations.localizationsDelegates, + supportedLocales: ZulipLocalizations.supportedLocales, + theme: zulipThemeData(context), + + navigatorObservers: navigatorObservers ?? const [], + + home: accountId != null + ? PerAccountStoreWidget(accountId: accountId!, child: child) + : child, + )); + } +} diff --git a/test/widgets/text_test.dart b/test/widgets/text_test.dart index 837f515124..f362186485 100644 --- a/test/widgets/text_test.dart +++ b/test/widgets/text_test.dart @@ -2,12 +2,11 @@ import 'package:checks/checks.dart'; import 'package:collection/collection.dart'; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; -import 'package:zulip/widgets/app.dart'; -import 'package:zulip/widgets/page.dart'; import 'package:zulip/widgets/text.dart'; import '../flutter_checks.dart'; import '../model/binding.dart'; +import 'test_app.dart'; // From trying the options on an iPhone 13 Pro running iOS 16.6.1: const kTextScaleFactors = [ @@ -397,14 +396,11 @@ void main() { addTearDown(tester.platformDispatcher.clearLocaleTestValue); addTearDown(tester.platformDispatcher.clearLocalesTestValue); - await tester.pumpWidget(const ZulipApp()); + await tester.pumpWidget(TestZulipApp( + child: Builder(builder: (context) => + Text('123', style: TextStyle(textBaseline: localizedTextBaseline(context)))))); await tester.pump(); - final navigator = await ZulipApp.navigator; - navigator.push(MaterialWidgetRoute(page: Builder(builder: (context) => - Text('123', style: TextStyle(textBaseline: localizedTextBaseline(context)))))); - await tester.pumpAndSettle(); - final TextStyle? style = tester.widget(find.text('123')).style; final actualTextBaseline = style!.textBaseline!; check(actualTextBaseline).equals(expected); diff --git a/test/widgets/theme_test.dart b/test/widgets/theme_test.dart index b6d3c98a68..5814c033c8 100644 --- a/test/widgets/theme_test.dart +++ b/test/widgets/theme_test.dart @@ -2,8 +2,6 @@ import 'package:checks/checks.dart'; import 'package:flutter/material.dart'; import 'package:flutter/rendering.dart'; import 'package:flutter_test/flutter_test.dart'; -import 'package:zulip/widgets/app.dart'; -import 'package:zulip/widgets/page.dart'; import 'package:zulip/widgets/stream_colors.dart'; import 'package:zulip/widgets/text.dart'; import 'package:zulip/widgets/theme.dart'; @@ -11,6 +9,7 @@ import 'package:zulip/widgets/theme.dart'; import '../example_data.dart' as eg; import '../flutter_checks.dart'; import '../model/binding.dart'; +import 'test_app.dart'; void main() { TestZulipBinding.ensureInitialized(); @@ -118,14 +117,9 @@ void main() { tester.platformDispatcher.platformBrightnessTestValue = Brightness.light; addTearDown(tester.platformDispatcher.clearPlatformBrightnessTestValue); - await tester.pumpWidget(const ZulipApp()); + await tester.pumpWidget(const TestZulipApp()); await tester.pump(); - final navigator = await ZulipApp.navigator; - navigator.push(MaterialWidgetRoute(page: Builder(builder: (context) => - const Placeholder()))); - await tester.pumpAndSettle(); - final element = tester.element(find.byType(Placeholder)); // Compares all the swatch's members; see [ColorSwatch]'s `operator ==`. check(colorSwatchFor(element, subscription)) From 2253978445f2e5af425cb218435d80cc697f316f Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 12 Jul 2024 17:07:57 -0700 Subject: [PATCH 05/10] test: Use TestZulipApp everywhere else it makes sense --- integration_test/unreadmarker_test.dart | 14 ++------- test/widgets/action_sheet_test.dart | 15 ++------- test/widgets/actions_test.dart | 17 +++------- test/widgets/app_test.dart | 11 ++----- test/widgets/autocomplete_test.dart | 21 +++---------- test/widgets/clipboard_test.dart | 22 ++++++------- test/widgets/compose_box_test.dart | 13 ++------ test/widgets/content_test.dart | 19 +++--------- test/widgets/emoji_reaction_test.dart | 30 ++++++++---------- test/widgets/lightbox_test.dart | 22 ++++++------- test/widgets/message_list_test.dart | 14 ++------- test/widgets/profile_test.dart | 18 +++-------- .../widgets/recent_dm_conversations_test.dart | 16 +++------- test/widgets/store_test.dart | 2 ++ test/widgets/subscription_list_test.dart | 8 ++--- test/widgets/text_test.dart | 31 +++++++++++-------- test/widgets/theme_test.dart | 23 ++++++-------- 17 files changed, 105 insertions(+), 191 deletions(-) diff --git a/integration_test/unreadmarker_test.dart b/integration_test/unreadmarker_test.dart index c13f83f36c..babc6cd3e8 100644 --- a/integration_test/unreadmarker_test.dart +++ b/integration_test/unreadmarker_test.dart @@ -1,5 +1,3 @@ - -import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:integration_test/integration_test.dart'; import 'package:zulip/api/model/events.dart'; @@ -7,13 +5,12 @@ import 'package:zulip/api/model/model.dart'; import 'package:zulip/model/narrow.dart'; import 'package:zulip/model/store.dart'; import 'package:zulip/widgets/message_list.dart'; -import 'package:zulip/widgets/page.dart'; -import 'package:zulip/widgets/store.dart'; import '../test/api/fake_api.dart'; import '../test/example_data.dart' as eg; import '../test/model/binding.dart'; import '../test/model/message_list_test.dart'; +import '../test/widgets/test_app.dart'; void main() { final binding = IntegrationTestWidgetsFlutterBinding.ensureInitialized(); @@ -34,13 +31,8 @@ void main() { connection.prepare(json: newestResult(foundOldest: true, messages: messages).toJson()); - await tester.pumpWidget( - MaterialApp( - home: GlobalStoreWidget( - child: PerAccountStoreWidget( - accountId: eg.selfAccount.id, - placeholder: const LoadingPlaceholderPage(), - child: const MessageListPage(narrow: CombinedFeedNarrow()))))); + await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, + child: const MessageListPage(narrow: CombinedFeedNarrow()))); await tester.pumpAndSettle(); return messages; } diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart index c0f970b643..9a1f102fac 100644 --- a/test/widgets/action_sheet_test.dart +++ b/test/widgets/action_sheet_test.dart @@ -3,7 +3,6 @@ import 'dart:convert'; import 'package:checks/checks.dart'; import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; -import 'package:flutter_gen/gen_l10n/zulip_localizations.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:http/http.dart' as http; import 'package:zulip/api/model/model.dart'; @@ -18,9 +17,7 @@ import 'package:zulip/widgets/compose_box.dart'; import 'package:zulip/widgets/content.dart'; import 'package:zulip/widgets/icons.dart'; import 'package:zulip/widgets/message_list.dart'; -import 'package:zulip/widgets/store.dart'; import 'package:share_plus_platform_interface/method_channel/method_channel_share.dart'; -import 'package:zulip/widgets/theme.dart'; import '../api/fake_api.dart'; import '../example_data.dart' as eg; @@ -32,6 +29,7 @@ import '../test_clipboard.dart'; import '../test_share_plus.dart'; import 'compose_box_checks.dart'; import 'dialog_checks.dart'; +import 'test_app.dart'; /// Simulates loading a [MessageListPage] and long-pressing on [message]. Future setupToMessageActionSheet(WidgetTester tester, { @@ -60,15 +58,8 @@ Future setupToMessageActionSheet(WidgetTester tester, { messages: [message], ).toJson()); - await tester.pumpWidget(Builder(builder: (context) => - MaterialApp( - theme: zulipThemeData(context), - localizationsDelegates: ZulipLocalizations.localizationsDelegates, - supportedLocales: ZulipLocalizations.supportedLocales, - home: GlobalStoreWidget( - child: PerAccountStoreWidget( - accountId: eg.selfAccount.id, - child: MessageListPage(narrow: narrow)))))); + await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, + child: MessageListPage(narrow: narrow))); // global store, per-account store, and message list get loaded await tester.pumpAndSettle(); diff --git a/test/widgets/actions_test.dart b/test/widgets/actions_test.dart index 759cfa4218..19eea4aa49 100644 --- a/test/widgets/actions_test.dart +++ b/test/widgets/actions_test.dart @@ -2,7 +2,6 @@ import 'dart:convert'; import 'package:checks/checks.dart'; import 'package:flutter/material.dart'; -import 'package:flutter_gen/gen_l10n/zulip_localizations.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:http/http.dart' as http; import 'package:zulip/api/model/initial_snapshot.dart'; @@ -12,8 +11,6 @@ import 'package:zulip/model/localizations.dart'; import 'package:zulip/model/narrow.dart'; import 'package:zulip/model/store.dart'; import 'package:zulip/widgets/actions.dart'; -import 'package:zulip/widgets/store.dart'; -import 'package:zulip/widgets/theme.dart'; import '../api/fake_api.dart'; import '../example_data.dart' as eg; @@ -21,6 +18,7 @@ import '../model/binding.dart'; import '../model/unreads_checks.dart'; import '../stdlib_checks.dart'; import 'dialog_checks.dart'; +import 'test_app.dart'; void main() { TestZulipBinding.ensureInitialized(); @@ -39,16 +37,9 @@ void main() { store = await testBinding.globalStore.perAccount(eg.selfAccount.id); connection = store.connection as FakeApiConnection; - await tester.pumpWidget(Builder(builder: (context) => - MaterialApp( - theme: zulipThemeData(context), - localizationsDelegates: ZulipLocalizations.localizationsDelegates, - supportedLocales: ZulipLocalizations.supportedLocales, - home: GlobalStoreWidget( - child: PerAccountStoreWidget( - accountId: eg.selfAccount.id, - child: const Scaffold( - body: Placeholder())))))); + await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, + child: const Scaffold(body: Placeholder()))); + // global store, per-account store get loaded await tester.pumpAndSettle(); context = tester.element(find.byType(Placeholder)); } diff --git a/test/widgets/app_test.dart b/test/widgets/app_test.dart index 775f827317..578fd4b3ed 100644 --- a/test/widgets/app_test.dart +++ b/test/widgets/app_test.dart @@ -1,18 +1,17 @@ import 'package:checks/checks.dart'; import 'package:flutter/material.dart'; -import 'package:flutter_gen/gen_l10n/zulip_localizations.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:zulip/model/database.dart'; import 'package:zulip/widgets/app.dart'; import 'package:zulip/widgets/inbox.dart'; import 'package:zulip/widgets/page.dart'; -import 'package:zulip/widgets/store.dart'; import '../example_data.dart' as eg; import '../flutter_checks.dart'; import '../model/binding.dart'; import '../test_navigation.dart'; import 'page_checks.dart'; +import 'test_app.dart'; void main() { TestZulipBinding.ensureInitialized(); @@ -67,12 +66,8 @@ void main() { .insertAccount(account.toCompanion(false)); } - await tester.pumpWidget( - const MaterialApp( - localizationsDelegates: ZulipLocalizations.localizationsDelegates, - supportedLocales: ZulipLocalizations.supportedLocales, - home: GlobalStoreWidget( - child: ChooseAccountPage()))); + await tester.pumpWidget(const TestZulipApp( + child: ChooseAccountPage())); // global store gets loaded await tester.pumpAndSettle(); diff --git a/test/widgets/autocomplete_test.dart b/test/widgets/autocomplete_test.dart index b3cd9173d7..b52eafb10f 100644 --- a/test/widgets/autocomplete_test.dart +++ b/test/widgets/autocomplete_test.dart @@ -1,6 +1,5 @@ import 'package:checks/checks.dart'; import 'package:flutter/material.dart'; -import 'package:flutter_gen/gen_l10n/zulip_localizations.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:zulip/api/model/model.dart'; import 'package:zulip/api/route/messages.dart'; @@ -8,14 +7,13 @@ import 'package:zulip/model/compose.dart'; import 'package:zulip/model/narrow.dart'; import 'package:zulip/model/store.dart'; import 'package:zulip/widgets/message_list.dart'; -import 'package:zulip/widgets/store.dart'; -import 'package:zulip/widgets/theme.dart'; import '../api/fake_api.dart'; import '../example_data.dart' as eg; import '../model/binding.dart'; import '../model/test_store.dart'; import '../test_images.dart'; +import 'test_app.dart'; /// Simulates loading a [MessageListPage] and tapping to focus the compose input. /// @@ -49,19 +47,10 @@ Future setupToComposeInput(WidgetTester tester, { prepareBoringImageHttpClient(); - await tester.pumpWidget(Builder(builder: (context) => - MaterialApp( - theme: zulipThemeData(context), - localizationsDelegates: ZulipLocalizations.localizationsDelegates, - supportedLocales: ZulipLocalizations.supportedLocales, - home: GlobalStoreWidget( - child: PerAccountStoreWidget( - accountId: eg.selfAccount.id, - child: MessageListPage( - narrow: DmNarrow( - allRecipientIds: [eg.selfUser.userId, eg.otherUser.userId], - selfUserId: eg.selfUser.userId, - ))))))); + await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, + child: MessageListPage(narrow: DmNarrow( + allRecipientIds: [eg.selfUser.userId, eg.otherUser.userId], + selfUserId: eg.selfUser.userId)))); // global store, per-account store, and message list get loaded await tester.pumpAndSettle(); diff --git a/test/widgets/clipboard_test.dart b/test/widgets/clipboard_test.dart index 9f9796220f..a1010e39ec 100644 --- a/test/widgets/clipboard_test.dart +++ b/test/widgets/clipboard_test.dart @@ -8,6 +8,7 @@ import 'package:zulip/widgets/clipboard.dart'; import '../flutter_checks.dart'; import '../model/binding.dart'; import '../test_clipboard.dart'; +import 'test_app.dart'; void main() { TestZulipBinding.ensureInitialized(); @@ -26,17 +27,16 @@ void main() { group('copyWithPopup', () { Future call(WidgetTester tester, {required String text}) async { - await tester.pumpWidget( - MaterialApp( - home: Scaffold( - body: Builder(builder: (context) => Center( - child: ElevatedButton( - onPressed: () async { - copyWithPopup(context: context, successContent: const Text('Text copied'), - data: ClipboardData(text: text)); - }, - child: const Text('Copy'))))), - )); + await tester.pumpWidget(TestZulipApp( + child: Scaffold( + body: Builder(builder: (context) => Center( + child: ElevatedButton( + onPressed: () async { + copyWithPopup(context: context, successContent: const Text('Text copied'), + data: ClipboardData(text: text)); + }, + child: const Text('Copy'))))))); + await tester.pump(); await tester.tap(find.text('Copy')); await tester.pump(); // copy await tester.pump(Duration.zero); // await platform info (awkwardly async) diff --git a/test/widgets/compose_box_test.dart b/test/widgets/compose_box_test.dart index 2bc9b2279f..ceba33565f 100644 --- a/test/widgets/compose_box_test.dart +++ b/test/widgets/compose_box_test.dart @@ -4,7 +4,6 @@ import 'package:checks/checks.dart'; import 'package:file_picker/file_picker.dart'; import 'package:http/http.dart' as http; import 'package:flutter/material.dart'; -import 'package:flutter_gen/gen_l10n/zulip_localizations.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:image_picker/image_picker.dart'; import 'package:zulip/api/route/messages.dart'; @@ -12,7 +11,6 @@ import 'package:zulip/model/localizations.dart'; import 'package:zulip/model/narrow.dart'; import 'package:zulip/model/store.dart'; import 'package:zulip/widgets/compose_box.dart'; -import 'package:zulip/widgets/store.dart'; import '../api/fake_api.dart'; import '../example_data.dart' as eg; @@ -20,6 +18,7 @@ import '../flutter_checks.dart'; import '../model/binding.dart'; import '../stdlib_checks.dart'; import 'dialog_checks.dart'; +import 'test_app.dart'; void main() { TestZulipBinding.ensureInitialized(); @@ -35,14 +34,8 @@ void main() { connection = store.connection as FakeApiConnection; final controllerKey = GlobalKey(); - await tester.pumpWidget( - MaterialApp( - localizationsDelegates: ZulipLocalizations.localizationsDelegates, - supportedLocales: ZulipLocalizations.supportedLocales, - home: GlobalStoreWidget( - child: PerAccountStoreWidget( - accountId: eg.selfAccount.id, - child: ComposeBox(controllerKey: controllerKey, narrow: narrow))))); + await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, + child: ComposeBox(controllerKey: controllerKey, narrow: narrow))); await tester.pumpAndSettle(); return controllerKey; diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index 4831bfa866..1b49b043e4 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -3,7 +3,6 @@ import 'package:flutter/cupertino.dart'; import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:flutter/rendering.dart'; -import 'package:flutter_gen/gen_l10n/zulip_localizations.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:url_launcher/url_launcher.dart'; import 'package:zulip/api/core.dart'; @@ -16,7 +15,6 @@ import 'package:zulip/widgets/message_list.dart'; import 'package:zulip/widgets/page.dart'; import 'package:zulip/widgets/store.dart'; import 'package:zulip/widgets/text.dart'; -import 'package:zulip/widgets/theme.dart'; import '../example_data.dart' as eg; import '../flutter_checks.dart'; @@ -29,6 +27,7 @@ import '../test_navigation.dart'; import 'dialog_checks.dart'; import 'message_list_checks.dart'; import 'page_checks.dart'; +import 'test_app.dart'; /// Simulate a nested "inner" span's style by merging all ancestor-span /// styles, starting from the root. @@ -113,26 +112,18 @@ void main() { List navObservers = const [], bool wrapWithPerAccountStoreWidget = false, }) async { - Widget widget = child; - if (wrapWithPerAccountStoreWidget) { await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); - widget = PerAccountStoreWidget(accountId: eg.selfAccount.id, child: widget); } - widget = GlobalStoreWidget(child: widget); addTearDown(testBinding.reset); prepareBoringImageHttpClient(); - await tester.pumpWidget( - Builder(builder: (context) => - MaterialApp( - theme: zulipThemeData(context), - localizationsDelegates: ZulipLocalizations.localizationsDelegates, - supportedLocales: ZulipLocalizations.supportedLocales, - navigatorObservers: navObservers, - home: widget))); + await tester.pumpWidget(TestZulipApp( + accountId: wrapWithPerAccountStoreWidget ? eg.selfAccount.id : null, + navigatorObservers: navObservers, + child: child)); await tester.pump(); // global store if (wrapWithPerAccountStoreWidget) { await tester.pump(); diff --git a/test/widgets/emoji_reaction_test.dart b/test/widgets/emoji_reaction_test.dart index a9b8fb46c1..11261c9ee8 100644 --- a/test/widgets/emoji_reaction_test.dart +++ b/test/widgets/emoji_reaction_test.dart @@ -11,13 +11,13 @@ import 'package:zulip/api/model/events.dart'; import 'package:zulip/api/model/model.dart'; import 'package:zulip/model/store.dart'; import 'package:zulip/widgets/emoji_reaction.dart'; -import 'package:zulip/widgets/store.dart'; import '../example_data.dart' as eg; import '../flutter_checks.dart'; import '../model/binding.dart'; import '../model/test_store.dart'; import '../test_images.dart'; +import 'test_app.dart'; import 'text_test.dart'; void main() { @@ -46,22 +46,18 @@ void main() { }) async { final message = eg.streamMessage(reactions: reactions); - await tester.pumpWidget( - MaterialApp( - home: Directionality( - textDirection: textDirection, - child: GlobalStoreWidget( - child: PerAccountStoreWidget( - accountId: eg.selfAccount.id, - child: Center( - child: ColoredBox( - color: Colors.white, - child: SizedBox( - width: width, - child: ReactionChipsList( - messageId: message.id, - reactions: message.reactions!, - ))))))))); + await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, + child: Directionality( + textDirection: textDirection, + child: Center( + child: ColoredBox( + color: Colors.white, + child: SizedBox( + width: width, + child: ReactionChipsList( + messageId: message.id, + reactions: message.reactions!, + ))))))); // global store, per-account store await tester.pumpAndSettle(); diff --git a/test/widgets/lightbox_test.dart b/test/widgets/lightbox_test.dart index d3a62de14f..1473c90287 100644 --- a/test/widgets/lightbox_test.dart +++ b/test/widgets/lightbox_test.dart @@ -3,7 +3,6 @@ import 'dart:async'; import 'package:checks/checks.dart'; import 'package:clock/clock.dart'; import 'package:flutter/services.dart'; -import 'package:flutter_gen/gen_l10n/zulip_localizations.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:flutter/material.dart'; import 'package:plugin_platform_interface/plugin_platform_interface.dart'; @@ -14,12 +13,12 @@ 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/store.dart'; import '../example_data.dart' as eg; import '../model/binding.dart'; import '../test_images.dart'; import 'dialog_checks.dart'; +import 'test_app.dart'; const kTestVideoUrl = "https://a/video.mp4"; const kTestUnsupportedVideoUrl = "https://a/unsupported.mp4"; @@ -300,7 +299,10 @@ void main() { for (final (duration, expected, title) in cases) { testWidgets('with $title shows $expected', (tester) async { - await tester.pumpWidget(MaterialApp(home: VideoDurationLabel(duration))); + addTearDown(testBinding.reset); + await tester.pumpWidget(TestZulipApp( + child: VideoDurationLabel(duration))); + await tester.pump(); final text = tester.widget(find.byType(Text)); check(text.data) ..equals(VideoDurationLabel.formatDuration(duration)) @@ -359,15 +361,11 @@ void main() { await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); addTearDown(platform.reset); - await tester.pumpWidget(GlobalStoreWidget(child: MaterialApp( - localizationsDelegates: ZulipLocalizations.localizationsDelegates, - supportedLocales: ZulipLocalizations.supportedLocales, - home: PerAccountStoreWidget( - accountId: eg.selfAccount.id, - child: VideoLightboxPage( - routeEntranceAnimation: kAlwaysCompleteAnimation, - message: eg.streamMessage(), - src: videoSrc))))); + await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, + child: VideoLightboxPage( + routeEntranceAnimation: kAlwaysCompleteAnimation, + message: eg.streamMessage(), + src: videoSrc))); await tester.pump(); // global store await tester.pump(); // per-account store await tester.pump(); // video controller initialization diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index c804b02af1..1156e7d64b 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -4,7 +4,6 @@ import 'package:checks/checks.dart'; import 'package:collection/collection.dart'; import 'package:flutter/material.dart'; import 'package:flutter/rendering.dart'; -import 'package:flutter_gen/gen_l10n/zulip_localizations.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:http/http.dart' as http; import 'package:zulip/api/model/events.dart'; @@ -21,7 +20,6 @@ import 'package:zulip/widgets/icons.dart'; import 'package:zulip/widgets/message_list.dart'; import 'package:zulip/widgets/store.dart'; import 'package:zulip/widgets/stream_colors.dart'; -import 'package:zulip/widgets/theme.dart'; import '../api/fake_api.dart'; import '../example_data.dart' as eg; @@ -34,6 +32,7 @@ import '../stdlib_checks.dart'; import '../test_images.dart'; import 'content_checks.dart'; import 'dialog_checks.dart'; +import 'test_app.dart'; void main() { TestZulipBinding.ensureInitialized(); @@ -68,15 +67,8 @@ void main() { connection.prepare(json: newestResult(foundOldest: foundOldest, messages: messages).toJson()); - await tester.pumpWidget(Builder(builder: (context) => - MaterialApp( - theme: zulipThemeData(context), - localizationsDelegates: ZulipLocalizations.localizationsDelegates, - supportedLocales: ZulipLocalizations.supportedLocales, - home: GlobalStoreWidget( - child: PerAccountStoreWidget( - accountId: eg.selfAccount.id, - child: MessageListPage(narrow: narrow)))))); + await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, + child: MessageListPage(narrow: narrow))); // global store, per-account store, and message list get loaded await tester.pumpAndSettle(); diff --git a/test/widgets/profile_test.dart b/test/widgets/profile_test.dart index f3243b47f8..55e90f6340 100644 --- a/test/widgets/profile_test.dart +++ b/test/widgets/profile_test.dart @@ -1,6 +1,5 @@ import 'package:checks/checks.dart'; import 'package:flutter/material.dart'; -import 'package:flutter_gen/gen_l10n/zulip_localizations.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:url_launcher/url_launcher.dart'; import 'package:zulip/api/model/initial_snapshot.dart'; @@ -10,8 +9,6 @@ import 'package:zulip/widgets/content.dart'; import 'package:zulip/widgets/message_list.dart'; import 'package:zulip/widgets/page.dart'; import 'package:zulip/widgets/profile.dart'; -import 'package:zulip/widgets/store.dart'; -import 'package:zulip/widgets/theme.dart'; import '../example_data.dart' as eg; import '../model/binding.dart'; @@ -20,6 +17,7 @@ import '../test_navigation.dart'; import 'message_list_checks.dart'; import 'page_checks.dart'; import 'profile_page_checks.dart'; +import 'test_app.dart'; Future setupPage(WidgetTester tester, { required int pageUserId, @@ -41,16 +39,10 @@ Future setupPage(WidgetTester tester, { await store.addUsers(users); } - await tester.pumpWidget( - GlobalStoreWidget(child: Builder(builder: (context) => - MaterialApp( - theme: zulipThemeData(context), - navigatorObservers: navigatorObserver != null ? [navigatorObserver] : [], - localizationsDelegates: ZulipLocalizations.localizationsDelegates, - supportedLocales: ZulipLocalizations.supportedLocales, - home: PerAccountStoreWidget( - accountId: eg.selfAccount.id, - child: ProfilePage(userId: pageUserId)))))); + await tester.pumpWidget(TestZulipApp( + accountId: eg.selfAccount.id, + navigatorObservers: navigatorObserver != null ? [navigatorObserver] : [], + child: ProfilePage(userId: pageUserId))); // global store, per-account store, and page get loaded await tester.pumpAndSettle(); diff --git a/test/widgets/recent_dm_conversations_test.dart b/test/widgets/recent_dm_conversations_test.dart index a5a214827d..69817ec5ce 100644 --- a/test/widgets/recent_dm_conversations_test.dart +++ b/test/widgets/recent_dm_conversations_test.dart @@ -1,7 +1,6 @@ import 'package:checks/checks.dart'; import 'package:flutter/material.dart'; import 'package:flutter/rendering.dart'; -import 'package:flutter_gen/gen_l10n/zulip_localizations.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:zulip/api/model/events.dart'; import 'package:zulip/api/model/model.dart'; @@ -11,7 +10,6 @@ import 'package:zulip/widgets/icons.dart'; import 'package:zulip/widgets/message_list.dart'; import 'package:zulip/widgets/page.dart'; import 'package:zulip/widgets/recent_dm_conversations.dart'; -import 'package:zulip/widgets/store.dart'; import '../example_data.dart' as eg; import '../flutter_checks.dart'; @@ -21,6 +19,7 @@ import '../test_navigation.dart'; import 'content_checks.dart'; import 'message_list_checks.dart'; import 'page_checks.dart'; +import 'test_app.dart'; Future setupPage(WidgetTester tester, { required List dmMessages, @@ -47,15 +46,10 @@ Future setupPage(WidgetTester tester, { fullName: newNameForSelfUser)); } - await tester.pumpWidget( - GlobalStoreWidget( - child: MaterialApp( - localizationsDelegates: ZulipLocalizations.localizationsDelegates, - supportedLocales: ZulipLocalizations.supportedLocales, - navigatorObservers: navigatorObserver != null ? [navigatorObserver] : [], - home: PerAccountStoreWidget( - accountId: eg.selfAccount.id, - child: const RecentDmConversationsPage())))); + await tester.pumpWidget(TestZulipApp( + accountId: eg.selfAccount.id, + navigatorObservers: navigatorObserver != null ? [navigatorObserver] : [], + child: const RecentDmConversationsPage())); // global store, per-account store, and page get loaded await tester.pumpAndSettle(); diff --git a/test/widgets/store_test.dart b/test/widgets/store_test.dart index a4b8500ba4..4826e2d642 100644 --- a/test/widgets/store_test.dart +++ b/test/widgets/store_test.dart @@ -156,6 +156,8 @@ void main() { await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); Future pumpWithParams({required bool light, required int accountId}) async { + // TODO use [TestZulipApp] + // (seeing some extraneous dep changes when trying that) await tester.pumpWidget( MaterialApp( theme: light ? ThemeData.light() : ThemeData.dark(), diff --git a/test/widgets/subscription_list_test.dart b/test/widgets/subscription_list_test.dart index 1d504ceb66..933874b8ff 100644 --- a/test/widgets/subscription_list_test.dart +++ b/test/widgets/subscription_list_test.dart @@ -3,7 +3,6 @@ import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:zulip/api/model/initial_snapshot.dart'; import 'package:zulip/api/model/model.dart'; -import 'package:zulip/widgets/app.dart'; import 'package:zulip/widgets/icons.dart'; import 'package:zulip/widgets/stream_colors.dart'; import 'package:zulip/widgets/subscription_list.dart'; @@ -12,6 +11,7 @@ import 'package:zulip/widgets/unread_count_badge.dart'; import '../flutter_checks.dart'; import '../model/binding.dart'; import '../example_data.dart' as eg; +import 'test_app.dart'; void main() { TestZulipBinding.ensureInitialized(); @@ -30,10 +30,8 @@ void main() { ); await testBinding.globalStore.add(eg.selfAccount, initialSnapshot); - await tester.pumpWidget(const ZulipApp()); - await tester.pump(); - final navigator = await ZulipApp.navigator; - navigator.push(SubscriptionListPage.buildRoute(accountId: eg.selfAccount.id)); + await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, + child: const SubscriptionListPage())); // global store, per-account store await tester.pumpAndSettle(); diff --git a/test/widgets/text_test.dart b/test/widgets/text_test.dart index f362186485..9b47d6d966 100644 --- a/test/widgets/text_test.dart +++ b/test/widgets/text_test.dart @@ -95,11 +95,14 @@ void main() { required FontWeight expectedFontWeight, }) async { testWidgets(description, (WidgetTester tester) async { - await tester.pumpWidget( - MaterialApp( - home: MediaQuery( - data: MediaQueryData(boldText: platformRequestsBold), - child: Builder(builder: (context) => Text('', style: styleBuilder(context)))))); + addTearDown(testBinding.reset); + tester.platformDispatcher.accessibilityFeaturesTestValue = + FakeAccessibilityFeatures(boldText: platformRequestsBold); + addTearDown(tester.platformDispatcher.clearAccessibilityFeaturesTestValue); + await tester.pumpWidget(TestZulipApp( + child: Builder(builder: (context) => + Text('', style: styleBuilder(context))))); + await tester.pump(); final TextStyle? style = tester.widget(find.byType(Text)).style; check(style) @@ -183,13 +186,14 @@ void main() { required FontWeight expectedFontWeight, }) async { testWidgets(description, (WidgetTester tester) async { + addTearDown(testBinding.reset); tester.platformDispatcher.accessibilityFeaturesTestValue = FakeAccessibilityFeatures(boldText: platformRequestsBold); - await tester.pumpWidget( - MaterialApp( - home: Builder(builder: (context) => - Text('', style: makeStyle(context))))); + await tester.pumpWidget(TestZulipApp( + child: Builder(builder: (context) => + Text('', style: makeStyle(context))))); + await tester.pump(); final TextStyle? style = tester.widget(find.byType(Text)).style; @@ -342,14 +346,15 @@ void main() { required double expected, }) async { testWidgets(description, (WidgetTester tester) async { + addTearDown(testBinding.reset); if (ambientTextScaleFactor != null) { tester.platformDispatcher.textScaleFactorTestValue = ambientTextScaleFactor; addTearDown(tester.platformDispatcher.clearTextScaleFactorTestValue); } - await tester.pumpWidget( - MaterialApp( - home: Builder(builder: (context) => Text('', - style: TextStyle(letterSpacing: getValue(context)))))); + await tester.pumpWidget(TestZulipApp( + child: Builder(builder: (context) => Text('', + style: TextStyle(letterSpacing: getValue(context)))))); + await tester.pump(); final TextStyle? style = tester.widget(find.byType(Text)).style; final actualLetterSpacing = style!.letterSpacing!; diff --git a/test/widgets/theme_test.dart b/test/widgets/theme_test.dart index 5814c033c8..c30ead3a7b 100644 --- a/test/widgets/theme_test.dart +++ b/test/widgets/theme_test.dart @@ -23,24 +23,19 @@ void main() { double? ambientTextScaleFactor, }) async { testWidgets(description, (WidgetTester tester) async { + addTearDown(testBinding.reset); if (ambientTextScaleFactor != null) { tester.platformDispatcher.textScaleFactorTestValue = ambientTextScaleFactor; addTearDown(tester.platformDispatcher.clearTextScaleFactorTestValue); } - late final double expectedFontSize; - late final double expectedLetterSpacing; - await tester.pumpWidget( - Builder(builder: (context) => MaterialApp( - theme: zulipThemeData(context), - home: Builder(builder: (context) { - expectedFontSize = Theme.of(context).textTheme.labelLarge!.fontSize!; - expectedLetterSpacing = proportionalLetterSpacing(context, - 0.01, baseFontSize: expectedFontSize); - return button; - })))); - - final text = tester.renderObject(find.text(buttonText)).text; - check(text.style!) + await tester.pumpWidget(TestZulipApp( + child: button)); + await tester.pump(); + final context = tester.element(find.text(buttonText)); + final expectedFontSize = Theme.of(context).textTheme.labelLarge!.fontSize!; + final expectedLetterSpacing = proportionalLetterSpacing(context, + 0.01, baseFontSize: expectedFontSize); + check((context.renderObject as RenderParagraph).text.style!) ..fontSize.equals(expectedFontSize) ..letterSpacing.equals(expectedLetterSpacing); }); From ccefc0eaebc14c1d17712d767dcb551bc6525d91 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 25 Jul 2024 18:16:53 -0700 Subject: [PATCH 06/10] emoji_reaction test: Set up text direction more realistically --- test/widgets/emoji_reaction_test.dart | 33 +++++++++++++++------------ 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/test/widgets/emoji_reaction_test.dart b/test/widgets/emoji_reaction_test.dart index 11261c9ee8..a3e0590a7f 100644 --- a/test/widgets/emoji_reaction_test.dart +++ b/test/widgets/emoji_reaction_test.dart @@ -46,21 +46,26 @@ void main() { }) async { final message = eg.streamMessage(reactions: reactions); + final locale = switch (textDirection) { + TextDirection.ltr => const Locale('en'), + TextDirection.rtl => const Locale('ar'), + }; + tester.platformDispatcher.localeTestValue = locale; + tester.platformDispatcher.localesTestValue = [locale]; + addTearDown(tester.platformDispatcher.clearLocaleTestValue); + addTearDown(tester.platformDispatcher.clearLocalesTestValue); + await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, - child: Directionality( - textDirection: textDirection, - child: Center( - child: ColoredBox( - color: Colors.white, - child: SizedBox( - width: width, - child: ReactionChipsList( - messageId: message.id, - reactions: message.reactions!, - ))))))); - - // global store, per-account store - await tester.pumpAndSettle(); + child: Center( + child: ColoredBox( + color: Colors.white, + child: SizedBox( + width: width, + child: ReactionChipsList( + messageId: message.id, + reactions: message.reactions!, + )))))); + await tester.pumpAndSettle(); // global store, per-account store final reactionChipsList = tester.element(find.byType(ReactionChipsList)); check(Directionality.of(reactionChipsList)).equals(textDirection); From 5605e2e44bc7eeba23443c06377eec6015c5d533 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 16 Jul 2024 12:20:34 -0700 Subject: [PATCH 07/10] emoji_reaction test [nfc]: Move some setup code out next to similar code --- test/widgets/emoji_reaction_test.dart | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/test/widgets/emoji_reaction_test.dart b/test/widgets/emoji_reaction_test.dart index a3e0590a7f..6b78de6b4c 100644 --- a/test/widgets/emoji_reaction_test.dart +++ b/test/widgets/emoji_reaction_test.dart @@ -42,19 +42,9 @@ void main() { Future setupChipsInBox(WidgetTester tester, { required List reactions, double width = 245.0, // (seen in context on an iPhone 13 Pro) - TextDirection textDirection = TextDirection.ltr, }) async { final message = eg.streamMessage(reactions: reactions); - final locale = switch (textDirection) { - TextDirection.ltr => const Locale('en'), - TextDirection.rtl => const Locale('ar'), - }; - tester.platformDispatcher.localeTestValue = locale; - tester.platformDispatcher.localesTestValue = [locale]; - addTearDown(tester.platformDispatcher.clearLocaleTestValue); - addTearDown(tester.platformDispatcher.clearLocalesTestValue); - await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, child: Center( child: ColoredBox( @@ -68,7 +58,6 @@ void main() { await tester.pumpAndSettle(); // global store, per-account store final reactionChipsList = tester.element(find.byType(ReactionChipsList)); - check(Directionality.of(reactionChipsList)).equals(textDirection); check(reactionChipsList).size.isNotNull().width.equals(width); } @@ -106,6 +95,15 @@ void main() { tester.platformDispatcher.textScaleFactorTestValue = textScaleFactor; addTearDown(tester.platformDispatcher.clearTextScaleFactorTestValue); + final locale = switch (textDirection) { + TextDirection.ltr => const Locale('en'), + TextDirection.rtl => const Locale('ar'), + }; + tester.platformDispatcher.localeTestValue = locale; + tester.platformDispatcher.localesTestValue = [locale]; + addTearDown(tester.platformDispatcher.clearLocaleTestValue); + addTearDown(tester.platformDispatcher.clearLocalesTestValue); + await prepare(); await store.addUsers(users); @@ -126,7 +124,10 @@ void main() { ..statusCode = HttpStatus.ok ..content = kSolidBlueAvatar; - await setupChipsInBox(tester, textDirection: textDirection, reactions: reactions); + await setupChipsInBox(tester, reactions: reactions); + + final reactionChipsList = tester.element(find.byType(ReactionChipsList)); + check(Directionality.of(reactionChipsList)).equals(textDirection); // TODO(upstream) Do these in an addTearDown, once we can: // https://github.com/flutter/flutter/issues/123189 From ae476b25b6b7997c78a15af56e919b960a1587a3 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 16 Jul 2024 12:23:49 -0700 Subject: [PATCH 08/10] emoji_reaction test: Add another convenient test-setup check --- test/flutter_checks.dart | 5 +++++ test/widgets/emoji_reaction_test.dart | 2 ++ 2 files changed, 7 insertions(+) diff --git a/test/flutter_checks.dart b/test/flutter_checks.dart index af66b031a5..b3b6b88980 100644 --- a/test/flutter_checks.dart +++ b/test/flutter_checks.dart @@ -121,3 +121,8 @@ extension ElementChecks on Subject { Subject get size => has((t) => t.size, 'size'); // TODO more } + +extension MediaQueryDataChecks on Subject { + Subject get textScaler => has((x) => x.textScaler, 'textScaler'); + // TODO more +} diff --git a/test/widgets/emoji_reaction_test.dart b/test/widgets/emoji_reaction_test.dart index 6b78de6b4c..f8af9d0d7b 100644 --- a/test/widgets/emoji_reaction_test.dart +++ b/test/widgets/emoji_reaction_test.dart @@ -127,6 +127,8 @@ void main() { await setupChipsInBox(tester, reactions: reactions); final reactionChipsList = tester.element(find.byType(ReactionChipsList)); + check(MediaQuery.of(reactionChipsList)) + .textScaler.equals(TextScaler.linear(textScaleFactor)); check(Directionality.of(reactionChipsList)).equals(textDirection); // TODO(upstream) Do these in an addTearDown, once we can: From 0bae1dcc13f1015837dc5372023a0b7ae4bb7393 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 25 Jul 2024 18:43:14 -0700 Subject: [PATCH 09/10] emoji_reaction test [nfc]: Move some helper logic out of its group --- test/widgets/emoji_reaction_test.dart | 74 +++++++++++++-------------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/test/widgets/emoji_reaction_test.dart b/test/widgets/emoji_reaction_test.dart index f8af9d0d7b..96549d2497 100644 --- a/test/widgets/emoji_reaction_test.dart +++ b/test/widgets/emoji_reaction_test.dart @@ -23,44 +23,44 @@ import 'text_test.dart'; void main() { TestZulipBinding.ensureInitialized(); - group('ReactionChipsList', () { - late PerAccountStore store; - - Future prepare() async { - addTearDown(testBinding.reset); - await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); - store = await testBinding.globalStore.perAccount(eg.selfAccount.id); - - await store.addUser(eg.selfUser); - - // TODO do this more centrally, or put in reusable helper - final Future font = rootBundle.load('assets/Source_Sans_3/SourceSans3VF-Upright.otf'); - final fontLoader = FontLoader('Source Sans 3')..addFont(font); - await fontLoader.load(); - } - - Future setupChipsInBox(WidgetTester tester, { - required List reactions, - double width = 245.0, // (seen in context on an iPhone 13 Pro) - }) async { - final message = eg.streamMessage(reactions: reactions); - - await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, - child: Center( - child: ColoredBox( - color: Colors.white, - child: SizedBox( - width: width, - child: ReactionChipsList( - messageId: message.id, - reactions: message.reactions!, - )))))); - await tester.pumpAndSettle(); // global store, per-account store - - final reactionChipsList = tester.element(find.byType(ReactionChipsList)); - check(reactionChipsList).size.isNotNull().width.equals(width); - } + late PerAccountStore store; + + Future prepare() async { + addTearDown(testBinding.reset); + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + + await store.addUser(eg.selfUser); + + // TODO do this more centrally, or put in reusable helper + final Future font = rootBundle.load('assets/Source_Sans_3/SourceSans3VF-Upright.otf'); + final fontLoader = FontLoader('Source Sans 3')..addFont(font); + await fontLoader.load(); + } + + Future setupChipsInBox(WidgetTester tester, { + required List reactions, + double width = 245.0, // (seen in context on an iPhone 13 Pro) + }) async { + final message = eg.streamMessage(reactions: reactions); + + await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, + child: Center( + child: ColoredBox( + color: Colors.white, + child: SizedBox( + width: width, + child: ReactionChipsList( + messageId: message.id, + reactions: message.reactions!, + )))))); + await tester.pumpAndSettle(); // global store, per-account store + + final reactionChipsList = tester.element(find.byType(ReactionChipsList)); + check(reactionChipsList).size.isNotNull().width.equals(width); + } + group('ReactionChipsList', () { // Smoke tests under various conditions. for (final displayEmojiReactionUsers in [true, false]) { for (final emojiset in [Emojiset.text, Emojiset.google]) { From 6324bf3ca5af28c59ad6b440c9efa556a7f798d5 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 11 Jul 2024 15:54:43 -0700 Subject: [PATCH 10/10] emoji_reaction: Add EmojiReactionTheme, including dark variant Related: #95 --- lib/widgets/emoji_reaction.dart | 126 +++++++++++++++++++++----- lib/widgets/theme.dart | 7 +- test/flutter_checks.dart | 5 + test/widgets/emoji_reaction_test.dart | 42 +++++++++ 4 files changed, 155 insertions(+), 25 deletions(-) diff --git a/lib/widgets/emoji_reaction.dart b/lib/widgets/emoji_reaction.dart index 24210af8c1..e3f24b5601 100644 --- a/lib/widgets/emoji_reaction.dart +++ b/lib/widgets/emoji_reaction.dart @@ -8,6 +8,102 @@ import 'content.dart'; import 'store.dart'; import 'text.dart'; +/// Emoji-reaction styles that differ between light and dark themes. +class EmojiReactionTheme extends ThemeExtension { + EmojiReactionTheme.light() : + this._( + bgSelected: Colors.white, + + // TODO shadow effect, following web, which uses `box-shadow: inset`: + // https://developer.mozilla.org/en-US/docs/Web/CSS/box-shadow#inset + // Needs Flutter support for something like that: + // https://github.com/flutter/flutter/issues/18636 + // https://github.com/flutter/flutter/issues/52999 + // Until then use a solid color; a much-lightened version of the shadow color. + // Also adapt by making [borderUnselected] more transparent, so we'll + // want to check that against web when implementing the shadow. + bgUnselected: const HSLColor.fromAHSL(0.08, 210, 0.50, 0.875).toColor(), + + borderSelected: Colors.black.withOpacity(0.45), + + // TODO see TODO on [bgUnselected] about shadow effect + borderUnselected: Colors.black.withOpacity(0.05), + + textSelected: const HSLColor.fromAHSL(1, 210, 0.20, 0.20).toColor(), + textUnselected: const HSLColor.fromAHSL(1, 210, 0.20, 0.25).toColor(), + ); + + EmojiReactionTheme.dark() : + this._( + bgSelected: Colors.black.withOpacity(0.8), + bgUnselected: Colors.black.withOpacity(0.3), + borderSelected: Colors.white.withOpacity(0.75), + borderUnselected: Colors.white.withOpacity(0.15), + textSelected: Colors.white.withOpacity(0.85), + textUnselected: Colors.white.withOpacity(0.75), + ); + + EmojiReactionTheme._({ + required this.bgSelected, + required this.bgUnselected, + required this.borderSelected, + required this.borderUnselected, + required this.textSelected, + required this.textUnselected, + }); + + /// The [EmojiReactionTheme] from the context's active theme. + /// + /// The [ThemeData] must include [EmojiReactionTheme] in [ThemeData.extensions]. + static EmojiReactionTheme of(BuildContext context) { + final theme = Theme.of(context); + final extension = theme.extension(); + assert(extension != null); + return extension!; + } + + final Color bgSelected; + final Color bgUnselected; + final Color borderSelected; + final Color borderUnselected; + final Color textSelected; + final Color textUnselected; + + @override + EmojiReactionTheme copyWith({ + Color? bgSelected, + Color? bgUnselected, + Color? borderSelected, + Color? borderUnselected, + Color? textSelected, + Color? textUnselected, + }) { + return EmojiReactionTheme._( + bgSelected: bgSelected ?? this.bgSelected, + bgUnselected: bgUnselected ?? this.bgUnselected, + borderSelected: borderSelected ?? this.borderSelected, + borderUnselected: borderUnselected ?? this.borderUnselected, + textSelected: textSelected ?? this.textSelected, + textUnselected: textUnselected ?? this.textUnselected, + ); + } + + @override + EmojiReactionTheme lerp(EmojiReactionTheme other, double t) { + if (identical(this, other)) { + return this; + } + return EmojiReactionTheme._( + bgSelected: Color.lerp(bgSelected, other.bgSelected, t)!, + bgUnselected: Color.lerp(bgUnselected, other.bgUnselected, t)!, + borderSelected: Color.lerp(borderSelected, other.borderSelected, t)!, + borderUnselected: Color.lerp(borderUnselected, other.borderUnselected, t)!, + textSelected: Color.lerp(textSelected, other.textSelected, t)!, + textUnselected: Color.lerp(textUnselected, other.textUnselected, t)!, + ); + } +} + class ReactionChipsList extends StatelessWidget { const ReactionChipsList({ super.key, @@ -32,24 +128,6 @@ class ReactionChipsList extends StatelessWidget { } } -final _textColorSelected = const HSLColor.fromAHSL(1, 210, 0.20, 0.20).toColor(); -final _textColorUnselected = const HSLColor.fromAHSL(1, 210, 0.20, 0.25).toColor(); - -const _backgroundColorSelected = Colors.white; -// TODO shadow effect, following web, which uses `box-shadow: inset`: -// https://developer.mozilla.org/en-US/docs/Web/CSS/box-shadow#inset -// Needs Flutter support for something like that: -// https://github.com/flutter/flutter/issues/18636 -// https://github.com/flutter/flutter/issues/52999 -// Until then use a solid color; a much-lightened version of the shadow color. -// Also adapt by making [_borderColorUnselected] more transparent, so we'll -// want to check that against web when implementing the shadow. -final _backgroundColorUnselected = const HSLColor.fromAHSL(0.08, 210, 0.50, 0.875).toColor(); - -final _borderColorSelected = Colors.black.withOpacity(0.45); -// TODO see TODO on [_backgroundColorUnselected] about shadow effect -final _borderColorUnselected = Colors.black.withOpacity(0.05); - class ReactionChip extends StatelessWidget { final bool showName; final int messageId; @@ -85,10 +163,11 @@ class ReactionChip extends StatelessWidget { }).join(', ') : userIds.length.toString(); - final borderColor = selfVoted ? _borderColorSelected : _borderColorUnselected; - final labelColor = selfVoted ? _textColorSelected : _textColorUnselected; - final backgroundColor = selfVoted ? _backgroundColorSelected : _backgroundColorUnselected; - final splashColor = selfVoted ? _backgroundColorUnselected : _backgroundColorSelected; + final reactionTheme = EmojiReactionTheme.of(context); + final borderColor = selfVoted ? reactionTheme.borderSelected : reactionTheme.borderUnselected; + final labelColor = selfVoted ? reactionTheme.textSelected : reactionTheme.textUnselected; + final backgroundColor = selfVoted ? reactionTheme.bgSelected : reactionTheme.bgUnselected; + final splashColor = selfVoted ? reactionTheme.bgUnselected : reactionTheme.bgSelected; final highlightColor = splashColor.withOpacity(0.5); final borderSide = BorderSide( @@ -349,6 +428,7 @@ class _TextEmoji extends StatelessWidget { @override Widget build(BuildContext context) { + final reactionTheme = EmojiReactionTheme.of(context); return Text( textAlign: TextAlign.end, textScaler: _textEmojiScalerClamped(context), @@ -356,7 +436,7 @@ class _TextEmoji extends StatelessWidget { style: TextStyle( fontSize: 14 * 0.8, height: 1, // to be denser when we have to wrap - color: selected ? _textColorSelected : _textColorUnselected, + color: selected ? reactionTheme.textSelected : reactionTheme.textUnselected, ).merge(weightVariableTextStyle(context, wght: selected ? 600 : null)), // Encourage line breaks before "_" (common in these), but try not diff --git a/lib/widgets/theme.dart b/lib/widgets/theme.dart index c9415e7be2..d9de1268e4 100644 --- a/lib/widgets/theme.dart +++ b/lib/widgets/theme.dart @@ -2,6 +2,7 @@ import 'package:flutter/material.dart'; import '../api/model/model.dart'; import 'content.dart'; +import 'emoji_reaction.dart'; import 'stream_colors.dart'; import 'text.dart'; @@ -37,11 +38,13 @@ ThemeData zulipThemeData(BuildContext context) { switch (brightness) { case Brightness.light: { designVariables = DesignVariables.light(); - themeExtensions = [ContentTheme.light(context), designVariables]; + themeExtensions = + [ContentTheme.light(context), designVariables, EmojiReactionTheme.light()]; } case Brightness.dark: { designVariables = DesignVariables.dark(); - themeExtensions = [ContentTheme.dark(context), designVariables]; + themeExtensions = + [ContentTheme.dark(context), designVariables, EmojiReactionTheme.dark()]; } } diff --git a/test/flutter_checks.dart b/test/flutter_checks.dart index b3b6b88980..d0906d849b 100644 --- a/test/flutter_checks.dart +++ b/test/flutter_checks.dart @@ -126,3 +126,8 @@ extension MediaQueryDataChecks on Subject { Subject get textScaler => has((x) => x.textScaler, 'textScaler'); // TODO more } + +extension MaterialChecks on Subject { + Subject get color => has((x) => x.color, 'color'); + // TODO more +} diff --git a/test/widgets/emoji_reaction_test.dart b/test/widgets/emoji_reaction_test.dart index 96549d2497..fa96d1426e 100644 --- a/test/widgets/emoji_reaction_test.dart +++ b/test/widgets/emoji_reaction_test.dart @@ -11,6 +11,7 @@ import 'package:zulip/api/model/events.dart'; import 'package:zulip/api/model/model.dart'; import 'package:zulip/model/store.dart'; import 'package:zulip/widgets/emoji_reaction.dart'; +import 'package:zulip/widgets/theme.dart'; import '../example_data.dart' as eg; import '../flutter_checks.dart'; @@ -216,6 +217,47 @@ void main() { } }); + testWidgets('Smoke test for light/dark/lerped', (tester) async { + await prepare(); + await store.addUsers([eg.selfUser, eg.otherUser]); + + assert(!debugFollowPlatformBrightness); // to be removed with #95 + debugFollowPlatformBrightness = true; + addTearDown(() { debugFollowPlatformBrightness = false; }); + tester.platformDispatcher.platformBrightnessTestValue = Brightness.light; + addTearDown(tester.platformDispatcher.clearPlatformBrightnessTestValue); + + await setupChipsInBox(tester, reactions: [ + Reaction.fromJson({ + 'user_id': eg.selfUser.userId, + 'emoji_name': 'smile', 'emoji_code': '1f642', 'reaction_type': 'unicode_emoji'}), + Reaction.fromJson({ + 'user_id': eg.otherUser.userId, + 'emoji_name': 'tada', 'emoji_code': '1f389', 'reaction_type': 'unicode_emoji'}), + ]); + + Color? backgroundColor(String emojiName) { + final material = tester.widget(find.descendant( + of: find.byTooltip(emojiName), matching: find.byType(Material))); + return material.color; + } + + check(backgroundColor('smile')).equals(EmojiReactionTheme.light().bgSelected); + check(backgroundColor('tada')).equals(EmojiReactionTheme.light().bgUnselected); + + tester.platformDispatcher.platformBrightnessTestValue = Brightness.dark; + await tester.pump(); + + await tester.pump(kThemeAnimationDuration * 0.4); + final expectedLerped = EmojiReactionTheme.light().lerp(EmojiReactionTheme.dark(), 0.4); + check(backgroundColor('smile')).equals(expectedLerped.bgSelected); + check(backgroundColor('tada')).equals(expectedLerped.bgUnselected); + + await tester.pump(kThemeAnimationDuration * 0.6); + check(backgroundColor('smile')).equals(EmojiReactionTheme.dark().bgSelected); + check(backgroundColor('tada')).equals(EmojiReactionTheme.dark().bgUnselected); + }); + // TODO more tests: // - Tapping a chip does the right thing // - When an image emoji fails to load, falls back to :text_emoji: