diff --git a/packages/flutter/lib/src/material/dialog.dart b/packages/flutter/lib/src/material/dialog.dart index 8c9599bd364..9ef31d26d69 100644 --- a/packages/flutter/lib/src/material/dialog.dart +++ b/packages/flutter/lib/src/material/dialog.dart @@ -5,6 +5,7 @@ // @dart = 2.8 import 'dart:async'; +import 'dart:ui'; import 'package:flutter/foundation.dart'; import 'package:flutter/widgets.dart'; @@ -310,7 +311,10 @@ class AlertDialog extends StatelessWidget { /// The (optional) set of actions that are displayed at the bottom of the /// dialog. /// - /// Typically this is a list of [FlatButton] widgets. + /// Typically this is a list of [FlatButton] widgets. It is recommended to + /// set the [textAlign] to [TextAlign.end] for the [Text] within the + /// [FlatButton], so that buttons whose labels wrap to an extra line align + /// with the overall [ButtonBar]'s alignment within the dialog. /// /// These widgets will be wrapped in a [ButtonBar], which introduces 8 pixels /// of padding on each side. @@ -466,12 +470,24 @@ class AlertDialog extends StatelessWidget { } } + // The paddingScaleFactor is used to adjust the padding of Dialog's + // children. + final double paddingScaleFactor = _paddingScaleFactor(MediaQuery.of(context).textScaleFactor); + final TextDirection textDirection = Directionality.of(context); + Widget titleWidget; Widget contentWidget; Widget actionsWidget; - if (title != null) - titleWidget = Padding( - padding: titlePadding ?? EdgeInsets.fromLTRB(24.0, 24.0, 24.0, content == null ? 20.0 : 0.0), + if (title != null) { + final EdgeInsets defaultTitlePadding = EdgeInsets.fromLTRB(24.0, 24.0, 24.0, content == null ? 20.0 : 0.0); + final EdgeInsets effectiveTitlePadding = titlePadding?.resolve(textDirection) ?? defaultTitlePadding; + titleWidget = Padding( + padding: EdgeInsets.only( + left: effectiveTitlePadding.left * paddingScaleFactor, + right: effectiveTitlePadding.right * paddingScaleFactor, + top: effectiveTitlePadding.top * paddingScaleFactor, + bottom: effectiveTitlePadding.bottom, + ), child: DefaultTextStyle( style: titleTextStyle ?? dialogTheme.titleTextStyle ?? theme.textTheme.headline6, child: Semantics( @@ -481,17 +497,26 @@ class AlertDialog extends StatelessWidget { ), ), ); + } - if (content != null) + if (content != null) { + final EdgeInsets effectiveContentPadding = contentPadding.resolve(textDirection); contentWidget = Padding( - padding: contentPadding, + padding: EdgeInsets.only( + left: effectiveContentPadding.left * paddingScaleFactor, + right: effectiveContentPadding.right * paddingScaleFactor, + top: title == null ? effectiveContentPadding.top * paddingScaleFactor : effectiveContentPadding.top, + bottom: effectiveContentPadding.bottom, + ), child: DefaultTextStyle( style: contentTextStyle ?? dialogTheme.contentTextStyle ?? theme.textTheme.subtitle1, child: content, ), ); + } - if (actions != null) + + if (actions != null) { actionsWidget = Padding( padding: actionsPadding, child: ButtonBar( @@ -501,6 +526,7 @@ class AlertDialog extends StatelessWidget { children: actions, ), ); + } List columnChildren; if (scrollable) { @@ -805,6 +831,44 @@ class SimpleDialog extends StatelessWidget { } } + // The paddingScaleFactor is used to adjust the padding of Dialog + // children. + final double paddingScaleFactor = _paddingScaleFactor(MediaQuery.of(context).textScaleFactor); + final TextDirection textDirection = Directionality.of(context); + + Widget titleWidget; + if (title != null) { + final EdgeInsets effectiveTitlePadding = titlePadding.resolve(textDirection); + titleWidget = Padding( + padding: EdgeInsets.only( + left: effectiveTitlePadding.left * paddingScaleFactor, + right: effectiveTitlePadding.right * paddingScaleFactor, + top: effectiveTitlePadding.top * paddingScaleFactor, + bottom: children == null ? effectiveTitlePadding.bottom * paddingScaleFactor : effectiveTitlePadding.bottom, + ), + child: DefaultTextStyle( + style: titleTextStyle ?? DialogTheme.of(context).titleTextStyle ?? theme.textTheme.headline6, + child: Semantics(namesRoute: true, child: title), + ), + ); + } + + Widget contentWidget; + if (children != null) { + final EdgeInsets effectiveContentPadding = contentPadding.resolve(textDirection); + contentWidget = Flexible( + child: SingleChildScrollView( + padding: EdgeInsets.only( + left: effectiveContentPadding.left * paddingScaleFactor, + right: effectiveContentPadding.right * paddingScaleFactor, + top: title == null ? effectiveContentPadding.top * paddingScaleFactor : effectiveContentPadding.top, + bottom: effectiveContentPadding.bottom * paddingScaleFactor, + ), + child: ListBody(children: children), + ), + ); + } + Widget dialogChild = IntrinsicWidth( stepWidth: 56.0, child: ConstrainedBox( @@ -814,20 +878,9 @@ class SimpleDialog extends StatelessWidget { crossAxisAlignment: CrossAxisAlignment.stretch, children: [ if (title != null) - Padding( - padding: titlePadding, - child: DefaultTextStyle( - style: titleTextStyle ?? DialogTheme.of(context).titleTextStyle ?? theme.textTheme.headline6, - child: Semantics(namesRoute: true, child: title), - ), - ), + titleWidget, if (children != null) - Flexible( - child: SingleChildScrollView( - padding: contentPadding, - child: ListBody(children: children), - ), - ), + contentWidget, ], ), ), @@ -959,3 +1012,10 @@ Future showDialog({ routeSettings: routeSettings, ); } + +double _paddingScaleFactor(double textScaleFactor) { + final double clampedTextScaleFactor = textScaleFactor.clamp(1.0, 2.0).toDouble(); + // The final padding scale factor is clamped between 1/3 and 1. For example, + // a non-scaled padding of 24 will produce a padding between 24 and 8. + return lerpDouble(1.0, 1.0 / 3.0, clampedTextScaleFactor - 1.0); +} diff --git a/packages/flutter/test/material/dialog_test.dart b/packages/flutter/test/material/dialog_test.dart index 531a7e0b408..039aae6c4a6 100644 --- a/packages/flutter/test/material/dialog_test.dart +++ b/packages/flutter/test/material/dialog_test.dart @@ -13,7 +13,7 @@ import 'package:matcher/matcher.dart'; import '../widgets/semantics_tester.dart'; -MaterialApp _buildAppWithDialog(Widget dialog, { ThemeData theme }) { +MaterialApp _buildAppWithDialog(Widget dialog, { ThemeData theme, double textScaleFactor = 1.0 }) { return MaterialApp( theme: theme, home: Material( @@ -26,7 +26,10 @@ MaterialApp _buildAppWithDialog(Widget dialog, { ThemeData theme }) { showDialog( context: context, builder: (BuildContext context) { - return dialog; + return MediaQuery( + data: MediaQuery.of(context).copyWith(textScaleFactor: textScaleFactor), + child: dialog, + ); }, ); }, @@ -644,6 +647,400 @@ void main() { ); // right }); + group('Dialog children padding is correct', () { + final List textScaleFactors = [0.5, 1.0, 1.5, 2.0, 3.0]; + final Map paddingScaleFactors = { + 0.5: 1.0, + 1.0: 1.0, + 1.5: 2.0 / 3.0, + 2.0: 1.0 / 3.0, + 3.0: 1.0 / 3.0, + }; + + final GlobalKey titleKey = GlobalKey(); + final GlobalKey contentKey = GlobalKey(); + final GlobalKey childrenKey = GlobalKey(); + + final Finder dialogFinder = find.descendant(of: find.byType(Dialog), matching: find.byType(Material)).first; + final Finder titleFinder = find.byKey(titleKey); + final Finder contentFinder = find.byKey(contentKey); + final Finder actionsFinder = find.byType(ButtonBar); + final Finder childrenFinder = find.byKey(childrenKey); + + Future openDialog(WidgetTester tester, Widget dialog, double textScaleFactor) async { + await tester.pumpWidget( + _buildAppWithDialog(dialog, textScaleFactor: textScaleFactor), + ); + + await tester.tap(find.text('X')); + await tester.pumpAndSettle(); + } + + void expectLeftEdgePadding( + WidgetTester tester, { + Finder finder, + double textScaleFactor, + double unscaledValue, + }) { + expect( + tester.getTopLeft(dialogFinder).dx, + closeTo(tester.getTopLeft(finder).dx - unscaledValue * paddingScaleFactors[textScaleFactor], 1e-6), + ); + expect( + tester.getBottomLeft(dialogFinder).dx, + closeTo(tester.getBottomLeft(finder).dx - unscaledValue * paddingScaleFactors[textScaleFactor], 1e-6), + ); + } + + void expectRightEdgePadding( + WidgetTester tester, { + Finder finder, + double textScaleFactor, + double unscaledValue, + }) { + expect( + tester.getTopRight(dialogFinder).dx, + closeTo(tester.getTopRight(finder).dx + unscaledValue * paddingScaleFactors[textScaleFactor], 1e-6), + ); + expect( + tester.getBottomRight(dialogFinder).dx, + closeTo(tester.getBottomRight(finder).dx + unscaledValue * paddingScaleFactors[textScaleFactor], 1e-6), + ); + } + + void expectTopEdgePadding( + WidgetTester tester, { + Finder finder, + double textScaleFactor, + double unscaledValue, + }) { + expect( + tester.getTopLeft(dialogFinder).dy, + closeTo(tester.getTopLeft(finder).dy - unscaledValue * paddingScaleFactors[textScaleFactor], 1e-6), + ); + expect( + tester.getTopRight(dialogFinder).dy, + closeTo(tester.getTopRight(finder).dy - unscaledValue * paddingScaleFactors[textScaleFactor], 1e-6), + ); + } + + void expectBottomEdgePadding( + WidgetTester tester, { + Finder finder, + double textScaleFactor, + double unscaledValue, + }) { + expect( + tester.getBottomLeft(dialogFinder).dy, + closeTo(tester.getBottomRight(finder).dy + unscaledValue * paddingScaleFactors[textScaleFactor], 1e-6), + ); + expect( + tester.getBottomRight(dialogFinder).dy, + closeTo(tester.getBottomRight(finder).dy + unscaledValue * paddingScaleFactors[textScaleFactor], 1e-6), + ); + } + + void expectVerticalInnerPadding( + WidgetTester tester, { + Finder top, + Finder bottom, + double value, + }) { + expect( + tester.getBottomLeft(top).dy, + tester.getTopLeft(bottom).dy - value, + ); + expect( + tester.getBottomRight(top).dy, + tester.getTopRight(bottom).dy - value, + ); + } + + final Widget title = Text( + 'title', + key: titleKey, + ); + final Widget content = Text( + 'content', + key: contentKey, + ); + final List actions = [ + RaisedButton( + onPressed: () {}, + child: const Text('button'), + ), + ]; + final List children = [ + SimpleDialogOption( + key: childrenKey, + child: const Text('child'), + onPressed: () { }, + ), + ]; + + for (final double textScaleFactor in textScaleFactors) { + testWidgets('AlertDialog padding is correct when only title and actions are specified [textScaleFactor]=$textScaleFactor}', (WidgetTester tester) async { + final AlertDialog dialog = AlertDialog( + title: title, + actions: actions, + ); + + await openDialog(tester, dialog, textScaleFactor); + + expectTopEdgePadding( + tester, + finder: titleFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 24.0, + ); + expectLeftEdgePadding( + tester, + finder: titleFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 24.0, + ); + expectRightEdgePadding( + tester, + finder: titleFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 24.0, + ); + expectVerticalInnerPadding( + tester, + top: titleFinder, + bottom: actionsFinder, + value: 20.0, + ); + expectLeftEdgePadding( + tester, + finder: actionsFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 0.0, + ); + expectRightEdgePadding( + tester, + finder: actionsFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 0.0, + ); + expectBottomEdgePadding( + tester, + finder: actionsFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 0.0, + ); + }); + + testWidgets('AlertDialog padding is correct when only content and actions are specified [textScaleFactor]=$textScaleFactor}', (WidgetTester tester) async { + final AlertDialog dialog = AlertDialog( + content: content, + actions: actions, + ); + + await openDialog(tester, dialog, textScaleFactor); + + expectTopEdgePadding( + tester, + finder: contentFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 20.0, + ); + expectLeftEdgePadding( + tester, + finder: contentFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 24.0, + ); + expectRightEdgePadding( + tester, + finder: contentFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 24.0, + ); + expectVerticalInnerPadding( + tester, + top: contentFinder, + bottom: actionsFinder, + value: 24.0, + ); + expectLeftEdgePadding( + tester, + finder: actionsFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 0.0, + ); + expectRightEdgePadding( + tester, + finder: actionsFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 0.0, + ); + expectBottomEdgePadding( + tester, + finder: actionsFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 0.0, + ); + }); + + testWidgets('AlertDialog padding is correct when title, content, and actions are specified [textScaleFactor]=$textScaleFactor}', (WidgetTester tester) async { + final AlertDialog dialog = AlertDialog( + title: title, + content: content, + actions: actions, + ); + + await openDialog(tester, dialog, textScaleFactor); + + expectTopEdgePadding( + tester, + finder: titleFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 24.0, + ); + expectLeftEdgePadding( + tester, + finder: titleFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 24.0, + ); + expectRightEdgePadding( + tester, + finder: titleFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 24.0, + ); + expectVerticalInnerPadding( + tester, + top: titleFinder, + bottom: contentFinder, + value: 20.0, + ); + expectLeftEdgePadding( + tester, + finder: contentFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 24.0, + ); + expectRightEdgePadding( + tester, + finder: contentFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 24.0, + ); + expectVerticalInnerPadding( + tester, + top: contentFinder, + bottom: actionsFinder, + value: 24.0, + ); + expectLeftEdgePadding( + tester, + finder: actionsFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 0.0, + ); + expectRightEdgePadding( + tester, + finder: actionsFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 0.0, + ); + expectBottomEdgePadding( + tester, + finder: actionsFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 0.0, + ); + }); + + testWidgets('SimpleDialog padding is correct when only children are specified [textScaleFactor]=$textScaleFactor}', (WidgetTester tester) async { + final SimpleDialog dialog = SimpleDialog( + children: children, + ); + + await openDialog(tester, dialog, textScaleFactor); + + expectTopEdgePadding( + tester, + finder: childrenFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 12.0, + ); + expectLeftEdgePadding( + tester, + finder: childrenFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 0.0, + ); + expectRightEdgePadding( + tester, + finder: childrenFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 0.0, + ); + expectBottomEdgePadding( + tester, + finder: childrenFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 16.0, + ); + }); + + testWidgets('SimpleDialog padding is correct when title and children are specified [textScaleFactor]=$textScaleFactor}', (WidgetTester tester) async { + final SimpleDialog dialog = SimpleDialog( + title: title, + children: children, + ); + + await openDialog(tester, dialog, textScaleFactor); + + expectTopEdgePadding( + tester, + finder: titleFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 24.0, + ); + expectLeftEdgePadding( + tester, + finder: titleFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 24.0, + ); + expectRightEdgePadding( + tester, + finder: titleFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 24.0, + ); + expectVerticalInnerPadding( + tester, + top: titleFinder, + bottom: childrenFinder, + value: 12.0, + ); + expectLeftEdgePadding( + tester, + finder: childrenFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 0.0, + ); + expectRightEdgePadding( + tester, + finder: childrenFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 0.0, + ); + expectBottomEdgePadding( + tester, + finder: childrenFinder, + textScaleFactor: textScaleFactor, + unscaledValue: 16.0, + ); + }); + } + }); + testWidgets('Dialogs can set the vertical direction of overflowing actions', (WidgetTester tester) async { final GlobalKey key1 = GlobalKey(); final GlobalKey key2 = GlobalKey();