diff --git a/lib/model/settings.dart b/lib/model/settings.dart index 384d2e5c30..85a1c40f50 100644 --- a/lib/model/settings.dart +++ b/lib/model/settings.dart @@ -354,6 +354,12 @@ class GlobalSettingsStore extends ChangeNotifier { return _data.legacyUpgradeState ?? LegacyUpgradeState._default; } + /// Set [legacyUpgradeState], persistently for future runs of the app. + @visibleForTesting + Future debugSetLegacyUpgradeState(LegacyUpgradeState value) async { + await _update(GlobalSettingsCompanion(legacyUpgradeState: Value(value))); + } + /// The user's choice of the given bool-valued setting, or our default for it. /// /// See also [setBool]. diff --git a/lib/widgets/dialog.dart b/lib/widgets/dialog.dart index 61a6462592..c5e2e6e562 100644 --- a/lib/widgets/dialog.dart +++ b/lib/widgets/dialog.dart @@ -34,6 +34,27 @@ Widget _adaptiveAction({required VoidCallback onPressed, required String text}) } } +/// Platform-appropriate content for [AlertDialog.adaptive]'s [content] param. +Widget? _adaptiveContent(Widget? content) { + if (content == null) return null; + + switch (defaultTargetPlatform) { + case TargetPlatform.android: + case TargetPlatform.fuchsia: + case TargetPlatform.linux: + case TargetPlatform.windows: + // [AlertDialog] does not create a [SingleChildScrollView]; + // callers are asked to do that themselves, to handle long content. + return SingleChildScrollView(child: content); + + case TargetPlatform.iOS: + case TargetPlatform.macOS: + // A [SingleChildScrollView] (wrapping both title and content) is already + // created by [CupertinoAlertDialog]. + return content; + } +} + /// Tracks the status of a dialog, in being still open or already closed. /// /// Use [T] to identify the outcome of the interaction: @@ -86,7 +107,7 @@ DialogStatus showErrorDialog({ context: context, builder: (BuildContext context) => AlertDialog.adaptive( title: Text(title), - content: message != null ? SingleChildScrollView(child: Text(message)) : null, + content: message != null ? _adaptiveContent(Text(message)) : null, actions: [ if (learnMoreButtonUrl != null) _adaptiveAction( @@ -118,7 +139,7 @@ DialogStatus showSuggestedActionDialog({ context: context, builder: (BuildContext context) => AlertDialog.adaptive( title: Text(title), - content: SingleChildScrollView(child: Text(message)), + content: _adaptiveContent(Text(message)), actions: [ _adaptiveAction( onPressed: () => Navigator.pop(context, null), @@ -179,8 +200,8 @@ class UpgradeWelcomeDialog extends StatelessWidget { final zulipLocalizations = ZulipLocalizations.of(context); return AlertDialog.adaptive( title: Text(zulipLocalizations.upgradeWelcomeDialogTitle), - content: SingleChildScrollView( - child: Column(crossAxisAlignment: CrossAxisAlignment.start, children: [ + content: _adaptiveContent( + Column(crossAxisAlignment: CrossAxisAlignment.start, children: [ Text(zulipLocalizations.upgradeWelcomeDialogMessage), GestureDetector( onTap: () => PlatformActions.launchUrl(context, diff --git a/test/widgets/dialog_checks.dart b/test/widgets/dialog_checks.dart index 20ff0effd2..ce7effe2a9 100644 --- a/test/widgets/dialog_checks.dart +++ b/test/widgets/dialog_checks.dart @@ -13,10 +13,15 @@ import 'package:zulip/widgets/dialog.dart'; /// /// On success, returns the widget's "OK" button. /// Dismiss the dialog by calling `tester.tap(find.byWidget(okButton))`. +/// +/// See also: +/// - [checkNoDialog] Widget checkErrorDialog(WidgetTester tester, { required String expectedTitle, String? expectedMessage, }) { + // TODO if a dialog was found but it doesn't match expectations, + // show its details; see checkNoDialog for how to do that switch (defaultTargetPlatform) { case TargetPlatform.android: case TargetPlatform.fuchsia: @@ -46,12 +51,47 @@ Widget checkErrorDialog(WidgetTester tester, { } } -/// Checks that there is no dialog. -/// Fails if one is found. + +/// In a widget test, check that there aren't any alert dialogs. +/// +/// See also: +/// - [checkErrorDialog] void checkNoDialog(WidgetTester tester) { + final List alertDialogs = [ + ...tester.widgetList(find.bySubtype()), + ...tester.widgetList(find.byType(CupertinoAlertDialog)), + ]; + + if (alertDialogs.isNotEmpty) { + final message = StringBuffer()..write('Found dialog(s) when none were expected:\n'); + for (final alertDialog in alertDialogs) { + final (title, content) = switch (alertDialog) { + AlertDialog() => (alertDialog.title, alertDialog.content), + CupertinoAlertDialog() => (alertDialog.title, alertDialog.content), + _ => throw UnimplementedError(), + }; + + message.write('Dialog:\n' + ' title: ${title is Text ? title.data : title.toString()}\n'); + + if (content != null) { + final contentTexts = tester.widgetList(find.descendant( + matchRoot: true, + of: find.byWidget(content), + matching: find.byType(Text))); + message.write(' content: '); + if (contentTexts.isNotEmpty) { + message.write(contentTexts.map((t) => t.data).join('\n ')); + } else { + // (Could show more detail here as necessary.) + message.write(content.toString()); + } + } + } + throw TestFailure(message.toString()); + } + check(find.byType(Dialog)).findsNothing(); - check(find.bySubtype()).findsNothing(); - check(find.byType(CupertinoAlertDialog)).findsNothing(); } /// In a widget test, check that [showSuggestedActionDialog] was called diff --git a/test/widgets/dialog_test.dart b/test/widgets/dialog_test.dart index 855affbb3f..b6f44e2b14 100644 --- a/test/widgets/dialog_test.dart +++ b/test/widgets/dialog_test.dart @@ -1,9 +1,13 @@ import 'package:checks/checks.dart'; import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; +import 'package:flutter_checks/flutter_checks.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:url_launcher/url_launcher.dart'; +import 'package:zulip/model/settings.dart'; +import 'package:zulip/widgets/app.dart'; import 'package:zulip/widgets/dialog.dart'; +import 'package:zulip/widgets/store.dart'; import '../model/binding.dart'; import 'dialog_checks.dart'; @@ -64,6 +68,17 @@ void main() { check(testBinding.takeLaunchUrlCalls()).single .equals((url: learnMoreButtonUrl, mode: expectedMode)); }, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS})); + + testWidgets('only one SingleChildScrollView created', (tester) async { + await prepare(tester); + + showErrorDialog(context: context, title: title, message: message); + await tester.pump(); + checkErrorDialog(tester, expectedTitle: title, expectedMessage: message); + + check(find.ancestor(of: find.text(message), + matching: find.byType(SingleChildScrollView))).findsOne(); + }, variant: TargetPlatformVariant.all()); }); group('showSuggestedActionDialog', () { @@ -113,5 +128,43 @@ void main() { }, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS})); }); - // TODO(#1594): test UpgradeWelcomeDialog + testWidgets('only one SingleChildScrollView created', (tester) async { + addTearDown(testBinding.reset); + await tester.pumpWidget(TestZulipApp()); + await tester.pump(); + final element = tester.element(find.byType(Placeholder)); + + showSuggestedActionDialog(context: element, + title: 'Continue?', + message: 'Do the thing?', + actionButtonText: 'Sure'); + await tester.pump(); + + check(find.ancestor(of: find.text('Do the thing?'), + matching: find.byType(SingleChildScrollView))).findsOne(); + }, variant: TargetPlatformVariant.all()); + + group('UpgradeWelcomeDialog', () { + // TODO(#1594): test LegacyUpgradeState and BoolGlobalSetting.upgradeWelcomeDialogShown + + testWidgets('only one SingleChildScrollView created', (tester) async { + final transitionDurationObserver = TransitionDurationObserver(); + addTearDown(testBinding.reset); + + // Real ZulipApp needed because the show-dialog function calls + // `await ZulipApp.navigator`. + await tester.pumpWidget(ZulipApp(navigatorObservers: [transitionDurationObserver])); + await tester.pump(); + + await testBinding.globalStore.settings + .debugSetLegacyUpgradeState(LegacyUpgradeState.found); + + UpgradeWelcomeDialog.maybeShow(); + await transitionDurationObserver.pumpPastTransition(tester); + + final expectedMessage = 'You’ll find a familiar experience in a faster, sleeker package.'; + check(find.ancestor(of: find.text(expectedMessage), + matching: find.byType(SingleChildScrollView))).findsOne(); + }, variant: TargetPlatformVariant.all()); + }); }