Unverified Commit 8b15b537 authored by Ian Hickson's avatar Ian Hickson Committed by GitHub

DropdownButton RTL (#13040)

This fixes DropdownButtons to align their popups correctly in RTL as well.

Also while I was there I fixed the issue with text scale factor in the gallery.
parent f0e88198
...@@ -72,6 +72,17 @@ class GalleryAppState extends State<GalleryApp> { ...@@ -72,6 +72,17 @@ class GalleryAppState extends State<GalleryApp> {
super.dispose(); super.dispose();
} }
Widget _applyScaleFactor(Widget child) {
return new Builder(
builder: (BuildContext context) => new MediaQuery(
data: MediaQuery.of(context).copyWith(
textScaleFactor: _textScaleFactor,
),
child: child,
),
);
}
@override @override
Widget build(BuildContext context) { Widget build(BuildContext context) {
Widget home = new GalleryHome( Widget home = new GalleryHome(
...@@ -145,26 +156,11 @@ class GalleryAppState extends State<GalleryApp> { ...@@ -145,26 +156,11 @@ class GalleryAppState extends State<GalleryApp> {
// https://docs.flutter.io/flutter/widgets/Navigator-class.html // https://docs.flutter.io/flutter/widgets/Navigator-class.html
kAllGalleryItems, kAllGalleryItems,
key: (GalleryItem item) => item.routeName, key: (GalleryItem item) => item.routeName,
value: (GalleryItem item) => value: (GalleryItem item) {
(BuildContext context) { return (BuildContext context) => _applyScaleFactor(item.buildRoute(context));
if (_textScaleFactor != null) { },
return new MediaQuery(
data: new MediaQueryData(textScaleFactor: _textScaleFactor),
child: item.buildRoute(context),
);
} else {
return item.buildRoute(context);
}
}
); );
if (_textScaleFactor != null) {
home = new MediaQuery(
data: new MediaQueryData(textScaleFactor: _textScaleFactor),
child: home,
);
}
return new MaterialApp( return new MaterialApp(
title: 'Flutter Gallery', title: 'Flutter Gallery',
color: Colors.grey, color: Colors.grey,
...@@ -173,7 +169,7 @@ class GalleryAppState extends State<GalleryApp> { ...@@ -173,7 +169,7 @@ class GalleryAppState extends State<GalleryApp> {
checkerboardRasterCacheImages: _checkerboardRasterCacheImages, checkerboardRasterCacheImages: _checkerboardRasterCacheImages,
checkerboardOffscreenLayers: _checkerboardOffscreenLayers, checkerboardOffscreenLayers: _checkerboardOffscreenLayers,
routes: _kRoutes, routes: _kRoutes,
home: home, home: _applyScaleFactor(home),
); );
} }
} }
...@@ -190,11 +190,17 @@ class _DropdownMenuState<T> extends State<_DropdownMenu<T>> { ...@@ -190,11 +190,17 @@ class _DropdownMenuState<T> extends State<_DropdownMenu<T>> {
} }
class _DropdownMenuRouteLayout<T> extends SingleChildLayoutDelegate { class _DropdownMenuRouteLayout<T> extends SingleChildLayoutDelegate {
_DropdownMenuRouteLayout({ this.buttonRect, this.menuTop, this.menuHeight }); _DropdownMenuRouteLayout({
@required this.buttonRect,
@required this.menuTop,
@required this.menuHeight,
@required this.textDirection,
});
final Rect buttonRect; final Rect buttonRect;
final double menuTop; final double menuTop;
final double menuHeight; final double menuHeight;
final TextDirection textDirection;
@override @override
BoxConstraints getConstraintsForChild(BoxConstraints constraints) { BoxConstraints getConstraintsForChild(BoxConstraints constraints) {
...@@ -227,14 +233,25 @@ class _DropdownMenuRouteLayout<T> extends SingleChildLayoutDelegate { ...@@ -227,14 +233,25 @@ class _DropdownMenuRouteLayout<T> extends SingleChildLayoutDelegate {
} }
return true; return true;
}()); }());
return new Offset(buttonRect.left.clamp(0.0, size.width - childSize.width), menuTop); assert(textDirection != null);
double left;
switch (textDirection) {
case TextDirection.rtl:
left = buttonRect.right.clamp(0.0, size.width - childSize.width) - childSize.width;
break;
case TextDirection.ltr:
left = buttonRect.left.clamp(0.0, size.width - childSize.width);
break;
}
return new Offset(left, menuTop);
} }
@override @override
bool shouldRelayout(_DropdownMenuRouteLayout<T> oldDelegate) { bool shouldRelayout(_DropdownMenuRouteLayout<T> oldDelegate) {
return buttonRect != oldDelegate.buttonRect return buttonRect != oldDelegate.buttonRect
|| menuTop != oldDelegate.menuTop || menuTop != oldDelegate.menuTop
|| menuHeight != oldDelegate.menuHeight; || menuHeight != oldDelegate.menuHeight
|| textDirection != oldDelegate.textDirection;
} }
} }
...@@ -288,6 +305,7 @@ class _DropdownRoute<T> extends PopupRoute<_DropdownRouteResult<T>> { ...@@ -288,6 +305,7 @@ class _DropdownRoute<T> extends PopupRoute<_DropdownRouteResult<T>> {
@override @override
Widget buildPage(BuildContext context, Animation<double> animation, Animation<double> secondaryAnimation) { Widget buildPage(BuildContext context, Animation<double> animation, Animation<double> secondaryAnimation) {
assert(debugCheckHasDirectionality(context));
final double screenHeight = MediaQuery.of(context).size.height; final double screenHeight = MediaQuery.of(context).size.height;
final double maxMenuHeight = screenHeight - 2.0 * _kMenuItemHeight; final double maxMenuHeight = screenHeight - 2.0 * _kMenuItemHeight;
final double preferredMenuHeight = (items.length * _kMenuItemHeight) + kMaterialListPadding.vertical; final double preferredMenuHeight = (items.length * _kMenuItemHeight) + kMaterialListPadding.vertical;
...@@ -322,6 +340,7 @@ class _DropdownRoute<T> extends PopupRoute<_DropdownRouteResult<T>> { ...@@ -322,6 +340,7 @@ class _DropdownRoute<T> extends PopupRoute<_DropdownRouteResult<T>> {
buttonRect: buttonRect, buttonRect: buttonRect,
menuTop: menuTop, menuTop: menuTop,
menuHeight: menuHeight, menuHeight: menuHeight,
textDirection: Directionality.of(context),
), ),
child: menu, child: menu,
); );
...@@ -361,7 +380,7 @@ class DropdownMenuItem<T> extends StatelessWidget { ...@@ -361,7 +380,7 @@ class DropdownMenuItem<T> extends StatelessWidget {
Widget build(BuildContext context) { Widget build(BuildContext context) {
return new Container( return new Container(
height: _kMenuItemHeight, height: _kMenuItemHeight,
alignment: Alignment.centerLeft, alignment: AlignmentDirectional.centerStart,
child: child, child: child,
); );
} }
...@@ -589,7 +608,7 @@ class _DropdownButtonState<T> extends State<DropdownButton<T>> with WidgetsBindi ...@@ -589,7 +608,7 @@ class _DropdownButtonState<T> extends State<DropdownButton<T>> with WidgetsBindi
// the hint or nothing at all. // the hint or nothing at all.
new IndexedStack( new IndexedStack(
index: _selectedIndex ?? hintIndex, index: _selectedIndex ?? hintIndex,
alignment: Alignment.centerLeft, alignment: AlignmentDirectional.centerStart,
children: items, children: items,
), ),
new Icon(Icons.arrow_drop_down, new Icon(Icons.arrow_drop_down,
......
...@@ -7,6 +7,7 @@ import 'dart:ui' show window; ...@@ -7,6 +7,7 @@ import 'dart:ui' show window;
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
import 'package:flutter/material.dart'; import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
import '../widgets/semantics_tester.dart'; import '../widgets/semantics_tester.dart';
...@@ -18,16 +19,18 @@ final Type dropdownButtonType = new DropdownButton<String>( ...@@ -18,16 +19,18 @@ final Type dropdownButtonType = new DropdownButton<String>(
).runtimeType; ).runtimeType;
Widget buildFrame({ Widget buildFrame({
Key buttonKey, Key buttonKey,
String value: 'two', String value: 'two',
ValueChanged<String> onChanged, ValueChanged<String> onChanged,
bool isDense: false, bool isDense: false,
Widget hint, Widget hint,
List<String> items: menuItems, List<String> items: menuItems,
Alignment alignment: Alignment.center, Alignment alignment: Alignment.center,
}) { TextDirection textDirection: TextDirection.ltr,
return new MaterialApp( }) {
home: new Material( return new TestApp(
textDirection: textDirection,
child: new Material(
child: new Align( child: new Align(
alignment: alignment, alignment: alignment,
child: new DropdownButton<String>( child: new DropdownButton<String>(
...@@ -49,6 +52,35 @@ Widget buildFrame({ ...@@ -49,6 +52,35 @@ Widget buildFrame({
); );
} }
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) => widget.child,
);
},
),
),
);
}
}
// When the dropdown's menu is popped up, a RenderParagraph for the selected // When the dropdown's menu is popped up, a RenderParagraph for the selected
// menu's text item will appear both in the dropdown button and in the menu. // menu's text item will appear both in the dropdown button and in the menu.
// The RenderParagraphs should be aligned, i.e. they should have the same // The RenderParagraphs should be aligned, i.e. they should have the same
...@@ -251,39 +283,52 @@ void main() { ...@@ -251,39 +283,52 @@ void main() {
await tester.pump(const Duration(seconds: 1)); // finish the menu animation await tester.pump(const Duration(seconds: 1)); // finish the menu animation
}); });
testWidgets('Dropdown button aligns selected menu item', (WidgetTester tester) async { for (TextDirection textDirection in TextDirection.values) {
final Key buttonKey = new UniqueKey(); testWidgets('Dropdown button aligns selected menu item ($textDirection)', (WidgetTester tester) async {
final String value = 'two'; final Key buttonKey = new UniqueKey();
final String value = 'two';
Widget build() => buildFrame(buttonKey: buttonKey, value: value);
Widget build() => buildFrame(buttonKey: buttonKey, value: value, textDirection: textDirection);
await tester.pumpWidget(build());
final RenderBox buttonBox = tester.renderObject(find.byKey(buttonKey)); await tester.pumpWidget(build());
assert(buttonBox.attached); final RenderBox buttonBox = tester.renderObject(find.byKey(buttonKey));
final Offset buttonOriginBeforeTap = buttonBox.localToGlobal(Offset.zero); assert(buttonBox.attached);
final Offset buttonOriginBeforeTap = buttonBox.localToGlobal(Offset.zero);
await tester.tap(find.text('two'));
await tester.pump(); await tester.tap(find.text('two'));
await tester.pump(const Duration(seconds: 1)); // finish the menu animation await tester.pump();
await tester.pump(const Duration(seconds: 1)); // finish the menu animation
// Tapping the dropdown button should not cause it to move.
expect(buttonBox.localToGlobal(Offset.zero), equals(buttonOriginBeforeTap)); // Tapping the dropdown button should not cause it to move.
expect(buttonBox.localToGlobal(Offset.zero), equals(buttonOriginBeforeTap));
// The selected dropdown item is both in menu we just popped up, and in
// the IndexedStack contained by the dropdown button. Both of them should
// have the same origin and height as the dropdown button.
final List<RenderObject> itemBoxes = tester.renderObjectList(find.byKey(const ValueKey<String>('two'))).toList();
expect(itemBoxes.length, equals(2));
for (RenderBox itemBox in itemBoxes) {
assert(itemBox.attached);
assert(textDirection != null);
switch (textDirection) {
case TextDirection.rtl:
expect(buttonBox.localToGlobal(buttonBox.size.bottomRight(Offset.zero)),
equals(itemBox.localToGlobal(itemBox.size.bottomRight(Offset.zero))));
break;
case TextDirection.ltr:
expect(buttonBox.localToGlobal(Offset.zero), equals(itemBox.localToGlobal(Offset.zero)));
break;
}
expect(buttonBox.size.height, equals(itemBox.size.height));
}
// The selected dropdown item is both in menu we just popped up, and in // The two RenderParagraph objects, for the 'two' items' Text children,
// the IndexedStack contained by the dropdown button. Both of them should // should have the same size and location.
// have the same origin and height as the dropdown button. checkSelectedItemTextGeometry(tester, 'two');
final List<RenderObject> itemBoxes = tester.renderObjectList(find.byKey(const ValueKey<String>('two'))).toList();
expect(itemBoxes.length, equals(2));
for (RenderBox itemBox in itemBoxes) {
assert(itemBox.attached);
expect(buttonBox.localToGlobal(Offset.zero), equals(itemBox.localToGlobal(Offset.zero)));
expect(buttonBox.size.height, equals(itemBox.size.height));
}
// The two RenderParagraph objects, for the 'two' items' Text children, await tester.pumpWidget(new Container()); // reset test
// should have the same size and location. });
checkSelectedItemTextGeometry(tester, 'two'); }
});
testWidgets('Dropdown button with isDense:true aligns selected menu item', (WidgetTester tester) async { testWidgets('Dropdown button with isDense:true aligns selected menu item', (WidgetTester tester) async {
final Key buttonKey = new UniqueKey(); final Key buttonKey = new UniqueKey();
...@@ -406,7 +451,7 @@ void main() { ...@@ -406,7 +451,7 @@ void main() {
if (element.toString().startsWith('_DropdownMenu')) { if (element.toString().startsWith('_DropdownMenu')) {
final RenderBox box = element.findRenderObject(); final RenderBox box = element.findRenderObject();
assert(box != null); assert(box != null);
menuRect = box.localToGlobal(Offset.zero) & box.size; menuRect = box.localToGlobal(Offset.zero) & box.size;
return false; return false;
} }
return true; return true;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment