Correct the initial rendering of non-scrollable dropdown menus (#10255)

packages/flutter/lib/src/material/dropdown.dart

@@ -5,7 +5,6 @@
 import 'dart:math' as math;
 
 import 'package:flutter/foundation.dart';
-import 'package:flutter/scheduler.dart';
 import 'package:flutter/widgets.dart';
 
 import 'colors.dart';
@@ -191,13 +190,11 @@ class _DropdownMenuState<T> extends State<_DropdownMenu<T>> {
 }
 
 class _DropdownMenuRouteLayout<T> extends SingleChildLayoutDelegate {
-  _DropdownMenuRouteLayout({ this.route });
+  _DropdownMenuRouteLayout({ this.buttonRect, this.menuTop, this.menuHeight });
 
-  final _DropdownRoute<T> route;
+  final Rect buttonRect;
-
+  final double menuTop;
-  Rect get buttonRect => route.buttonRect;
+  final double menuHeight;
-  int get selectedIndex => route.selectedIndex;
-  ScrollController get scrollController => route.scrollController;
 
   @override
   BoxConstraints getConstraintsForChild(BoxConstraints constraints) {
@@ -217,45 +214,28 @@ class _DropdownMenuRouteLayout<T> extends SingleChildLayoutDelegate {
 
   @override
   Offset getPositionForChild(Size size, Size childSize) {
-    final double buttonTop = buttonRect.top;
-    final double selectedItemOffset = selectedIndex * _kMenuItemHeight + kMaterialListPadding.top;
-    double top = (buttonTop - selectedItemOffset) - (_kMenuItemHeight - buttonRect.height) / 2.0;
-    final double topPreferredLimit = _kMenuItemHeight;
-    if (top < topPreferredLimit)
-      top = math.min(buttonTop, topPreferredLimit);
-    double bottom = top + childSize.height;
-    final double bottomPreferredLimit = size.height - _kMenuItemHeight;
-    if (bottom > bottomPreferredLimit) {
-      bottom = math.max(buttonTop + _kMenuItemHeight, bottomPreferredLimit);
-      top = bottom - childSize.height;
-    }
     assert(() {
       final Rect container = Offset.zero & size;
       if (container.intersect(buttonRect) == buttonRect) {
         // If the button was entirely on-screen, then verify
         // that the menu is also on-screen.
         // If the button was a bit off-screen, then, oh well.
-        assert(top >= 0.0);
+        assert(menuTop >= 0.0);
-        assert(top + childSize.height <= size.height);
+        assert(menuTop + menuHeight <= size.height);
       }
       return true;
     });
 
-    if (route.initialLayout) {
+    final double width = buttonRect.width + 8.0;
-      route.initialLayout = false;
+    return new Offset(buttonRect.left.clamp(0.0, size.width - width), menuTop);
-      final double scrollOffset = selectedItemOffset - (buttonTop - top);
-      SchedulerBinding.instance.addPostFrameCallback((Duration timeStamp) {
-        // TODO(ianh): Compute and set this during layout instead of being
-        // lagged by one frame. https://github.com/flutter/flutter/issues/5751
-        scrollController.jumpTo(scrollOffset);
-      });
-    }
-
-    return new Offset(buttonRect.left, top);
   }
 
   @override
-  bool shouldRelayout(_DropdownMenuRouteLayout<T> oldDelegate) => oldDelegate.route != route;
+  bool shouldRelayout(_DropdownMenuRouteLayout<T> oldDelegate) {
+    return buttonRect != oldDelegate.buttonRect
+      || menuTop != oldDelegate.menuTop
+      || menuHeight != oldDelegate.menuHeight;
+  }
 }
 
 // We box the return value so that the return value can be null. Otherwise,
@@ -290,7 +270,6 @@ class _DropdownRoute<T> extends PopupRoute<_DropdownRouteResult<T>> {
     assert(style != null);
   }
 
-  final ScrollController scrollController = new ScrollController();
   final List<DropdownMenuItem<T>> items;
   final Rect buttonRect;
   final int selectedIndex;
@@ -298,9 +277,7 @@ class _DropdownRoute<T> extends PopupRoute<_DropdownRouteResult<T>> {
   final ThemeData theme;
   final TextStyle style;
 
-  // The layout gets this route's scrollController so that it can scroll the
+  ScrollController scrollController;
-  // selected item into position, but only on the initial layout.
-  bool initialLayout = true;
 
   @override
   Duration get transitionDuration => _kDropdownMenuDuration;
@@ -313,12 +290,41 @@ class _DropdownRoute<T> extends PopupRoute<_DropdownRouteResult<T>> {
 
   @override
   Widget buildPage(BuildContext context, Animation<double> animation, Animation<double> secondaryAnimation) {
+    final double screenHeight = MediaQuery.of(context).size.height;
+    final double maxMenuHeight = screenHeight - 2.0 * _kMenuItemHeight;
+    final double preferredMenuHeight = (items.length * _kMenuItemHeight) + kMaterialListPadding.vertical;
+    final double menuHeight = math.min(maxMenuHeight, preferredMenuHeight);
+
+    final double buttonTop = buttonRect.top;
+    final double selectedItemOffset = selectedIndex * _kMenuItemHeight + kMaterialListPadding.top;
+    double menuTop = (buttonTop - selectedItemOffset) - (_kMenuItemHeight - buttonRect.height) / 2.0;
+    final double topPreferredLimit = _kMenuItemHeight;
+    if (menuTop < topPreferredLimit)
+      menuTop = math.min(buttonTop, topPreferredLimit);
+    double bottom = menuTop + menuHeight;
+    final double bottomPreferredLimit = screenHeight - _kMenuItemHeight;
+    if (bottom > bottomPreferredLimit) {
+      bottom = math.max(buttonTop + _kMenuItemHeight, bottomPreferredLimit);
+      menuTop = bottom - menuHeight;
+    }
+
+    if (scrollController == null) {
+      double scrollOffset = 0.0;
+      if (preferredMenuHeight > maxMenuHeight)
+        scrollOffset = selectedItemOffset - (buttonTop - menuTop);
+      scrollController = new ScrollController(initialScrollOffset: scrollOffset);
+    }
+
     Widget menu = new _DropdownMenu<T>(route: this);
     if (theme != null)
       menu = new Theme(data: theme, child: menu);
 
     return new CustomSingleChildLayout(
-      delegate: new _DropdownMenuRouteLayout<T>(route: this),
+      delegate: new _DropdownMenuRouteLayout<T>(
+        buttonRect: buttonRect,
+        menuTop: menuTop,
+        menuHeight: menuHeight,
+      ),
       child: menu,
     );
   }
@@ -466,13 +472,30 @@ class DropdownButton<T> extends StatefulWidget {
   _DropdownButtonState<T> createState() => new _DropdownButtonState<T>();
 }
 
-class _DropdownButtonState<T> extends State<DropdownButton<T>> {
+class _DropdownButtonState<T> extends State<DropdownButton<T>> with WidgetsBindingObserver {
   int _selectedIndex;
+  _DropdownRoute<T> _dropdownRoute;
 
   @override
   void initState() {
     super.initState();
     _updateSelectedIndex();
+    WidgetsBinding.instance.addObserver(this);
+  }
+
+  @override
+  void dispose() {
+    WidgetsBinding.instance.removeObserver(this);
+    //TODO(hansmuller) if _dropDownRoute != null Navigator.remove(context, _dropdownRoute)
+    super.dispose();
+  }
+
+  // Typically called because the device's orientation has changed.
+  // Defined by WidgetsBindingObserver
+  @override
+  void didChangeMetrics() {
+    //TODO(hansmuller) if _dropDownRoute != null Navigator.remove(context, _dropdownRoute)
+    _dropdownRoute = null;
   }
 
   @override
@@ -498,14 +521,19 @@ class _DropdownButtonState<T> extends State<DropdownButton<T>> {
   void _handleTap() {
     final RenderBox itemBox = context.findRenderObject();
     final Rect itemRect = itemBox.localToGlobal(Offset.zero) & itemBox.size;
-    Navigator.push(context, new _DropdownRoute<T>(
+
+    assert(_dropdownRoute == null);
+    _dropdownRoute = new _DropdownRoute<T>(
       items: widget.items,
       buttonRect: _kMenuHorizontalPadding.inflateRect(itemRect),
       selectedIndex: _selectedIndex ?? 0,
       elevation: widget.elevation,
       theme: Theme.of(context, shadowThemeOnly: true),
       style: _textStyle,
-    )).then<Null>((_DropdownRouteResult<T> newValue) {
+    );
+
+    Navigator.push(context, _dropdownRoute).then<Null>((_DropdownRouteResult<T> newValue) {
+      _dropdownRoute = null;
       if (!mounted || newValue == null)
         return null;
       if (widget.onChanged != null)

packages/flutter/lib/src/widgets/scroll_position.dart

@@ -466,9 +466,6 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
   /// If this method changes the scroll position, a sequence of start/update/end
   /// scroll notifications will be dispatched. No overscroll notifications can
   /// be generated by this method.
-  ///
-  /// If settle is true then, immediately after the jump, a ballistic activity
-  /// is started, in case the value was out of range.
   void jumpTo(double value);
 
   /// Deprecated. Use [jumpTo] or a custom [ScrollPosition] instead.

packages/flutter/test/material/dropdown_test.dart

@@ -7,7 +7,12 @@ import 'dart:math' as math;
 import 'package:flutter_test/flutter_test.dart';
 import 'package:flutter/material.dart';
 
-final List<String> menuItems = <String>['one', 'two', 'three', 'four'];
+const List<String> menuItems = const <String>['one', 'two', 'three', 'four'];
+
+final Type dropdownButtonType = new DropdownButton<String>(
+  onChanged: (_){ },
+  items: const <DropdownMenuItem<String>>[]
+).runtimeType;
 
 Widget buildFrame({
     Key buttonKey,
@@ -15,17 +20,20 @@ Widget buildFrame({
     ValueChanged<String> onChanged,
     bool isDense: false,
     Widget hint,
+    List<String> items: menuItems,
+    FractionalOffset alignment: FractionalOffset.center,
   }) {
   return new MaterialApp(
     home: new Material(
-      child: new Center(
+      child: new Align(
+        alignment: alignment,
         child: new DropdownButton<String>(
           key: buttonKey,
           value: value,
           hint: hint,
           onChanged: onChanged,
           isDense: isDense,
-          items: menuItems.map((String item) {
+          items: items.map((String item) {
             return new DropdownMenuItem<String>(
               key: new ValueKey<String>(item),
               value: item,
@@ -57,7 +65,6 @@ bool sameGeometry(RenderBox box1, RenderBox box2) {
   return true;
 }
 
-
 void main() {
   testWidgets('Dropdown button control test', (WidgetTester tester) async {
     String value = 'one';
@@ -348,4 +355,112 @@ void main() {
     expect(buttonBox.size, equals(buttonBoxHintValue.size));
   });
 
+  testWidgets('Dropdown menus must fit within the screen', (WidgetTester tester) async {
+
+    // The dropdown menu isn't readaily accessible. To find it we're assuming that it
+    // contains a ListView and that it's an instance of _DropdownMenu.
+    Rect getMenuRect() {
+      Rect menuRect;
+      tester.element(find.byType(ListView)).visitAncestorElements((Element element) {
+        if (element.toString().startsWith("_DropdownMenu")) {
+          final RenderBox box = element.findRenderObject();
+          assert(box != null);
+          menuRect =  box.localToGlobal(Offset.zero) & box.size;
+          return false;
+        }
+        return true;
+      });
+      assert(menuRect != null);
+      return menuRect;
+    }
+
+    // In all of the tests that follow we're assuming that the dropdown menu
+    // is horizontally aligned with the center of the dropdown button and padded
+    // on the top, left, and right.
+    const EdgeInsets buttonPadding = const EdgeInsets.only(top: 8.0, left: 16.0, right: 24.0);
+
+    Rect getExpandedButtonRect() {
+      final RenderBox box = tester.renderObject<RenderBox>(find.byType(dropdownButtonType));
+      final Rect buttonRect = box.localToGlobal(Offset.zero) & box.size;
+      return buttonPadding.inflateRect(buttonRect);
+    }
+
+    Rect buttonRect;
+    Rect menuRect;
+
+    Future<Null> popUpAndDown(Widget frame) async {
+      await tester.pumpWidget(frame);
+      await tester.tap(find.byType(dropdownButtonType));
+      await tester.pumpAndSettle();
+      menuRect = getMenuRect();
+      buttonRect = getExpandedButtonRect();
+      await tester.tap(find.byType(dropdownButtonType));
+    }
+
+    // Dropdown button is along the top of the app. The top of the menu is
+    // aligned with the top of the expanded button and shifted horizontally
+    // so that it fits within the frame.
+
+    await popUpAndDown(
+      buildFrame(alignment: FractionalOffset.topLeft, value: menuItems.last)
+    );
+    expect(menuRect.topLeft, Offset.zero);
+    expect(menuRect.topRight, new Offset(menuRect.width, 0.0));
+
+    await popUpAndDown(
+      buildFrame(alignment: FractionalOffset.topCenter, value: menuItems.last)
+    );
+    expect(menuRect.topLeft, new Offset(buttonRect.left, 0.0));
+    expect(menuRect.topRight, new Offset(buttonRect.right, 0.0));
+
+    await popUpAndDown(
+      buildFrame(alignment: FractionalOffset.topRight, value: menuItems.last)
+    );
+    expect(menuRect.topLeft, new Offset(800.0 - menuRect.width, 0.0));
+    expect(menuRect.topRight, const Offset(800.0, 0.0));
+
+    // Dropdown button is along the middle of the app. The top of the menu is
+    // aligned with the top of the expanded button (because the 1st item
+    // is selected) and shifted horizontally so that it fits within the frame.
+
+    await popUpAndDown(
+      buildFrame(alignment: FractionalOffset.centerLeft, value: menuItems.first)
+    );
+    expect(menuRect.topLeft, new Offset(0.0, buttonRect.top));
+    expect(menuRect.topRight, new Offset(menuRect.width, buttonRect.top));
+
+    await popUpAndDown(
+      buildFrame(alignment: FractionalOffset.center, value: menuItems.first)
+    );
+    expect(menuRect.topLeft, buttonRect.topLeft);
+    expect(menuRect.topRight, buttonRect.topRight);
+
+    await popUpAndDown(
+      buildFrame(alignment: FractionalOffset.centerRight, value: menuItems.first)
+    );
+    expect(menuRect.topLeft, new Offset(800.0 - menuRect.width, buttonRect.top));
+    expect(menuRect.topRight, new Offset(800.0, buttonRect.top));
+
+    // Dropdown button is along the bottom of the app. The bottom of the menu is
+    // aligned with the bottom of the expanded button and shifted horizontally
+    // so that it fits within the frame.
+
+    await popUpAndDown(
+      buildFrame(alignment: FractionalOffset.bottomLeft, value: menuItems.first)
+    );
+    expect(menuRect.bottomLeft, const Offset(0.0, 600.0));
+    expect(menuRect.bottomRight, new Offset(menuRect.width, 600.0));
+
+    await popUpAndDown(
+      buildFrame(alignment: FractionalOffset.bottomCenter, value: menuItems.first)
+    );
+    expect(menuRect.bottomLeft, new Offset(buttonRect.left, 600.0));
+    expect(menuRect.bottomRight, new Offset(buttonRect.right, 600.0));
+
+    await popUpAndDown(
+      buildFrame(alignment: FractionalOffset.bottomRight, value: menuItems.first)
+    );
+    expect(menuRect.bottomLeft, new Offset(800.0 - menuRect.width, 600.0));
+    expect(menuRect.bottomRight, const Offset(800.0, 600.0));
+  });
 }