dialog: Remove no-op SingleChildScrollView on iOS

In zulip#1017, we overlooked the fact that a SingleChildScrollView is
added automatically on iOS but not on Android.

I haven't reproduced an observable bug that comes from this, but it
calls for a fix.

I tested this change manually on iOS (showErrorDialog,
showSuggestedActionDialog, and UpgradeWelcomeDialog), with short
text and long text (longer than a screenful, to check the scrolling
works).

Author: chrisbobbe

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,34 @@ 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;
+  }
+}
+
+/// Platform-appropriate message content for [AlertDialog.adaptive]'s [content] param.
+Widget? _adaptiveMessage(String? message) {
+  if (message == null) return null;
+
+  return _adaptiveContent(Text(message));
+}
+
 /// Tracks the status of a dialog, in being still open or already closed.
 ///
 /// Use [T] to identify the outcome of the interaction:
@@ -86,7 +114,7 @@ DialogStatus<void> showErrorDialog({
     context: context,
     builder: (BuildContext context) => AlertDialog.adaptive(
       title: Text(title),
-      content: message != null ? SingleChildScrollView(child: Text(message)) : null,
+      content: _adaptiveMessage(message),
       actions: [
         if (learnMoreButtonUrl != null)
           _adaptiveAction(
@@ -118,7 +146,7 @@ DialogStatus<bool> showSuggestedActionDialog({
     context: context,
     builder: (BuildContext context) => AlertDialog.adaptive(
       title: Text(title),
-      content: SingleChildScrollView(child: Text(message)),
+      content: _adaptiveMessage(message),
       actions: [
         _adaptiveAction(
           onPressed: () => Navigator.pop<bool>(context, null),
@@ -179,8 +207,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_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,45 @@ 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();
+
+      final context = tester.element(find.byType(ChooseAccountPage));
+      // TODO could do this with [TestGlobalStore] or similar?
+      await GlobalStoreWidget.of(context)
+        .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());
+  });
 }