Commit 2592f8f4 authored by Adam Barth's avatar Adam Barth

DropDownMenu should use ScrollableList (#3745)

Previously it used Block, which is less efficient for large numbers of items.
Also move the top margin out of the menu item to fix the baseline alignment of
the text.

Fixes #1615
parent b357aa32
...@@ -28,7 +28,6 @@ class _DropDownMenuPainter extends CustomPainter { ...@@ -28,7 +28,6 @@ class _DropDownMenuPainter extends CustomPainter {
_DropDownMenuPainter({ _DropDownMenuPainter({
Color color, Color color,
int elevation, int elevation,
this.buttonRect,
this.selectedIndex, this.selectedIndex,
Animation<double> resize Animation<double> resize
}) : color = color, }) : color = color,
...@@ -43,7 +42,6 @@ class _DropDownMenuPainter extends CustomPainter { ...@@ -43,7 +42,6 @@ class _DropDownMenuPainter extends CustomPainter {
final Color color; final Color color;
final int elevation; final int elevation;
final Rect buttonRect;
final int selectedIndex; final int selectedIndex;
final Animation<double> resize; final Animation<double> resize;
...@@ -52,12 +50,12 @@ class _DropDownMenuPainter extends CustomPainter { ...@@ -52,12 +50,12 @@ class _DropDownMenuPainter extends CustomPainter {
@override @override
void paint(Canvas canvas, Size size) { void paint(Canvas canvas, Size size) {
final Tween<double> top = new Tween<double>( final Tween<double> top = new Tween<double>(
begin: (selectedIndex * buttonRect.height + _kMenuVerticalPadding.top).clamp(0.0, size.height - buttonRect.height), begin: (selectedIndex * _kMenuItemHeight + _kMenuVerticalPadding.top).clamp(0.0, size.height - _kMenuItemHeight),
end: 0.0 end: 0.0
); );
final Tween<double> bottom = new Tween<double>( final Tween<double> bottom = new Tween<double>(
begin: (top.begin + buttonRect.height).clamp(buttonRect.height, size.height), begin: (top.begin + _kMenuItemHeight).clamp(_kMenuItemHeight, size.height),
end: size.height end: size.height
); );
...@@ -68,7 +66,6 @@ class _DropDownMenuPainter extends CustomPainter { ...@@ -68,7 +66,6 @@ class _DropDownMenuPainter extends CustomPainter {
bool shouldRepaint(_DropDownMenuPainter oldPainter) { bool shouldRepaint(_DropDownMenuPainter oldPainter) {
return oldPainter.color != color return oldPainter.color != color
|| oldPainter.elevation != elevation || oldPainter.elevation != elevation
|| oldPainter.buttonRect != buttonRect
|| oldPainter.selectedIndex != selectedIndex || oldPainter.selectedIndex != selectedIndex
|| oldPainter.resize != resize; || oldPainter.resize != resize;
} }
...@@ -130,7 +127,6 @@ class _DropDownMenu<T> extends StatusTransitionWidget { ...@@ -130,7 +127,6 @@ class _DropDownMenu<T> extends StatusTransitionWidget {
painter: new _DropDownMenuPainter( painter: new _DropDownMenuPainter(
color: Theme.of(context).canvasColor, color: Theme.of(context).canvasColor,
elevation: route.elevation, elevation: route.elevation,
buttonRect: route.buttonRect,
selectedIndex: route.selectedIndex, selectedIndex: route.selectedIndex,
resize: new CurvedAnimation( resize: new CurvedAnimation(
parent: route.animation, parent: route.animation,
...@@ -140,8 +136,9 @@ class _DropDownMenu<T> extends StatusTransitionWidget { ...@@ -140,8 +136,9 @@ class _DropDownMenu<T> extends StatusTransitionWidget {
), ),
child: new Material( child: new Material(
type: MaterialType.transparency, type: MaterialType.transparency,
child: new Block( child: new ScrollableList(
padding: _kMenuVerticalPadding, padding: _kMenuVerticalPadding,
itemExtent: _kMenuItemHeight,
children: children children: children
) )
) )
...@@ -162,10 +159,11 @@ class _DropDownMenuRouteLayout extends SingleChildLayoutDelegate { ...@@ -162,10 +159,11 @@ class _DropDownMenuRouteLayout extends SingleChildLayoutDelegate {
// the view height. This ensures a tappable area outside of the simple menu // the view height. This ensures a tappable area outside of the simple menu
// with which to dismiss the menu. // with which to dismiss the menu.
// -- https://www.google.com/design/spec/components/menus.html#menus-simple-menus // -- https://www.google.com/design/spec/components/menus.html#menus-simple-menus
final double maxHeight = math.max(0.0, constraints.maxHeight - 2 * buttonRect.height); final double maxHeight = math.max(0.0, constraints.maxHeight - 2 * _kMenuItemHeight);
final double width = buttonRect.width;
return new BoxConstraints( return new BoxConstraints(
minWidth: buttonRect.width, minWidth: width,
maxWidth: buttonRect.width, maxWidth: width,
minHeight: 0.0, minHeight: 0.0,
maxHeight: maxHeight maxHeight: maxHeight
); );
...@@ -173,14 +171,15 @@ class _DropDownMenuRouteLayout extends SingleChildLayoutDelegate { ...@@ -173,14 +171,15 @@ class _DropDownMenuRouteLayout extends SingleChildLayoutDelegate {
@override @override
Offset getPositionForChild(Size size, Size childSize) { Offset getPositionForChild(Size size, Size childSize) {
double top = buttonRect.top - selectedIndex * buttonRect.height - _kMenuVerticalPadding.top; final double buttonTop = buttonRect.top;
double topPreferredLimit = buttonRect.height; double top = buttonTop - selectedIndex * _kMenuItemHeight - _kMenuVerticalPadding.top;
double topPreferredLimit = _kMenuItemHeight;
if (top < topPreferredLimit) if (top < topPreferredLimit)
top = math.min(buttonRect.top, topPreferredLimit); top = math.min(buttonTop, topPreferredLimit);
double bottom = top + childSize.height; double bottom = top + childSize.height;
double bottomPreferredLimit = size.height - buttonRect.height; double bottomPreferredLimit = size.height - _kMenuItemHeight;
if (bottom > bottomPreferredLimit) { if (bottom > bottomPreferredLimit) {
bottom = math.max(buttonRect.bottom, bottomPreferredLimit); bottom = math.max(buttonTop + _kMenuItemHeight, bottomPreferredLimit);
top = bottom - childSize.height; top = bottom - childSize.height;
} }
assert(top >= 0.0); assert(top >= 0.0);
...@@ -277,7 +276,7 @@ class DropDownMenuItem<T> extends StatelessWidget { ...@@ -277,7 +276,7 @@ class DropDownMenuItem<T> extends StatelessWidget {
Widget build(BuildContext context) { Widget build(BuildContext context) {
return new Container( return new Container(
height: _kMenuItemHeight, height: _kMenuItemHeight,
padding: const EdgeInsets.only(left: 8.0, right: 8.0, top: _kTopMargin), padding: const EdgeInsets.symmetric(horizontal: 8.0),
child: new DefaultTextStyle( child: new DefaultTextStyle(
style: Theme.of(context).textTheme.subhead, style: Theme.of(context).textTheme.subhead,
child: new Baseline( child: new Baseline(
...@@ -363,7 +362,7 @@ class DropDownButton<T> extends StatefulWidget { ...@@ -363,7 +362,7 @@ class DropDownButton<T> extends StatefulWidget {
} }
class _DropDownButtonState<T> extends State<DropDownButton<T>> { class _DropDownButtonState<T> extends State<DropDownButton<T>> {
final GlobalKey indexedStackKey = new GlobalKey(debugLabel: 'DropDownButton.IndexedStack'); final GlobalKey _itemKey = new GlobalKey(debugLabel: 'DropDownButton item key');
@override @override
void initState() { void initState() {
...@@ -390,13 +389,13 @@ class _DropDownButtonState<T> extends State<DropDownButton<T>> { ...@@ -390,13 +389,13 @@ class _DropDownButtonState<T> extends State<DropDownButton<T>> {
} }
void _handleTap() { void _handleTap() {
final RenderBox renderBox = indexedStackKey.currentContext.findRenderObject(); final RenderBox itemBox = _itemKey.currentContext.findRenderObject();
final Rect buttonRect = renderBox.localToGlobal(Point.origin) & renderBox.size; final Rect itemRect = itemBox.localToGlobal(Point.origin) & itemBox.size;
final Completer<_DropDownRouteResult<T>> completer = new Completer<_DropDownRouteResult<T>>(); final Completer<_DropDownRouteResult<T>> completer = new Completer<_DropDownRouteResult<T>>();
Navigator.push(context, new _DropDownRoute<T>( Navigator.push(context, new _DropDownRoute<T>(
completer: completer, completer: completer,
items: config.items, items: config.items,
buttonRect: _kMenuHorizontalPadding.inflateRect(buttonRect), buttonRect: _kMenuHorizontalPadding.inflateRect(itemRect),
selectedIndex: _selectedIndex, selectedIndex: _selectedIndex,
elevation: config.elevation elevation: config.elevation
)); ));
...@@ -412,27 +411,27 @@ class _DropDownButtonState<T> extends State<DropDownButton<T>> { ...@@ -412,27 +411,27 @@ class _DropDownButtonState<T> extends State<DropDownButton<T>> {
Widget build(BuildContext context) { Widget build(BuildContext context) {
assert(debugCheckHasMaterial(context)); assert(debugCheckHasMaterial(context));
Widget result = new Row( Widget result = new Row(
mainAxisAlignment: MainAxisAlignment.collapse,
children: <Widget>[ children: <Widget>[
// We use an IndexedStack to make sure we have enough width to show any
// possible item as the selected item without changing size.
new IndexedStack( new IndexedStack(
children: config.items, children: config.items,
key: indexedStackKey, key: _itemKey,
index: _selectedIndex, index: _selectedIndex,
alignment: FractionalOffset.topCenter alignment: FractionalOffset.topCenter
), ),
new Container( new Icon(icon: Icons.arrow_drop_down, size: 36.0)
child: new Icon(icon: Icons.arrow_drop_down, size: 36.0), ]
padding: const EdgeInsets.only(top: _kTopMargin)
)
],
mainAxisAlignment: MainAxisAlignment.collapse
); );
if (DropDownButtonHideUnderline.at(context)) { if (DropDownButtonHideUnderline.at(context)) {
result = new Padding( result = new Padding(
padding: const EdgeInsets.only(bottom: _kBottomBorderHeight), padding: const EdgeInsets.only(top: _kTopMargin, bottom: _kBottomBorderHeight),
child: result child: result
); );
} else { } else {
result = new Container( result = new Container(
padding: const EdgeInsets.only(top: _kTopMargin),
decoration: const BoxDecoration(border: _kDropDownUnderline), decoration: const BoxDecoration(border: _kDropDownUnderline),
child: result child: result
); );
......
...@@ -38,6 +38,14 @@ void main() { ...@@ -38,6 +38,14 @@ void main() {
tester.pump(); tester.pump();
tester.pump(const Duration(seconds: 1)); // finish the menu animation tester.pump(const Duration(seconds: 1)); // finish the menu animation
// We should have two copies of item 5, one in the menu and one in the
// button itself.
expect(find.text('5').evaluate().length, 2);
// We should only have one copy of item 19, which is in the button itself.
// The copy in the menu shouldn't be in the tree because it's off-screen.
expect(find.text('19').evaluate().length, 1);
tester.tap(find.byConfig(button)); tester.tap(find.byConfig(button));
// Ideally this would be 4 because the menu would be overscrolled to the // Ideally this would be 4 because the menu would be overscrolled to the
......
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