From 08bf1b6bceed550ead72209b9f7c90fc00701138 Mon Sep 17 00:00:00 2001 From: Hans Muller Date: Mon, 27 Jun 2016 10:28:20 -0700 Subject: [PATCH] Scrollable dropdown, dropdown underline cosmetics (#4766) --- .../lib/demo/buttons_demo.dart | 71 ++++++++++---- .../flutter/lib/src/material/drop_down.dart | 94 ++++++++++++------- .../flutter/test/material/drop_down_test.dart | 6 +- 3 files changed, 116 insertions(+), 55 deletions(-) diff --git a/examples/flutter_gallery/lib/demo/buttons_demo.dart b/examples/flutter_gallery/lib/demo/buttons_demo.dart index 1c7ba3b02e..95a5486139 100644 --- a/examples/flutter_gallery/lib/demo/buttons_demo.dart +++ b/examples/flutter_gallery/lib/demo/buttons_demo.dart @@ -135,26 +135,61 @@ class _ButtonsDemoState extends State { ); } - String dropdownValue = 'Free'; + // https://en.wikipedia.org/wiki/Free_Four + String dropdown1Value = 'Free'; + String dropdown2Value = 'Four'; Widget buildDropdownButton() { - return new Align( - alignment: new FractionalOffset(0.5, 0.4), - child: new DropDownButton( - value: dropdownValue, - onChanged: (String newValue) { - setState(() { - if (newValue != null) - dropdownValue = newValue; - }); - }, - items: ['One', 'Two', 'Free', 'Four'] - .map((String value) { - return new DropDownMenuItem( - value: value, - child: new Text(value)); - }) - .toList() + return new Padding( + padding: const EdgeInsets.all(24.0), + child: new Column( + mainAxisAlignment: MainAxisAlignment.start, + children: [ + new ListItem( + title: new Text('Scrollable dropdown:'), + trailing: new DropDownButton( + value: dropdown1Value, + onChanged: (String newValue) { + setState(() { + if (newValue != null) + dropdown1Value = newValue; + }); + }, + items: [ + 'One', 'Two', 'Free', 'Four', 'Can', 'I', 'Have', 'A', 'Little', + 'Bit', 'More', 'Five', 'Six', 'Seven', 'Eight', 'Nine', 'Ten' + ] + .map((String value) { + return new DropDownMenuItem( + value: value, + child: new Text(value)); + }) + .toList() + ) + ), + new SizedBox( + height: 24.0 + ), + new ListItem( + title: new Text('Simple dropdown:'), + trailing: new DropDownButton( + value: dropdown2Value, + onChanged: (String newValue) { + setState(() { + if (newValue != null) + dropdown2Value = newValue; + }); + }, + items: ['One', 'Two', 'Free', 'Four'] + .map((String value) { + return new DropDownMenuItem( + value: value, + child: new Text(value)); + }) + .toList() + ) + ) + ] ) ); } diff --git a/packages/flutter/lib/src/material/drop_down.dart b/packages/flutter/lib/src/material/drop_down.dart index 6ea5a91bb2..dc9d2c6838 100644 --- a/packages/flutter/lib/src/material/drop_down.dart +++ b/packages/flutter/lib/src/material/drop_down.dart @@ -8,10 +8,12 @@ import 'dart:math' as math; import 'package:flutter/widgets.dart'; import 'package:meta/meta.dart'; +import 'colors.dart'; import 'debug.dart'; import 'icon.dart'; import 'icons.dart'; import 'ink_well.dart'; +import 'scrollbar.dart'; import 'shadows.dart'; import 'theme.dart'; import 'material.dart'; @@ -19,10 +21,7 @@ import 'material.dart'; const Duration _kDropDownMenuDuration = const Duration(milliseconds: 300); const double _kMenuItemHeight = 48.0; const EdgeInsets _kMenuVerticalPadding = const EdgeInsets.symmetric(vertical: 8.0); -const EdgeInsets _kMenuHorizontalPadding = const EdgeInsets.symmetric(horizontal: 4.0); -const double _kBaselineOffsetFromBottom = 20.0; -const double _kBottomBorderHeight = 2.0; -const Border _kDropDownUnderline = const Border(bottom: const BorderSide(color: const Color(0xFFBDBDBD), width: _kBottomBorderHeight)); +const EdgeInsets _kMenuHorizontalPadding = const EdgeInsets.symmetric(horizontal: 16.0); class _DropDownMenuPainter extends CustomPainter { _DropDownMenuPainter({ @@ -121,7 +120,6 @@ class _DropDownMenuState extends State<_DropDownMenu> { // // When the menu is dismissed we just fade the entire thing out // in the first 0.25s. - final _DropDownRoute route = config.route; final double unit = 0.5 / (route.items.length + 1.5); final List children = []; @@ -161,10 +159,13 @@ class _DropDownMenuState extends State<_DropDownMenu> { child: new Material( type: MaterialType.transparency, textStyle: route.style, - child: new ScrollableList( - padding: _kMenuVerticalPadding, - itemExtent: _kMenuItemHeight, - children: children + child: new Scrollbar( + child: new ScrollableList( + scrollableKey: config.route.scrollableKey, + padding: _kMenuVerticalPadding, + itemExtent: _kMenuItemHeight, + children: children + ) ) ) ) @@ -172,11 +173,14 @@ class _DropDownMenuState extends State<_DropDownMenu> { } } -class _DropDownMenuRouteLayout extends SingleChildLayoutDelegate { - _DropDownMenuRouteLayout(this.buttonRect, this.selectedIndex); +class _DropDownMenuRouteLayout extends SingleChildLayoutDelegate { + _DropDownMenuRouteLayout({ this.route }); - final Rect buttonRect; - final int selectedIndex; + final _DropDownRoute route; + + Rect get buttonRect => route.buttonRect; + int get selectedIndex => route.selectedIndex; + GlobalKey get scrollableKey => route.scrollableKey; @override BoxConstraints getConstraintsForChild(BoxConstraints constraints) { @@ -185,7 +189,7 @@ class _DropDownMenuRouteLayout extends SingleChildLayoutDelegate { // with which to dismiss the menu. // -- https://www.google.com/design/spec/components/menus.html#menus-simple-menus final double maxHeight = math.max(0.0, constraints.maxHeight - 2 * _kMenuItemHeight); - final double width = buttonRect.width; + final double width = buttonRect.width + 8.0; return new BoxConstraints( minWidth: width, maxWidth: width, @@ -197,12 +201,13 @@ class _DropDownMenuRouteLayout extends SingleChildLayoutDelegate { @override Offset getPositionForChild(Size size, Size childSize) { final double buttonTop = buttonRect.top; - double top = buttonTop - selectedIndex * _kMenuItemHeight - _kMenuVerticalPadding.top; - double topPreferredLimit = _kMenuItemHeight; + final double selectedItemOffset = selectedIndex * _kMenuItemHeight + _kMenuVerticalPadding.top; + double top = buttonTop - selectedItemOffset; + final double topPreferredLimit = _kMenuItemHeight; if (top < topPreferredLimit) top = math.min(buttonTop, topPreferredLimit); double bottom = top + childSize.height; - double bottomPreferredLimit = size.height - _kMenuItemHeight; + final double bottomPreferredLimit = size.height - _kMenuItemHeight; if (bottom > bottomPreferredLimit) { bottom = math.max(buttonTop + _kMenuItemHeight, bottomPreferredLimit); top = bottom - childSize.height; @@ -218,14 +223,18 @@ class _DropDownMenuRouteLayout extends SingleChildLayoutDelegate { } return true; }); + + if (route.initialLayout) { + route.initialLayout = false; + final double scrollOffset = selectedItemOffset - (buttonTop - top); + scrollableKey.currentState.scrollTo(scrollOffset); + } + return new Offset(buttonRect.left, top); } @override - bool shouldRelayout(_DropDownMenuRouteLayout oldDelegate) { - return oldDelegate.buttonRect != buttonRect - || oldDelegate.selectedIndex != selectedIndex; - } + bool shouldRelayout(_DropDownMenuRouteLayout oldDelegate) => oldDelegate.route != route; } // We box the return value so that the return value can be null. Otherwise, @@ -260,10 +269,14 @@ class _DropDownRoute extends PopupRoute<_DropDownRouteResult> { assert(style != null); } + final GlobalKey scrollableKey = new GlobalKey(debugLabel: '_DropDownMenu'); final List> items; final Rect buttonRect; final int selectedIndex; final int elevation; + // The layout gets this route's scrollableKey so that it can scroll the + /// selected item into position, but only on the initial layout. + bool initialLayout = true; TextStyle get style => _style; TextStyle _style; @@ -288,7 +301,7 @@ class _DropDownRoute extends PopupRoute<_DropDownRouteResult> { @override Widget buildPage(BuildContext context, Animation animation, Animation forwardAnimation) { return new CustomSingleChildLayout( - delegate: new _DropDownMenuRouteLayout(buttonRect, selectedIndex), + delegate: new _DropDownMenuRouteLayout(route: this), child: new _DropDownMenu(route: this) ); } @@ -324,10 +337,8 @@ class DropDownMenuItem extends StatelessWidget { Widget build(BuildContext context) { return new Container( height: _kMenuItemHeight, - padding: const EdgeInsets.symmetric(horizontal: 8.0), - child: new Baseline( - baselineType: TextBaseline.alphabetic, - baseline: _kMenuItemHeight - _kBaselineOffsetFromBottom, + child: new Align( + alignment: FractionalOffset.centerLeft, child: child ) ); @@ -387,7 +398,7 @@ class DropDownButton extends StatefulWidget { @required this.onChanged, this.elevation: 8, this.style, - this.iconSize: 36.0 + this.iconSize: 24.0 }) : super(key: key) { assert(items != null); assert(items.where((DropDownMenuItem item) => item.value == value).length == 1); @@ -416,7 +427,7 @@ class DropDownButton extends StatefulWidget { /// The size to use for the drop-down button's down arrow icon button. /// - /// Defaults to 36.0. + /// Defaults to 24.0. final double iconSize; @override @@ -493,18 +504,37 @@ class _DropDownButtonState extends State> { alignment: FractionalOffset.centerLeft, children: config.items ), - new Icon(Icons.arrow_drop_down, size: config.iconSize) + new Icon(Icons.arrow_drop_down, + size: config.iconSize, + // These colors are not defined in the Material Design spec. + color: Theme.of(context).brightness == Brightness.light ? Colors.grey[700] : Colors.white70 + ) ] ) ); + if (!DropDownButtonHideUnderline.at(context)) { - result = new Container( - decoration: const BoxDecoration(border: _kDropDownUnderline), - child: result + result = new Stack( + children: [ + result, + new Positioned( + left: 0.0, + right: 0.0, + bottom: 8.0, + child: new Container( + height: 1.0, + decoration: const BoxDecoration( + border: const Border(bottom: const BorderSide(color: const Color(0xFFBDBDBD), width: 0.0)) + ) + ) + ) + ] ); } + return new GestureDetector( onTap: _handleTap, + behavior: HitTestBehavior.opaque, child: result ); } diff --git a/packages/flutter/test/material/drop_down_test.dart b/packages/flutter/test/material/drop_down_test.dart index f2d4173a5f..c24075de2b 100644 --- a/packages/flutter/test/material/drop_down_test.dart +++ b/packages/flutter/test/material/drop_down_test.dart @@ -49,11 +49,7 @@ void main() { await tester.tap(find.byConfig(button)); expect(value, 4); await tester.idle(); // this waits for the route's completer to complete, which calls handleChanged - - // Ideally this would be 4 because the menu would be overscrolled to the - // correct position, but currently we just reposition the menu so that it - // is visible on screen. - expect(value, 0); + expect(value, 4); // TODO(abarth): Remove these calls to pump once navigator cleans up its // pop transitions.