diff --git a/packages/flutter/lib/src/material/about.dart b/packages/flutter/lib/src/material/about.dart index 6cb5ca352c..caffabcf64 100644 --- a/packages/flutter/lib/src/material/about.dart +++ b/packages/flutter/lib/src/material/about.dart @@ -138,6 +138,9 @@ class AboutListTile extends StatelessWidget { /// /// The licenses shown on the [LicensePage] are those returned by the /// [LicenseRegistry] API, which can be used to add more licenses to the list. +/// +/// The `context` argument is passed to [showDialog], the documentation for +/// which discusses how it is used. void showAboutDialog({ @required BuildContext context, String applicationName, diff --git a/packages/flutter/lib/src/material/bottom_sheet.dart b/packages/flutter/lib/src/material/bottom_sheet.dart index dca197391c..f5a0fa6150 100644 --- a/packages/flutter/lib/src/material/bottom_sheet.dart +++ b/packages/flutter/lib/src/material/bottom_sheet.dart @@ -252,6 +252,11 @@ class _ModalBottomSheetRoute extends PopupRoute { /// preventing the use from interacting with the app. Persistent bottom sheets /// can be created and displayed with the [ScaffoldState.showBottomSheet] function. /// +/// The `context` argument is used to look up the [Navigator] and [Theme] for +/// the bottom sheet. It is only used when the method is called. Its +/// corresponding widget can be safely removed from the tree before the bottom +/// sheet is closed. +/// /// Returns a `Future` that resolves to the value (if any) that was passed to /// [Navigator.pop] when the modal bottom sheet was closed. /// diff --git a/packages/flutter/lib/src/material/date_picker.dart b/packages/flutter/lib/src/material/date_picker.dart index f570ba0890..acef552084 100644 --- a/packages/flutter/lib/src/material/date_picker.dart +++ b/packages/flutter/lib/src/material/date_picker.dart @@ -934,6 +934,9 @@ typedef bool SelectableDayPredicate(DateTime day); /// provided by [Directionality]. If both [locale] and [textDirection] are not /// null, [textDirection] overrides the direction chosen for the [locale]. /// +/// The `context` argument is passed to [showDialog], the documentation for +/// which discusses how it is used. +/// /// See also: /// /// * [showTimePicker] diff --git a/packages/flutter/lib/src/material/dialog.dart b/packages/flutter/lib/src/material/dialog.dart index 97190f3bfc..bbc5b987a3 100644 --- a/packages/flutter/lib/src/material/dialog.dart +++ b/packages/flutter/lib/src/material/dialog.dart @@ -447,6 +447,10 @@ class _DialogRoute extends PopupRoute { /// This function typically receives a [Dialog] widget as its child argument. /// Content below the dialog is dimmed with a [ModalBarrier]. /// +/// The `context` argument is used to look up the [Navigator] and [Theme] for +/// the dialog. It is only used when the method is called. Its corresponding +/// widget can be safely removed from the tree before the dialog is closed. +/// /// Returns a [Future] that resolves to the value (if any) that was passed to /// [Navigator.pop] when the dialog was closed. /// diff --git a/packages/flutter/lib/src/material/popup_menu.dart b/packages/flutter/lib/src/material/popup_menu.dart index 4e526a46e5..d0b9867216 100644 --- a/packages/flutter/lib/src/material/popup_menu.dart +++ b/packages/flutter/lib/src/material/popup_menu.dart @@ -447,7 +447,7 @@ class _PopupMenu extends StatelessWidget { type: MaterialType.card, elevation: route.elevation, child: new Align( - alignment: Alignment.topRight, + alignment: AlignmentDirectional.topEnd, widthFactor: width.evaluate(route.animation), heightFactor: height.evaluate(route.animation), child: child, @@ -460,37 +460,76 @@ class _PopupMenu extends StatelessWidget { } } +// Positioning of the menu on the screen. class _PopupMenuRouteLayout extends SingleChildLayoutDelegate { - _PopupMenuRouteLayout(this.position, this.selectedItemOffset); + _PopupMenuRouteLayout(this.position, this.selectedItemOffset, this.textDirection); + // Rectangle of underlying button, relative to the overlay's dimensions. final RelativeRect position; + + // The distance from the top of the menu to the middle of selected item. + // + // This will be null if there's no item to position in this way. final double selectedItemOffset; + // Whether to prefer going to the left or to the right. + final TextDirection textDirection; + + // We put the child wherever position specifies, so long as it will fit within + // the specified parent size padded (inset) by 8. If necessary, we adjust the + // child's position so that it fits. + @override BoxConstraints getConstraintsForChild(BoxConstraints constraints) { - return constraints.loosen(); + // The menu can be at most the size of the overlay minus 8.0 pixels in each + // direction. + return new BoxConstraints.loose(constraints.biggest - const Offset(_kMenuScreenPadding * 2.0, _kMenuScreenPadding * 2.0)); } - // Put the child wherever position specifies, so long as it will fit within the - // specified parent size padded (inset) by 8. If necessary, adjust the child's - // position so that it fits. @override Offset getPositionForChild(Size size, Size childSize) { - double x = position?.left - ?? (position?.right != null ? size.width - (position.right + childSize.width) : _kMenuScreenPadding); - double y = position?.top - ?? (position?.bottom != null ? size.height - (position.bottom - childSize.height) : _kMenuScreenPadding); + // size: The size of the overlay. + // childSize: The size of the menu, when fully open, as determined by + // getConstraintsForChild. - if (selectedItemOffset != null) - y -= selectedItemOffset + _kMenuVerticalPadding; + // Find the ideal vertical position. + double y; + if (selectedItemOffset == null) { + y = position.top; + } else { + y = position.top + (size.height - position.top - position.bottom) / 2.0 - selectedItemOffset; + } + // Find the ideal horizontal position. + double x; + if (position.left > position.right) { + // Menu button is closer to the right edge, so grow to the left, aligned to the right edge. + x = size.width - position.right - childSize.width; + } else if (position.left < position.right) { + // Menu button is closer to the left edge, so grow to the right, aligned to the left edge. + x = position.left; + } else { + // Menu button is equidistant from both edges, so grow in reading direction. + assert(textDirection != null); + switch (textDirection) { + case TextDirection.rtl: + x = size.width - position.right - childSize.width; + break; + case TextDirection.ltr: + x = position.left; + break; + } + } + + // Avoid going outside an area defined as the rectangle 8.0 pixels from the + // edge of the screen in every direction. if (x < _kMenuScreenPadding) x = _kMenuScreenPadding; - else if (x + childSize.width > size.width - 2 * _kMenuScreenPadding) + else if (x + childSize.width > size.width - _kMenuScreenPadding) x = size.width - childSize.width - _kMenuScreenPadding; if (y < _kMenuScreenPadding) y = _kMenuScreenPadding; - else if (y + childSize.height > size.height - 2 * _kMenuScreenPadding) + else if (y + childSize.height > size.height - _kMenuScreenPadding) y = size.height - childSize.height - _kMenuScreenPadding; return new Offset(x, y); } @@ -538,13 +577,13 @@ class _PopupMenuRoute extends PopupRoute { Widget buildPage(BuildContext context, Animation animation, Animation secondaryAnimation) { double selectedItemOffset; if (initialValue != null) { - selectedItemOffset = 0.0; + double y = _kMenuVerticalPadding; for (PopupMenuEntry entry in items) { if (entry.represents(initialValue)) { - selectedItemOffset += entry.height / 2.0; + selectedItemOffset = y + entry.height / 2.0; break; } - selectedItemOffset += entry.height; + y += entry.height; } } @@ -552,25 +591,41 @@ class _PopupMenuRoute extends PopupRoute { if (theme != null) menu = new Theme(data: theme, child: menu); - return new CustomSingleChildLayout( - delegate: new _PopupMenuRouteLayout(position, selectedItemOffset), - child: menu, + return new Builder( + builder: (BuildContext context) { + return new CustomSingleChildLayout( + delegate: new _PopupMenuRouteLayout( + position, + selectedItemOffset, + Directionality.of(context), + ), + child: menu, + ); + }, ); } } -/// Show a popup menu that contains the `items` at `position`. If `initialValue` -/// is specified then the first item with a matching value will be highlighted -/// and the value of `position` implies where the left, center point of the -/// highlighted item should appear. If `initialValue` is not specified then -/// `position` specifies the menu's origin. +/// Show a popup menu that contains the `items` at `position`. /// -/// The `context` argument is used to look up a [Navigator] to show the menu and -/// a [Theme] to use for the menu. +/// If `initialValue` is specified then the first item with a matching value +/// will be highlighted and the value of `position` gives the rectangle whose +/// vertical center will be aligned with the vertical center of the highlighted +/// item (when possible). /// -/// The `elevation` argument specifies the z-coordinate at which to place the -/// menu. The elevation defaults to 8, the appropriate elevation for popup -/// menus. +/// If `initialValue` is not specified then the top of the menu will be aligned +/// with the top of the `position` rectangle. +/// +/// In both cases, the menu position will be adjusted if necessary to fit on the +/// screen. +/// +/// Horizontally, the menu is positioned so that it grows in the direction that +/// has the most room. For example, if the `position` describes a rectangle on +/// the left edge of the screen, then the left edge of the menu is aligned with +/// the left edge of the `position`, and the menu grows to the right. If both +/// edges of the `position` are equidistant from the opposite edge of the +/// screen, then the ambient [Directionality] is used as a tie-breaker, +/// preferring to grow in the reading direction. /// /// The positioning of the `initialValue` at the `position` is implemented by /// iterating over the `items` to find the first whose @@ -578,6 +633,14 @@ class _PopupMenuRoute extends PopupRoute { /// summing the values of [PopupMenuEntry.height] for all the preceding widgets /// in the list. /// +/// The `elevation` argument specifies the z-coordinate at which to place the +/// menu. The elevation defaults to 8, the appropriate elevation for popup +/// menus. +/// +/// The `context` argument is used to look up the [Navigator] and [Theme] for +/// the menu. It is only used when the method is called. Its corresponding +/// widget can be safely removed from the tree before the popup menu is closed. +/// /// See also: /// /// * [PopupMenuItem], a popup menu entry for a single value. @@ -590,7 +653,7 @@ Future showMenu({ RelativeRect position, @required List> items, T initialValue, - double elevation: 8.0 + double elevation: 8.0, }) { assert(context != null); assert(items != null && items.isNotEmpty); @@ -722,19 +785,21 @@ class PopupMenuButton extends StatefulWidget { class _PopupMenuButtonState extends State> { void showButtonMenu() { - final RenderBox renderBox = context.findRenderObject(); - final Offset topLeft = renderBox.localToGlobal(Offset.zero); + final RenderBox button = context.findRenderObject(); + final RenderBox overlay = Overlay.of(context).context.findRenderObject(); + final RelativeRect position = new RelativeRect.fromRect( + new Rect.fromPoints( + button.localToGlobal(Offset.zero, ancestor: overlay), + button.localToGlobal(button.size.bottomRight(Offset.zero), ancestor: overlay), + ), + Offset.zero & overlay.size, + ); showMenu( context: context, elevation: widget.elevation, items: widget.itemBuilder(context), initialValue: widget.initialValue, - position: new RelativeRect.fromLTRB( - topLeft.dx, - topLeft.dy + (widget.initialValue != null ? renderBox.size.height / 2.0 : 0.0), - 0.0, - 0.0, - ) + position: position, ) .then((T newValue) { if (!mounted || newValue == null) diff --git a/packages/flutter/lib/src/material/time_picker.dart b/packages/flutter/lib/src/material/time_picker.dart index dc6339f813..ef1f80cfed 100644 --- a/packages/flutter/lib/src/material/time_picker.dart +++ b/packages/flutter/lib/src/material/time_picker.dart @@ -1249,13 +1249,17 @@ class _TimePickerDialogState extends State<_TimePickerDialog> { /// closes the dialog. If the user cancels the dialog, null is returned. /// /// To show a dialog with [initialTime] equal to the current time: +/// /// ```dart /// showTimePicker( /// initialTime: new TimeOfDay.now(), -/// context: context +/// context: context, /// ); /// ``` /// +/// The `context` argument is passed to [showDialog], the documentation for +/// which discusses how it is used. +/// /// See also: /// /// * [showDatePicker] diff --git a/packages/flutter/lib/src/rendering/object.dart b/packages/flutter/lib/src/rendering/object.dart index 1917ac97a8..f70f963519 100644 --- a/packages/flutter/lib/src/rendering/object.dart +++ b/packages/flutter/lib/src/rendering/object.dart @@ -3076,7 +3076,7 @@ abstract class _InterestingSemanticsFragment extends _SemanticsFragment { } } -/// A [_InterestingSemanticsFragment] that produces the root [SemanticsNode] of +/// An [_InterestingSemanticsFragment] that produces the root [SemanticsNode] of /// the semantics tree. /// /// The root node is available as only element in the Iterable returned by @@ -3130,7 +3130,7 @@ class _RootSemanticsFragment extends _InterestingSemanticsFragment { } } -/// A [_InterstingSemanticsFragment] that can be told to only add explicit +/// An [_InterestingSemanticsFragment] that can be told to only add explicit /// [SemanticsNode]s to the parent. /// /// If [markAsExplicit] was not called before this fragment is added to diff --git a/packages/flutter/lib/src/rendering/stack.dart b/packages/flutter/lib/src/rendering/stack.dart index 73273fa42f..bcd6646358 100644 --- a/packages/flutter/lib/src/rendering/stack.dart +++ b/packages/flutter/lib/src/rendering/stack.dart @@ -103,10 +103,24 @@ class RelativeRect { } /// Convert this [RelativeRect] to a [Rect], in the coordinate space of the container. + /// + /// See also: + /// + /// * [toSize], which returns the size part of the rect, based on the size of + /// the container. Rect toRect(Rect container) { return new Rect.fromLTRB(left, top, container.width - right, container.height - bottom); } + /// Convert this [RelativeRect] to a [Size], assuming a container with the given size. + /// + /// See also: + /// + /// * [toRect], which also computes the position relative to the container. + Size toSize(Size container) { + return new Size(container.width - left - right, container.height - top - bottom); + } + /// Linearly interpolate between two RelativeRects. /// /// If either rect is null, this function interpolates from [RelativeRect.fill]. diff --git a/packages/flutter/test/material/popup_menu_test.dart b/packages/flutter/test/material/popup_menu_test.dart index 7ef41b14b6..51401a1ca4 100644 --- a/packages/flutter/test/material/popup_menu_test.dart +++ b/packages/flutter/test/material/popup_menu_test.dart @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:ui' show window; + import 'package:flutter_test/flutter_test.dart'; import 'package:flutter/material.dart'; @@ -92,7 +94,6 @@ void main() { }); group('PopupMenuButton with Icon', () { - // Helper function to create simple and valid popup menus. List> simplePopupMenuItemBuilder(BuildContext context) { return >[ @@ -132,4 +133,217 @@ void main() { expect(find.byIcon(Icons.view_carousel), findsOneWidget); }); }); + + testWidgets('PopupMenu positioning', (WidgetTester tester) async { + final Widget testButton = new PopupMenuButton( + itemBuilder: (BuildContext context) { + return >[ + const PopupMenuItem(value: 1, child: const Text('AAA')), + const PopupMenuItem(value: 2, child: const Text('BBB')), + const PopupMenuItem(value: 3, child: const Text('CCC')), + ]; + }, + child: const SizedBox( + height: 100.0, + width: 100.0, + child: const Text('XXX'), + ), + ); + final WidgetPredicate popupMenu = (Widget widget) => widget.runtimeType.toString() == '_PopupMenu'; + + Future openMenu(TextDirection textDirection, Alignment alignment) async { + return TestAsyncUtils.guard(() async { + await tester.pumpWidget(new Container()); // reset in case we had a menu up already + await tester.pumpWidget(new TestApp( + textDirection: textDirection, + child: new Align( + alignment: alignment, + child: testButton, + ), + )); + await tester.tap(find.text('XXX')); + await tester.pump(); + }); + } + + Future testPositioningDown( + WidgetTester tester, + TextDirection textDirection, + Alignment alignment, + TextDirection growthDirection, + Rect startRect, + ) { + return TestAsyncUtils.guard(() async { + await openMenu(textDirection, alignment); + Rect rect = tester.getRect(find.byWidgetPredicate(popupMenu)); + expect(rect, startRect); + bool doneVertically = false; + bool doneHorizontally = false; + do { + await tester.pump(const Duration(milliseconds: 20)); + final Rect newRect = tester.getRect(find.byWidgetPredicate(popupMenu)); + expect(newRect.top, rect.top); + if (doneVertically) { + expect(newRect.bottom, rect.bottom); + } else { + if (newRect.bottom == rect.bottom) { + doneVertically = true; + } else { + expect(newRect.bottom, greaterThan(rect.bottom)); + } + } + switch (growthDirection) { + case TextDirection.rtl: + expect(newRect.right, rect.right); + if (doneHorizontally) { + expect(newRect.left, rect.left); + } else { + if (newRect.left == rect.left) { + doneHorizontally = true; + } else { + expect(newRect.left, lessThan(rect.left)); + } + } + break; + case TextDirection.ltr: + expect(newRect.left, rect.left); + if (doneHorizontally) { + expect(newRect.right, rect.right); + } else { + if (newRect.right == rect.right) { + doneHorizontally = true; + } else { + expect(newRect.right, greaterThan(rect.right)); + } + } + break; + } + rect = newRect; + } while (tester.binding.hasScheduledFrame); + }); + } + + Future testPositioningDownThenUp( + WidgetTester tester, + TextDirection textDirection, + Alignment alignment, + TextDirection growthDirection, + Rect startRect, + ) { + return TestAsyncUtils.guard(() async { + await openMenu(textDirection, alignment); + Rect rect = tester.getRect(find.byWidgetPredicate(popupMenu)); + expect(rect, startRect); + int verticalStage = 0; // 0=down, 1=up, 2=done + bool doneHorizontally = false; + do { + await tester.pump(const Duration(milliseconds: 20)); + final Rect newRect = tester.getRect(find.byWidgetPredicate(popupMenu)); + switch (verticalStage) { + case 0: + if (newRect.top < rect.top) { + verticalStage = 1; + expect(newRect.bottom, greaterThanOrEqualTo(rect.bottom)); + break; + } + expect(newRect.top, rect.top); + expect(newRect.bottom, greaterThan(rect.bottom)); + break; + case 1: + if (newRect.top == rect.top) { + verticalStage = 2; + expect(newRect.bottom, rect.bottom); + break; + } + expect(newRect.top, lessThan(rect.top)); + expect(newRect.bottom, rect.bottom); + break; + case 2: + expect(newRect.bottom, rect.bottom); + expect(newRect.top, rect.top); + break; + default: + assert(false); + } + switch (growthDirection) { + case TextDirection.rtl: + expect(newRect.right, rect.right); + if (doneHorizontally) { + expect(newRect.left, rect.left); + } else { + if (newRect.left == rect.left) { + doneHorizontally = true; + } else { + expect(newRect.left, lessThan(rect.left)); + } + } + break; + case TextDirection.ltr: + expect(newRect.left, rect.left); + if (doneHorizontally) { + expect(newRect.right, rect.right); + } else { + if (newRect.right == rect.right) { + doneHorizontally = true; + } else { + expect(newRect.right, greaterThan(rect.right)); + } + } + break; + } + rect = newRect; + } while (tester.binding.hasScheduledFrame); + }); + } + + await testPositioningDown(tester, TextDirection.ltr, Alignment.topRight, TextDirection.rtl, new Rect.fromLTWH(792.0, 8.0, 0.0, 0.0)); + await testPositioningDown(tester, TextDirection.rtl, Alignment.topRight, TextDirection.rtl, new Rect.fromLTWH(792.0, 8.0, 0.0, 0.0)); + await testPositioningDown(tester, TextDirection.ltr, Alignment.topLeft, TextDirection.ltr, new Rect.fromLTWH(8.0, 8.0, 0.0, 0.0)); + await testPositioningDown(tester, TextDirection.rtl, Alignment.topLeft, TextDirection.ltr, new Rect.fromLTWH(8.0, 8.0, 0.0, 0.0)); + await testPositioningDown(tester, TextDirection.ltr, Alignment.topCenter, TextDirection.ltr, new Rect.fromLTWH(350.0, 8.0, 0.0, 0.0)); + await testPositioningDown(tester, TextDirection.rtl, Alignment.topCenter, TextDirection.rtl, new Rect.fromLTWH(450.0, 8.0, 0.0, 0.0)); + await testPositioningDown(tester, TextDirection.ltr, Alignment.centerRight, TextDirection.rtl, new Rect.fromLTWH(792.0, 250.0, 0.0, 0.0)); + await testPositioningDown(tester, TextDirection.rtl, Alignment.centerRight, TextDirection.rtl, new Rect.fromLTWH(792.0, 250.0, 0.0, 0.0)); + await testPositioningDown(tester, TextDirection.ltr, Alignment.centerLeft, TextDirection.ltr, new Rect.fromLTWH(8.0, 250.0, 0.0, 0.0)); + await testPositioningDown(tester, TextDirection.rtl, Alignment.centerLeft, TextDirection.ltr, new Rect.fromLTWH(8.0, 250.0, 0.0, 0.0)); + await testPositioningDown(tester, TextDirection.ltr, Alignment.center, TextDirection.ltr, new Rect.fromLTWH(350.0, 250.0, 0.0, 0.0)); + await testPositioningDown(tester, TextDirection.rtl, Alignment.center, TextDirection.rtl, new Rect.fromLTWH(450.0, 250.0, 0.0, 0.0)); + await testPositioningDownThenUp(tester, TextDirection.ltr, Alignment.bottomRight, TextDirection.rtl, new Rect.fromLTWH(792.0, 500.0, 0.0, 0.0)); + await testPositioningDownThenUp(tester, TextDirection.rtl, Alignment.bottomRight, TextDirection.rtl, new Rect.fromLTWH(792.0, 500.0, 0.0, 0.0)); + await testPositioningDownThenUp(tester, TextDirection.ltr, Alignment.bottomLeft, TextDirection.ltr, new Rect.fromLTWH(8.0, 500.0, 0.0, 0.0)); + await testPositioningDownThenUp(tester, TextDirection.rtl, Alignment.bottomLeft, TextDirection.ltr, new Rect.fromLTWH(8.0, 500.0, 0.0, 0.0)); + await testPositioningDownThenUp(tester, TextDirection.ltr, Alignment.bottomCenter, TextDirection.ltr, new Rect.fromLTWH(350.0, 500.0, 0.0, 0.0)); + await testPositioningDownThenUp(tester, TextDirection.rtl, Alignment.bottomCenter, TextDirection.rtl, new Rect.fromLTWH(450.0, 500.0, 0.0, 0.0)); + }); +} + +class TestApp extends StatefulWidget { + const TestApp({ this.textDirection, this.child }); + final TextDirection textDirection; + final Widget child; + @override + _TestAppState createState() => new _TestAppState(); +} + +class _TestAppState extends State { + @override + Widget build(BuildContext context) { + return new MediaQuery( + data: new MediaQueryData.fromWindow(window), + child: new Directionality( + textDirection: widget.textDirection, + child: new Navigator( + onGenerateRoute: (RouteSettings settings) { + assert(settings.name == '/'); + return new MaterialPageRoute( + settings: settings, + builder: (BuildContext context) => new Material( + child: widget.child, + ), + ); + }, + ), + ), + ); + } } diff --git a/packages/flutter/test/rendering/relative_rect_test.dart b/packages/flutter/test/rendering/relative_rect_test.dart index 359df008d1..ae0146ff7a 100644 --- a/packages/flutter/test/rendering/relative_rect_test.dart +++ b/packages/flutter/test/rendering/relative_rect_test.dart @@ -38,6 +38,11 @@ void main() { final Rect r2 = r1.toRect(new Rect.fromLTRB(10.0, 20.0, 90.0, 180.0)); expect(r2, new Rect.fromLTRB(10.0, 20.0, 50.0, 120.0)); }); + test('RelativeRect.toSize', () { + final RelativeRect r1 = const RelativeRect.fromLTRB(10.0, 20.0, 30.0, 40.0); + final Size r2 = r1.toSize(const Size(80.0, 160.0)); + expect(r2, const Size(40.0, 100.0)); + }); test('RelativeRect.lerp', () { final RelativeRect r1 = RelativeRect.fill; final RelativeRect r2 = const RelativeRect.fromLTRB(10.0, 20.0, 30.0, 40.0);