From d271791e8c0d2c8974453760f7b7b5a052367f9c Mon Sep 17 00:00:00 2001 From: Taha Tesser Date: Tue, 13 Feb 2024 02:12:02 +0200 Subject: [PATCH] Fix `insetPadding` parameter nullability for dialogs (#143305) fixes [`insetPadding` should be nullable in dialogs](https://github.com/flutter/flutter/issues/117125) --- packages/flutter/lib/src/material/dialog.dart | 15 ++++---- .../lib/src/material/dialog_theme.dart | 26 ++++++++++++-- .../flutter/test/material/dialog_test.dart | 21 ++++++++++-- .../test/material/dialog_theme_test.dart | 34 +++++++++++++++++++ 4 files changed, 84 insertions(+), 12 deletions(-) diff --git a/packages/flutter/lib/src/material/dialog.dart b/packages/flutter/lib/src/material/dialog.dart index 24f8c8d462..cb2af1eb44 100644 --- a/packages/flutter/lib/src/material/dialog.dart +++ b/packages/flutter/lib/src/material/dialog.dart @@ -54,7 +54,7 @@ class Dialog extends StatelessWidget { this.surfaceTintColor, this.insetAnimationDuration = const Duration(milliseconds: 100), this.insetAnimationCurve = Curves.decelerate, - this.insetPadding = _defaultInsetPadding, + this.insetPadding, this.clipBehavior = Clip.none, this.shape, this.alignment, @@ -215,7 +215,8 @@ class Dialog extends StatelessWidget { Widget build(BuildContext context) { final ThemeData theme = Theme.of(context); final DialogTheme dialogTheme = DialogTheme.of(context); - final EdgeInsets effectivePadding = MediaQuery.viewInsetsOf(context) + (insetPadding ?? EdgeInsets.zero); + final EdgeInsets effectivePadding = MediaQuery.viewInsetsOf(context) + + (insetPadding ?? dialogTheme.insetPadding ?? _defaultInsetPadding); final DialogTheme defaults = theme.useMaterial3 ? (_fullscreen ? _DialogFullscreenDefaultsM3(context) : _DialogDefaultsM3(context)) : _DialogDefaultsM2(context); @@ -387,7 +388,7 @@ class AlertDialog extends StatelessWidget { this.shadowColor, this.surfaceTintColor, this.semanticLabel, - this.insetPadding = _defaultInsetPadding, + this.insetPadding, this.clipBehavior = Clip.none, this.shape, this.alignment, @@ -676,7 +677,7 @@ class AlertDialog extends StatelessWidget { final String? semanticLabel; /// {@macro flutter.material.dialog.insetPadding} - final EdgeInsets insetPadding; + final EdgeInsets? insetPadding; /// {@macro flutter.material.dialog.clipBehavior} final Clip clipBehavior; @@ -912,7 +913,7 @@ class _AdaptiveAlertDialog extends AlertDialog { super.shadowColor, super.surfaceTintColor, super.semanticLabel, - super.insetPadding = _defaultInsetPadding, + super.insetPadding, super.clipBehavior = Clip.none, super.shape, super.alignment, @@ -1110,7 +1111,7 @@ class SimpleDialog extends StatelessWidget { this.shadowColor, this.surfaceTintColor, this.semanticLabel, - this.insetPadding = _defaultInsetPadding, + this.insetPadding, this.clipBehavior = Clip.none, this.shape, this.alignment, @@ -1185,7 +1186,7 @@ class SimpleDialog extends StatelessWidget { final String? semanticLabel; /// {@macro flutter.material.dialog.insetPadding} - final EdgeInsets insetPadding; + final EdgeInsets? insetPadding; /// {@macro flutter.material.dialog.clipBehavior} final Clip clipBehavior; diff --git a/packages/flutter/lib/src/material/dialog_theme.dart b/packages/flutter/lib/src/material/dialog_theme.dart index ba5fc19ad6..26d9c39b14 100644 --- a/packages/flutter/lib/src/material/dialog_theme.dart +++ b/packages/flutter/lib/src/material/dialog_theme.dart @@ -39,6 +39,7 @@ class DialogTheme with Diagnosticable { this.contentTextStyle, this.actionsPadding, this.barrierColor, + this.insetPadding, }); /// Overrides the default value for [Dialog.backgroundColor]. @@ -76,6 +77,9 @@ class DialogTheme with Diagnosticable { /// Overrides the default value for [barrierColor] in [showDialog]. final Color? barrierColor; + /// Overrides the default value for [Dialog.insetPadding]. + final EdgeInsets? insetPadding; + /// Creates a copy of this object but with the given fields replaced with the /// new values. DialogTheme copyWith({ @@ -90,6 +94,7 @@ class DialogTheme with Diagnosticable { TextStyle? contentTextStyle, EdgeInsetsGeometry? actionsPadding, Color? barrierColor, + EdgeInsets? insetPadding, }) { return DialogTheme( backgroundColor: backgroundColor ?? this.backgroundColor, @@ -103,6 +108,7 @@ class DialogTheme with Diagnosticable { contentTextStyle: contentTextStyle ?? this.contentTextStyle, actionsPadding: actionsPadding ?? this.actionsPadding, barrierColor: barrierColor ?? this.barrierColor, + insetPadding: insetPadding ?? this.insetPadding, ); } @@ -130,11 +136,25 @@ class DialogTheme with Diagnosticable { contentTextStyle: TextStyle.lerp(a?.contentTextStyle, b?.contentTextStyle, t), actionsPadding: EdgeInsetsGeometry.lerp(a?.actionsPadding, b?.actionsPadding, t), barrierColor: Color.lerp(a?.barrierColor, b?.barrierColor, t), + insetPadding: EdgeInsets.lerp(a?.insetPadding, b?.insetPadding, t), ); } @override - int get hashCode => shape.hashCode; + int get hashCode => Object.hashAll([ + backgroundColor, + elevation, + shadowColor, + surfaceTintColor, + shape, + alignment, + iconColor, + titleTextStyle, + contentTextStyle, + actionsPadding, + barrierColor, + insetPadding, + ]); @override bool operator ==(Object other) { @@ -155,7 +175,8 @@ class DialogTheme with Diagnosticable { && other.titleTextStyle == titleTextStyle && other.contentTextStyle == contentTextStyle && other.actionsPadding == actionsPadding - && other.barrierColor == barrierColor; + && other.barrierColor == barrierColor + && other.insetPadding == insetPadding; } @override @@ -172,5 +193,6 @@ class DialogTheme with Diagnosticable { properties.add(DiagnosticsProperty('contentTextStyle', contentTextStyle, defaultValue: null)); properties.add(DiagnosticsProperty('actionsPadding', actionsPadding, defaultValue: null)); properties.add(ColorProperty('barrierColor', barrierColor)); + properties.add(DiagnosticsProperty('insetPadding', insetPadding, defaultValue: null)); } } diff --git a/packages/flutter/test/material/dialog_test.dart b/packages/flutter/test/material/dialog_test.dart index ac530f8524..9415e58aae 100644 --- a/packages/flutter/test/material/dialog_test.dart +++ b/packages/flutter/test/material/dialog_test.dart @@ -1656,12 +1656,12 @@ void main() { // The default testing screen (800, 600) const Rect screenRect = Rect.fromLTRB(0.0, 0.0, 800.0, 600.0); - // Test with no padding + // Test with no padding. await tester.pumpWidget( const MediaQuery( data: MediaQueryData(), child: Dialog( - insetPadding: null, + insetPadding: EdgeInsets.zero, child: Placeholder(), ), ), @@ -1669,7 +1669,7 @@ void main() { await tester.pumpAndSettle(); expect(tester.getRect(find.byType(Placeholder)), screenRect); - // Test with an insetPadding + // Test with an insetPadding. await tester.pumpWidget( const MediaQuery( data: MediaQueryData(), @@ -2841,6 +2841,21 @@ void main() { expect(okNode.hasFocus, false); expect(cancelNode.hasFocus, false); }); + + testWidgets('Dialog.insetPadding is nullable', (WidgetTester tester) async { + const Dialog dialog = Dialog(); + expect(dialog.insetPadding, isNull); + }); + + testWidgets('AlertDialog.insetPadding is nullable', (WidgetTester tester) async { + const AlertDialog alertDialog = AlertDialog(); + expect(alertDialog.insetPadding, isNull); + }); + + testWidgets('SimpleDialog.insetPadding is nullable', (WidgetTester tester) async { + const SimpleDialog simpleDialog = SimpleDialog(); + expect(simpleDialog.insetPadding, isNull); + }); } class _RestorableDialogTestWidget extends StatelessWidget { diff --git a/packages/flutter/test/material/dialog_theme_test.dart b/packages/flutter/test/material/dialog_theme_test.dart index 66ed2cf0ef..04c2bcb9cf 100644 --- a/packages/flutter/test/material/dialog_theme_test.dart +++ b/packages/flutter/test/material/dialog_theme_test.dart @@ -48,6 +48,11 @@ RenderParagraph _getTextRenderObject(WidgetTester tester, String text) { } void main() { + test('DialogTheme copyWith, ==, hashCode basics', () { + expect(const DialogTheme(), const DialogTheme().copyWith()); + expect(const DialogTheme().hashCode, const DialogTheme().copyWith().hashCode); + }); + test('DialogTheme lerp special cases', () { expect(DialogTheme.lerp(null, null, 0), const DialogTheme()); const DialogTheme theme = DialogTheme(); @@ -67,6 +72,7 @@ void main() { contentTextStyle: TextStyle(color: Color(0xff000000)), actionsPadding: EdgeInsets.all(8.0), barrierColor: Color(0xff000005), + insetPadding: EdgeInsets.all(20.0), ).debugFillProperties(builder); final List description = builder.properties .where((DiagnosticsNode n) => !n.isFiltered(DiagnosticLevel.info)) @@ -82,6 +88,7 @@ void main() { 'contentTextStyle: TextStyle(inherit: true, color: Color(0xff000000))', 'actionsPadding: EdgeInsets.all(8.0)', 'barrierColor: Color(0xff000005)', + 'insetPadding: EdgeInsets.all(20.0)', ]); }); @@ -514,4 +521,31 @@ void main() { final ModalBarrier modalBarrier = tester.widget(find.byType(ModalBarrier).last); expect(modalBarrier.color, barrierColor); }); + + testWidgets('DialogTheme.insetPadding updates Dialog insetPadding', (WidgetTester tester) async { + // The default testing screen (800, 600) + const Rect screenRect = Rect.fromLTRB(0.0, 0.0, 800.0, 600.0); + const DialogTheme dialogTheme = DialogTheme( + insetPadding: EdgeInsets.fromLTRB(10, 15, 20, 25) + ); + const Dialog dialog = Dialog(child: Placeholder()); + + await tester.pumpWidget(_appWithDialog( + tester, + dialog, + theme: ThemeData(dialogTheme: dialogTheme), + )); + await tester.tap(find.text('X')); + await tester.pump(); + + expect( + tester.getRect(find.byType(Placeholder)), + Rect.fromLTRB( + screenRect.left + dialogTheme.insetPadding!.left, + screenRect.top + dialogTheme.insetPadding!.top, + screenRect.right - dialogTheme.insetPadding!.right, + screenRect.bottom - dialogTheme.insetPadding!.bottom, + ), + ); + }); }