dialog: Remove no-op SingleChildScrollView; have tests' checkNoDialog describe dialog

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<void> 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].

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<void> 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<bool> 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<bool>(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,

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<Widget> alertDialogs = [
+    ...tester.widgetList(find.bySubtype<AlertDialog>()),
+    ...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<Text>(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<AlertDialog>()).findsNothing();
-  check(find.byType(CupertinoAlertDialog)).findsNothing();
 }
 
 /// In a widget test, check that [showSuggestedActionDialog] was called

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());
+  });
 }