From c461ff9d4a82b44cabd21f6c45aff0753bbd2835 Mon Sep 17 00:00:00 2001 From: Shi-Hao Hong Date: Mon, 28 Oct 2019 13:45:05 -0700 Subject: [PATCH] Implement AlertDialog title/content overflow scroll (#43226) * Wrap alert dialog title and content in single child scroll view * Scrollable alert dialog title and content tests * Remove unnecessary comment * Fix mainAxisSize and crossAxisAlignment issue --- packages/flutter/lib/src/material/dialog.dart | 66 ++++++++-------- .../flutter/test/material/dialog_test.dart | 77 +++++++++++++++++++ .../test/material/dialog_theme_test.dart | 34 ++++---- 3 files changed, 126 insertions(+), 51 deletions(-) diff --git a/packages/flutter/lib/src/material/dialog.dart b/packages/flutter/lib/src/material/dialog.dart index 7c00022c19..1314cd0a3b 100644 --- a/packages/flutter/lib/src/material/dialog.dart +++ b/packages/flutter/lib/src/material/dialog.dart @@ -133,15 +133,6 @@ class Dialog extends StatelessWidget { /// of actions. The title is displayed above the content and the actions are /// displayed below the content. /// -/// If the content is too large to fit on the screen vertically, the dialog will -/// display the title and the actions and let the content overflow, which is -/// rarely desired. Consider using a scrolling widget for [content], such as -/// [SingleChildScrollView], to avoid overflow. (However, be aware that since -/// [AlertDialog] tries to size itself using the intrinsic dimensions of its -/// children, widgets such as [ListView], [GridView], and [CustomScrollView], -/// which use lazy viewports, will not work. If this is a problem, consider -/// using [Dialog] directly.) -/// /// For dialogs that offer the user a choice between several options, consider /// using a [SimpleDialog]. /// @@ -161,13 +152,11 @@ class Dialog extends StatelessWidget { /// builder: (BuildContext context) { /// return AlertDialog( /// title: Text('Rewind and remember'), -/// content: SingleChildScrollView( -/// child: ListBody( -/// children: [ -/// Text('You will never be satisfied.'), -/// Text('You\’re like me. I’m never satisfied.'), -/// ], -/// ), +/// content: Column( +/// children: [ +/// Text('You will never be satisfied.'), +/// Text('You\’re like me. I’m never satisfied.'), +/// ], /// ), /// actions: [ /// FlatButton( @@ -321,25 +310,34 @@ class AlertDialog extends StatelessWidget { mainAxisSize: MainAxisSize.min, crossAxisAlignment: CrossAxisAlignment.stretch, children: [ - if (title != null) - Padding( - padding: titlePadding ?? EdgeInsets.fromLTRB(24.0, 24.0, 24.0, content == null ? 20.0 : 0.0), - child: DefaultTextStyle( - style: titleTextStyle ?? dialogTheme.titleTextStyle ?? theme.textTheme.title, - child: Semantics( - child: title, - namesRoute: true, - container: true, - ), - ), - ), - if (content != null) + if (title != null || content != null) Flexible( - child: Padding( - padding: contentPadding, - child: DefaultTextStyle( - style: contentTextStyle ?? dialogTheme.contentTextStyle ?? theme.textTheme.subhead, - child: content, + child: SingleChildScrollView( + child: Column( + mainAxisSize: MainAxisSize.min, + crossAxisAlignment: CrossAxisAlignment.stretch, + children: [ + if (title != null) + Padding( + padding: titlePadding ?? EdgeInsets.fromLTRB(24.0, 24.0, 24.0, content == null ? 20.0 : 0.0), + child: DefaultTextStyle( + style: titleTextStyle ?? dialogTheme.titleTextStyle ?? theme.textTheme.title, + child: Semantics( + child: title, + namesRoute: true, + container: true, + ), + ), + ), + if (content != null) + Padding( + padding: contentPadding, + child: DefaultTextStyle( + style: contentTextStyle ?? dialogTheme.contentTextStyle ?? theme.textTheme.subhead, + child: content, + ), + ), + ], ), ), ), diff --git a/packages/flutter/test/material/dialog_test.dart b/packages/flutter/test/material/dialog_test.dart index d93415addc..0bceb811a7 100644 --- a/packages/flutter/test/material/dialog_test.dart +++ b/packages/flutter/test/material/dialog_test.dart @@ -645,4 +645,81 @@ void main() { await tester.pumpWidget(buildFrame(UniqueKey())); await tester.pump(); }); + + group('Scrollable title and content', () { + testWidgets('Title is scrollable', (WidgetTester tester) async { + final Key titleKey = UniqueKey(); + final AlertDialog dialog = AlertDialog( + title: Container( + key: titleKey, + color: Colors.green, + height: 1000, + ), + ); + await tester.pumpWidget(_appWithAlertDialog(tester, dialog)); + await tester.tap(find.text('X')); + await tester.pumpAndSettle(); + + final RenderBox box = tester.renderObject(find.byKey(titleKey)); + final Offset originalOffset = box.localToGlobal(Offset.zero); + await tester.drag(find.byKey(titleKey), const Offset(0.0, -200.0)); + expect(box.localToGlobal(Offset.zero), equals(originalOffset.translate(0.0, -200.0))); + }); + + testWidgets('Content is scrollable', (WidgetTester tester) async { + final Key contentKey = UniqueKey(); + final AlertDialog dialog = AlertDialog( + content: Container( + key: contentKey, + color: Colors.orange, + height: 1000, + ), + ); + await tester.pumpWidget(_appWithAlertDialog(tester, dialog)); + await tester.tap(find.text('X')); + await tester.pumpAndSettle(); + + final RenderBox box = tester.renderObject(find.byKey(contentKey)); + final Offset originalOffset = box.localToGlobal(Offset.zero); + await tester.drag(find.byKey(contentKey), const Offset(0.0, -200.0)); + expect(box.localToGlobal(Offset.zero), equals(originalOffset.translate(0.0, -200.0))); + }); + + testWidgets('Title and content are scrollable', (WidgetTester tester) async { + final Key titleKey = UniqueKey(); + final Key contentKey = UniqueKey(); + final AlertDialog dialog = AlertDialog( + title: Container( + key: titleKey, + color: Colors.green, + height: 400, + ), + content: Container( + key: contentKey, + color: Colors.orange, + height: 400, + ), + ); + await tester.pumpWidget(_appWithAlertDialog(tester, dialog)); + await tester.tap(find.text('X')); + await tester.pumpAndSettle(); + + final RenderBox title = tester.renderObject(find.byKey(titleKey)); + final RenderBox content = tester.renderObject(find.byKey(contentKey)); + final Offset titleOriginalOffset = title.localToGlobal(Offset.zero); + final Offset contentOriginalOffset = content.localToGlobal(Offset.zero); + + // Dragging the title widget should scroll both the title + // and the content widgets. + await tester.drag(find.byKey(titleKey), const Offset(0.0, -200.0)); + expect(title.localToGlobal(Offset.zero), equals(titleOriginalOffset.translate(0.0, -200.0))); + expect(content.localToGlobal(Offset.zero), equals(contentOriginalOffset.translate(0.0, -200.0))); + + // Dragging the content widget should scroll both the title + // and the content widgets. + await tester.drag(find.byKey(contentKey), const Offset(0.0, 200.0)); + expect(title.localToGlobal(Offset.zero), equals(titleOriginalOffset)); + expect(content.localToGlobal(Offset.zero), equals(contentOriginalOffset)); + }); + }); } diff --git a/packages/flutter/test/material/dialog_theme_test.dart b/packages/flutter/test/material/dialog_theme_test.dart index 834cefebbf..fc2d491160 100644 --- a/packages/flutter/test/material/dialog_theme_test.dart +++ b/packages/flutter/test/material/dialog_theme_test.dart @@ -10,23 +10,23 @@ MaterialApp _appWithAlertDialog(WidgetTester tester, AlertDialog dialog, { Theme return MaterialApp( theme: theme, home: Material( - child: Builder( - builder: (BuildContext context) { - return Center( - child: RaisedButton( - child: const Text('X'), - onPressed: () { - showDialog( - context: context, - builder: (BuildContext context) { - return RepaintBoundary(key: _painterKey, child: dialog); - }, - ); - }, - ), - ); - } - ), + child: Builder( + builder: (BuildContext context) { + return Center( + child: RaisedButton( + child: const Text('X'), + onPressed: () { + showDialog( + context: context, + builder: (BuildContext context) { + return RepaintBoundary(key: _painterKey, child: dialog); + }, + ); + }, + ), + ); + }, + ), ), ); }