Skip to content

Commit 8d3c128

Browse files
committed
dialog: Remove no-op SingleChildScrollView on iOS
In #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).
1 parent 609b490 commit 8d3c128

File tree

3 files changed

+85
-5
lines changed

3 files changed

+85
-5
lines changed

lib/model/settings.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,12 @@ class GlobalSettingsStore extends ChangeNotifier {
354354
return _data.legacyUpgradeState ?? LegacyUpgradeState._default;
355355
}
356356

357+
/// Set [legacyUpgradeState], persistently for future runs of the app.
358+
@visibleForTesting
359+
Future<void> debugSetLegacyUpgradeState(LegacyUpgradeState value) async {
360+
await _update(GlobalSettingsCompanion(legacyUpgradeState: Value(value)));
361+
}
362+
357363
/// The user's choice of the given bool-valued setting, or our default for it.
358364
///
359365
/// See also [setBool].

lib/widgets/dialog.dart

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,27 @@ Widget _adaptiveAction({required VoidCallback onPressed, required String text})
3434
}
3535
}
3636

37+
/// Platform-appropriate content for [AlertDialog.adaptive]'s [content] param.
38+
Widget? _adaptiveContent(Widget? content) {
39+
if (content == null) return null;
40+
41+
switch (defaultTargetPlatform) {
42+
case TargetPlatform.android:
43+
case TargetPlatform.fuchsia:
44+
case TargetPlatform.linux:
45+
case TargetPlatform.windows:
46+
// [AlertDialog] does not create a [SingleChildScrollView];
47+
// callers are asked to do that themselves, to handle long content.
48+
return SingleChildScrollView(child: content);
49+
50+
case TargetPlatform.iOS:
51+
case TargetPlatform.macOS:
52+
// A [SingleChildScrollView] (wrapping both title and content) is already
53+
// created by [CupertinoAlertDialog].
54+
return content;
55+
}
56+
}
57+
3758
/// Tracks the status of a dialog, in being still open or already closed.
3859
///
3960
/// Use [T] to identify the outcome of the interaction:
@@ -86,7 +107,7 @@ DialogStatus<void> showErrorDialog({
86107
context: context,
87108
builder: (BuildContext context) => AlertDialog.adaptive(
88109
title: Text(title),
89-
content: message != null ? SingleChildScrollView(child: Text(message)) : null,
110+
content: message != null ? _adaptiveContent(Text(message)) : null,
90111
actions: [
91112
if (learnMoreButtonUrl != null)
92113
_adaptiveAction(
@@ -118,7 +139,7 @@ DialogStatus<bool> showSuggestedActionDialog({
118139
context: context,
119140
builder: (BuildContext context) => AlertDialog.adaptive(
120141
title: Text(title),
121-
content: SingleChildScrollView(child: Text(message)),
142+
content: _adaptiveContent(Text(message)),
122143
actions: [
123144
_adaptiveAction(
124145
onPressed: () => Navigator.pop<bool>(context, null),
@@ -179,8 +200,8 @@ class UpgradeWelcomeDialog extends StatelessWidget {
179200
final zulipLocalizations = ZulipLocalizations.of(context);
180201
return AlertDialog.adaptive(
181202
title: Text(zulipLocalizations.upgradeWelcomeDialogTitle),
182-
content: SingleChildScrollView(
183-
child: Column(crossAxisAlignment: CrossAxisAlignment.start, children: [
203+
content: _adaptiveContent(
204+
Column(crossAxisAlignment: CrossAxisAlignment.start, children: [
184205
Text(zulipLocalizations.upgradeWelcomeDialogMessage),
185206
GestureDetector(
186207
onTap: () => PlatformActions.launchUrl(context,

test/widgets/dialog_test.dart

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
11
import 'package:checks/checks.dart';
22
import 'package:flutter/foundation.dart';
33
import 'package:flutter/material.dart';
4+
import 'package:flutter_checks/flutter_checks.dart';
45
import 'package:flutter_test/flutter_test.dart';
56
import 'package:url_launcher/url_launcher.dart';
7+
import 'package:zulip/model/settings.dart';
8+
import 'package:zulip/widgets/app.dart';
69
import 'package:zulip/widgets/dialog.dart';
10+
import 'package:zulip/widgets/store.dart';
711

812
import '../model/binding.dart';
913
import 'dialog_checks.dart';
@@ -64,6 +68,17 @@ void main() {
6468
check(testBinding.takeLaunchUrlCalls()).single
6569
.equals((url: learnMoreButtonUrl, mode: expectedMode));
6670
}, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
71+
72+
testWidgets('only one SingleChildScrollView created', (tester) async {
73+
await prepare(tester);
74+
75+
showErrorDialog(context: context, title: title, message: message);
76+
await tester.pump();
77+
checkErrorDialog(tester, expectedTitle: title, expectedMessage: message);
78+
79+
check(find.ancestor(of: find.text(message),
80+
matching: find.byType(SingleChildScrollView))).findsOne();
81+
}, variant: TargetPlatformVariant.all());
6782
});
6883

6984
group('showSuggestedActionDialog', () {
@@ -113,5 +128,43 @@ void main() {
113128
}, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
114129
});
115130

116-
// TODO(#1594): test UpgradeWelcomeDialog
131+
testWidgets('only one SingleChildScrollView created', (tester) async {
132+
addTearDown(testBinding.reset);
133+
await tester.pumpWidget(TestZulipApp());
134+
await tester.pump();
135+
final element = tester.element(find.byType(Placeholder));
136+
137+
showSuggestedActionDialog(context: element,
138+
title: 'Continue?',
139+
message: 'Do the thing?',
140+
actionButtonText: 'Sure');
141+
await tester.pump();
142+
143+
check(find.ancestor(of: find.text('Do the thing?'),
144+
matching: find.byType(SingleChildScrollView))).findsOne();
145+
}, variant: TargetPlatformVariant.all());
146+
147+
group('UpgradeWelcomeDialog', () {
148+
// TODO(#1594): test LegacyUpgradeState and BoolGlobalSetting.upgradeWelcomeDialogShown
149+
150+
testWidgets('only one SingleChildScrollView created', (tester) async {
151+
final transitionDurationObserver = TransitionDurationObserver();
152+
addTearDown(testBinding.reset);
153+
154+
// Real ZulipApp needed because the show-dialog function calls
155+
// `await ZulipApp.navigator`.
156+
await tester.pumpWidget(ZulipApp(navigatorObservers: [transitionDurationObserver]));
157+
await tester.pump();
158+
159+
await testBinding.globalStore.settings
160+
.debugSetLegacyUpgradeState(LegacyUpgradeState.found);
161+
162+
UpgradeWelcomeDialog.maybeShow();
163+
await transitionDurationObserver.pumpPastTransition(tester);
164+
165+
final expectedMessage = 'You’ll find a familiar experience in a faster, sleeker package.';
166+
check(find.ancestor(of: find.text(expectedMessage),
167+
matching: find.byType(SingleChildScrollView))).findsOne();
168+
}, variant: TargetPlatformVariant.all());
169+
});
117170
}

0 commit comments

Comments
 (0)