Skip to content

Commit cc95ace

Browse files
authored
CupertinoAlertDialog should not create ScrollController on every build, if null values are passed in constructor. (#134075)
Relanding of flutter/flutter#134071 Verified failed tests succeeded now:
1 parent a863ad6 commit cc95ace

File tree

2 files changed

+33
-17
lines changed

2 files changed

+33
-17
lines changed

packages/flutter/lib/src/cupertino/dialog.dart

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ bool _isInAccessibilityMode(BuildContext context) {
188188
/// * [CupertinoDialogAction], which is an iOS-style dialog button.
189189
/// * [AlertDialog], a Material Design alert dialog.
190190
/// * <https://developer.apple.com/ios/human-interface-guidelines/views/alerts/>
191-
class CupertinoAlertDialog extends StatelessWidget {
191+
class CupertinoAlertDialog extends StatefulWidget {
192192
/// Creates an iOS-style alert dialog.
193193
///
194194
/// The [actions] must not be null.
@@ -233,9 +233,6 @@ class CupertinoAlertDialog extends StatelessWidget {
233233
/// section when there are many actions.
234234
final ScrollController? scrollController;
235235

236-
ScrollController get _effectiveScrollController =>
237-
scrollController ?? ScrollController();
238-
239236
/// A scroll controller that can be used to control the scrolling of the
240237
/// actions in the dialog.
241238
///
@@ -247,37 +244,49 @@ class CupertinoAlertDialog extends StatelessWidget {
247244
/// section when it is long.
248245
final ScrollController? actionScrollController;
249246

250-
ScrollController get _effectiveActionScrollController =>
251-
actionScrollController ?? ScrollController();
252-
253247
/// {@macro flutter.material.dialog.insetAnimationDuration}
254248
final Duration insetAnimationDuration;
255249

256250
/// {@macro flutter.material.dialog.insetAnimationCurve}
257251
final Curve insetAnimationCurve;
258252

253+
@override
254+
State<CupertinoAlertDialog> createState() => _CupertinoAlertDialogState();
255+
}
256+
257+
class _CupertinoAlertDialogState extends State<CupertinoAlertDialog> {
258+
ScrollController? _backupScrollController;
259+
260+
ScrollController? _backupActionScrollController;
261+
262+
ScrollController get _effectiveScrollController =>
263+
widget.scrollController ?? (_backupScrollController ??= ScrollController());
264+
265+
ScrollController get _effectiveActionScrollController =>
266+
widget.actionScrollController ?? (_backupActionScrollController ??= ScrollController());
267+
259268
Widget _buildContent(BuildContext context) {
260269
final double textScaleFactor = MediaQuery.textScalerOf(context).textScaleFactor;
261270

262271
final List<Widget> children = <Widget>[
263-
if (title != null || content != null)
272+
if (widget.title != null || widget.content != null)
264273
Flexible(
265274
flex: 3,
266275
child: _CupertinoAlertContentSection(
267-
title: title,
268-
message: content,
276+
title: widget.title,
277+
message: widget.content,
269278
scrollController: _effectiveScrollController,
270279
titlePadding: EdgeInsets.only(
271280
left: _kDialogEdgePadding,
272281
right: _kDialogEdgePadding,
273-
bottom: content == null ? _kDialogEdgePadding : 1.0,
282+
bottom: widget.content == null ? _kDialogEdgePadding : 1.0,
274283
top: _kDialogEdgePadding * textScaleFactor,
275284
),
276285
messagePadding: EdgeInsets.only(
277286
left: _kDialogEdgePadding,
278287
right: _kDialogEdgePadding,
279288
bottom: _kDialogEdgePadding * textScaleFactor,
280-
top: title == null ? _kDialogEdgePadding : 1.0,
289+
top: widget.title == null ? _kDialogEdgePadding : 1.0,
281290
),
282291
titleTextStyle: _kCupertinoDialogTitleStyle.copyWith(
283292
color: CupertinoDynamicColor.resolve(CupertinoColors.label, context),
@@ -303,10 +312,10 @@ class CupertinoAlertDialog extends StatelessWidget {
303312
Widget actionSection = Container(
304313
height: 0.0,
305314
);
306-
if (actions.isNotEmpty) {
315+
if (widget.actions.isNotEmpty) {
307316
actionSection = _CupertinoAlertActionSection(
308317
scrollController: _effectiveActionScrollController,
309-
children: actions,
318+
children: widget.actions,
310319
);
311320
}
312321

@@ -330,8 +339,8 @@ class CupertinoAlertDialog extends StatelessWidget {
330339
return AnimatedPadding(
331340
padding: MediaQuery.viewInsetsOf(context) +
332341
const EdgeInsets.symmetric(horizontal: 40.0, vertical: 24.0),
333-
duration: insetAnimationDuration,
334-
curve: insetAnimationCurve,
342+
duration: widget.insetAnimationDuration,
343+
curve: widget.insetAnimationCurve,
335344
child: MediaQuery.removeViewInsets(
336345
removeLeft: true,
337346
removeTop: true,
@@ -368,6 +377,13 @@ class CupertinoAlertDialog extends StatelessWidget {
368377
),
369378
);
370379
}
380+
381+
@override
382+
void dispose() {
383+
_backupScrollController?.dispose();
384+
_backupActionScrollController?.dispose();
385+
super.dispose();
386+
}
371387
}
372388

373389
/// Rounded rectangle surface that looks like an iOS popup surface, e.g., alert dialog

packages/flutter/test/material/dialog_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2664,7 +2664,7 @@ void main() {
26642664
okNode.dispose();
26652665
});
26662666

2667-
testWidgets('Adaptive AlertDialog shows correct widget on each platform', (WidgetTester tester) async {
2667+
testWidgetsWithLeakTracking('Adaptive AlertDialog shows correct widget on each platform', (WidgetTester tester) async {
26682668
final AlertDialog dialog = AlertDialog.adaptive(
26692669
content: Container(
26702670
height: 5000.0,

0 commit comments

Comments
 (0)