firka/flutter
firka/flutter
5
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Popup menus RTL (#13110)

Browse Source 
This fixes the popup menu code to do a better job of expanding
smoothly regardless of which side of the screen it's on. It still
results in a bidirection growth when positioned at the bottom of the
screen, so maybe we'll need to animate menus differently, but that's
a problem for another patch.

Also, improve some docs and provide RelativeRect.toSize which I needed
at one point while building this patch (though it didn't survive all
the way to the end).
This commit is contained in:
Ian Hickson 2017-11-22 15:15:21 -08:00 committed by GitHub
parent 8c902ad479
commit 34ba6be93a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 360 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
packages/flutter/lib/src/material/about.dart
View File

@ -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,

5
packages/flutter/lib/src/material/bottom_sheet.dart
View File

@ -252,6 +252,11 @@ class _ModalBottomSheetRoute<T> extends PopupRoute<T> {
/// 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.
///

3
packages/flutter/lib/src/material/date_picker.dart
View File

@ -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]

4
packages/flutter/lib/src/material/dialog.dart
View File

@ -447,6 +447,10 @@ class _DialogRoute<T> extends PopupRoute<T> {
/// 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.
///

143
packages/flutter/lib/src/material/popup_menu.dart
View File

@ -447,7 +447,7 @@ class _PopupMenu<T> 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<T> 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<T> extends PopupRoute<T> {
Widget buildPage(BuildContext context, Animation<double> animation, Animation<double> secondaryAnimation) {
double selectedItemOffset;
if (initialValue != null) {
selectedItemOffset = 0.0;
double y = _kMenuVerticalPadding;
for (PopupMenuEntry<T> 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<T> extends PopupRoute<T> {
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<T> extends PopupRoute<T> {
/// 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<T> showMenu<T>({
RelativeRect position,
@required List<PopupMenuEntry<T>> 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<T> extends StatefulWidget {
class _PopupMenuButtonState<T> extends State<PopupMenuButton<T>> {
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<T>(
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<Null>((T newValue) {
if (!mounted || newValue == null)

6
packages/flutter/lib/src/material/time_picker.dart
View File

@ -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]

4
packages/flutter/lib/src/rendering/object.dart
View File

@ -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

14
packages/flutter/lib/src/rendering/stack.dart
View File

@ -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].

216
packages/flutter/test/material/popup_menu_test.dart
View File

@ -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<PopupMenuItem<int>> simplePopupMenuItemBuilder(BuildContext context) {
return <PopupMenuItem<int>>[
@ -132,4 +133,217 @@ void main() {
expect(find.byIcon(Icons.view_carousel), findsOneWidget);
});
});
testWidgets('PopupMenu positioning', (WidgetTester tester) async {
final Widget testButton = new PopupMenuButton<int>(
itemBuilder: (BuildContext context) {
return <PopupMenuItem<int>>[
const PopupMenuItem<int>(value: 1, child: const Text('AAA')),
const PopupMenuItem<int>(value: 2, child: const Text('BBB')),
const PopupMenuItem<int>(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<Null> 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<Null> 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<Null> 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<TestApp> {
@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<dynamic>(
settings: settings,
builder: (BuildContext context) => new Material(
child: widget.child,
),
);
},
),
),
);
}
}

5
packages/flutter/test/rendering/relative_rect_test.dart
View File

@ -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);