Ensure that DropDownMenus are always onscreen (#3742)

This patch sizes the menu such that it is always on screen, but doesn't scroll
the menu to ensure that the currently selected item is always visible and on
top of the button. That will need to wait for a later patch.

Also, teach CustomPaint how to repaint animations more efficiently.

Fixes #3720

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

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 import 'dart:async';
+import 'dart:math' as math;
 
 import 'package:flutter/widgets.dart';
 
@@ -24,40 +25,52 @@ const double _kBottomBorderHeight = 2.0;
 const Border _kDropDownUnderline = const Border(bottom: const BorderSide(color: const Color(0xFFBDBDBD), width: _kBottomBorderHeight));
 
 class _DropDownMenuPainter extends CustomPainter {
-  const _DropDownMenuPainter({
-    this.color,
-    this.elevation,
-    this.menuTop,
-    this.menuBottom,
-    this.renderBox
-  });
+  _DropDownMenuPainter({
+    Color color,
+    int elevation,
+    this.buttonRect,
+    this.selectedIndex,
+    Animation<double> resize
+  }) : color = color,
+       elevation = elevation,
+       resize = resize,
+       _painter = new BoxDecoration(
+         backgroundColor: color,
+         borderRadius: 2.0,
+         boxShadow: kElevationToShadow[elevation]
+       ).createBoxPainter(),
+       super(repaint: resize);
 
   final Color color;
   final int elevation;
-  final double menuTop;
-  final double menuBottom;
-  final RenderBox renderBox;
+  final Rect buttonRect;
+  final int selectedIndex;
+  final Animation<double> resize;
+
+  final BoxPainter _painter;
 
   @override
   void paint(Canvas canvas, Size size) {
-    final BoxPainter painter = new BoxDecoration(
-      backgroundColor: color,
-      borderRadius: 2.0,
-      boxShadow: kElevationToShadow[elevation]
-    ).createBoxPainter();
-
-    double top = renderBox.globalToLocal(new Point(0.0, menuTop)).y;
-    double bottom = renderBox.globalToLocal(new Point(0.0, menuBottom)).y;
-    painter.paint(canvas, new Rect.fromLTRB(0.0, top, size.width, bottom));
+    final Tween<double> top = new Tween<double>(
+      begin: (selectedIndex * buttonRect.height + _kMenuVerticalPadding.top).clamp(0.0, size.height - buttonRect.height),
+      end: 0.0
+    );
+
+    final Tween<double> bottom = new Tween<double>(
+      begin: (top.begin + buttonRect.height).clamp(buttonRect.height, size.height),
+      end: size.height
+    );
+
+    _painter.paint(canvas, new Rect.fromLTRB(0.0, top.evaluate(resize), size.width, bottom.evaluate(resize)));
   }
 
   @override
   bool shouldRepaint(_DropDownMenuPainter oldPainter) {
     return oldPainter.color != color
         || oldPainter.elevation != elevation
-        || oldPainter.menuTop != menuTop
-        || oldPainter.menuBottom != menuBottom
-        || oldPainter.renderBox != renderBox;
+        || oldPainter.buttonRect != buttonRect
+        || oldPainter.selectedIndex != selectedIndex
+        || oldPainter.resize != resize;
   }
 }
 
@@ -84,12 +97,13 @@ class _DropDownMenu<T> extends StatusTransitionWidget {
     final List<Widget> children = <Widget>[];
     for (int itemIndex = 0; itemIndex < route.items.length; ++itemIndex) {
       CurvedAnimation opacity;
+      Interval reverseCurve = const Interval(0.75, 1.0);
       if (itemIndex == route.selectedIndex) {
-        opacity = new CurvedAnimation(parent: route.animation, curve: const Step(0.0), reverseCurve: const Interval(0.75, 1.0));
+        opacity = new CurvedAnimation(parent: route.animation, curve: const Step(0.0), reverseCurve: reverseCurve);
       } else {
         final double start = (0.5 + (itemIndex + 1) * unit).clamp(0.0, 1.0);
         final double end = (start + 1.5 * unit).clamp(0.0, 1.0);
-        opacity = new CurvedAnimation(parent: route.animation, curve: new Interval(start, end), reverseCurve: const Interval(0.75, 1.0));
+        opacity = new CurvedAnimation(parent: route.animation, curve: new Interval(start, end), reverseCurve: reverseCurve);
       }
       children.add(new FadeTransition(
         opacity: opacity,
@@ -106,51 +120,31 @@ class _DropDownMenu<T> extends StatusTransitionWidget {
       ));
     }
 
-    final CurvedAnimation opacity = new CurvedAnimation(
-      parent: route.animation,
-      curve: const Interval(0.0, 0.25),
-      reverseCurve: const Interval(0.75, 1.0)
-    );
-
-    final CurvedAnimation resize = new CurvedAnimation(
-      parent: route.animation,
-      curve: const Interval(0.25, 0.5),
-      reverseCurve: const Step(0.0)
-    );
-
-    final Tween<double> menuTop = new Tween<double>(
-      begin: route.buttonRect.top,
-      end: route.buttonRect.top - route.selectedIndex * route.buttonRect.height - _kMenuVerticalPadding.top
-    );
-    final Tween<double> menuBottom = new Tween<double>(
-      begin: route.buttonRect.bottom,
-      end: menuTop.end + route.items.length * route.buttonRect.height + _kMenuVerticalPadding.vertical
-    );
-
-    Widget child = new Material(
-      type: MaterialType.transparency,
-      child: new Block(
-        padding: _kMenuVerticalPadding,
-        children: children
-      )
-    );
     return new FadeTransition(
-      opacity: opacity,
-      child: new AnimatedBuilder(
-        animation: resize,
-        builder: (BuildContext context, Widget child) {
-          return new CustomPaint(
-            painter: new _DropDownMenuPainter(
-              color: Theme.of(context).canvasColor,
-              elevation: route.elevation,
-              menuTop: menuTop.evaluate(resize),
-              menuBottom: menuBottom.evaluate(resize),
-              renderBox: context.findRenderObject()
-            ),
-            child: child
-          );
-        },
-        child: child
+      opacity: new CurvedAnimation(
+        parent: route.animation,
+        curve: const Interval(0.0, 0.25),
+        reverseCurve: const Interval(0.75, 1.0)
+      ),
+      child: new CustomPaint(
+        painter: new _DropDownMenuPainter(
+          color: Theme.of(context).canvasColor,
+          elevation: route.elevation,
+          buttonRect: route.buttonRect,
+          selectedIndex: route.selectedIndex,
+          resize: new CurvedAnimation(
+            parent: route.animation,
+            curve: const Interval(0.25, 0.5),
+            reverseCurve: const Step(0.0)
+          )
+        ),
+        child: new Material(
+          type: MaterialType.transparency,
+          child: new Block(
+            padding: _kMenuVerticalPadding,
+            children: children
+          )
+        )
       )
     );
   }
@@ -164,20 +158,34 @@ class _DropDownMenuRouteLayout extends SingleChildLayoutDelegate {
 
   @override
   BoxConstraints getConstraintsForChild(BoxConstraints constraints) {
+    // The maximum height of a simple menu should be one or more rows less than
+    // the view height. This ensures a tappable area outside of the simple menu
+    // 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 * buttonRect.height);
     return new BoxConstraints(
       minWidth: buttonRect.width,
       maxWidth: buttonRect.width,
       minHeight: 0.0,
-      maxHeight: constraints.maxHeight
+      maxHeight: maxHeight
     );
   }
 
   @override
   Offset getPositionForChild(Size size, Size childSize) {
-    return new Offset(
-      buttonRect.left,
-      buttonRect.top - selectedIndex * buttonRect.height - _kMenuVerticalPadding.top
-    );
+    double top = buttonRect.top - selectedIndex * buttonRect.height - _kMenuVerticalPadding.top;
+    double topPreferredLimit = buttonRect.height;
+    if (top < topPreferredLimit)
+      top = math.min(buttonRect.top, topPreferredLimit);
+    double bottom = top + childSize.height;
+    double bottomPreferredLimit = size.height - buttonRect.height;
+    if (bottom > bottomPreferredLimit) {
+      bottom = math.max(buttonRect.bottom, bottomPreferredLimit);
+      top = bottom - childSize.height;
+    }
+    assert(top >= 0.0);
+    assert(top + childSize.height <= size.height);
+    return new Offset(buttonRect.left, top);
   }
 
   @override
@@ -256,6 +264,8 @@ class DropDownMenuItem<T> extends StatelessWidget {
   }
 
   /// The widget below this widget in the tree.
+  ///
+  /// Typically a [Text] widget.
   final Widget child;
 
   /// The value to return if the user selects this menu item.
@@ -330,6 +340,7 @@ class DropDownButton<T> extends StatefulWidget {
     this.onChanged,
     this.elevation: 8
   }) : super(key: key) {
+    assert(items != null);
     assert(items.where((DropDownMenuItem<T> item) => item.value == value).length == 1);
   }
 

packages/flutter/lib/src/rendering/proxy_box.dart

@@ -4,6 +4,7 @@
 
 import 'dart:ui' as ui show ImageFilter;
 
+import 'package:flutter/animation.dart';
 import 'package:flutter/gestures.dart';
 import 'package:vector_math/vector_math_64.dart';
 
@@ -1382,12 +1383,20 @@ class RenderFractionalTranslation extends RenderProxyBox {
 /// is provided, to check if the new instance actually represents different
 /// information.
 ///
+/// The most efficient way to trigger a repaint is to supply a repaint argument
+/// to the constructor of the [CustomPainter]. The custom object will listen to
+/// this animation and repaint whenever the animation ticks, avoiding both the
+/// build and layout phases of the pipeline.
+///
 /// The [hitTest] method is invoked when the user interacts with the underlying
 /// render object, to determine if the user hit the object or missed it.
 abstract class CustomPainter {
-  /// Abstract const constructor. This constructor enables subclasses to provide
-  /// const constructors so that they can be used in const expressions.
-  const CustomPainter();
+  /// Creates a custom painter.
+  ///
+  /// The painter will repaint whenever the [repaint] animation ticks.
+  const CustomPainter({ Animation<dynamic> repaint }) : _repaint = repaint;
+
+  final Animation<dynamic> _repaint;
 
   /// Called whenever the object needs to paint. The given [Canvas] has its
   /// coordinate space configured such that the origin is at the top left of the
@@ -1500,7 +1509,7 @@ class RenderCustomPaint extends RenderProxyBox {
       return;
     CustomPainter oldPainter = _painter;
     _painter = newPainter;
-    _checkForRepaint(_painter, oldPainter);
+    _didUpdatePainter(_painter, oldPainter);
   }
 
   /// The foreground custom paint delegate.
@@ -1525,10 +1534,10 @@ class RenderCustomPaint extends RenderProxyBox {
       return;
     CustomPainter oldPainter = _foregroundPainter;
     _foregroundPainter = newPainter;
-    _checkForRepaint(_foregroundPainter, oldPainter);
+    _didUpdatePainter(_foregroundPainter, oldPainter);
   }
 
-  void _checkForRepaint(CustomPainter newPainter, CustomPainter oldPainter) {
+  void _didUpdatePainter(CustomPainter newPainter, CustomPainter oldPainter) {
     if (newPainter == null) {
       assert(oldPainter != null); // We should be called only for changes.
       markNeedsPaint();
@@ -1537,6 +1546,24 @@ class RenderCustomPaint extends RenderProxyBox {
         newPainter.shouldRepaint(oldPainter)) {
       markNeedsPaint();
     }
+    if (attached) {
+      oldPainter._repaint?.removeListener(markNeedsPaint);
+      newPainter._repaint?.addListener(markNeedsPaint);
+    }
+  }
+
+  @override
+  void attach(PipelineOwner owner) {
+    super.attach(owner);
+    _painter?._repaint?.addListener(markNeedsPaint);
+    _foregroundPainter?._repaint?.addListener(markNeedsPaint);
+  }
+
+  @override
+  void detach() {
+    _painter?._repaint?.removeListener(markNeedsPaint);
+    _foregroundPainter?._repaint?.removeListener(markNeedsPaint);
+    super.detach();
   }
 
   @override

packages/flutter/test/material/drop_down_test.dart

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:flutter_test/flutter_test.dart';
+import 'package:flutter/material.dart';
+import 'package:test/test.dart';
+
+void main() {
+  testWidgets('Drop down screen edges', (WidgetTester tester) {
+    int value = 4;
+    List<DropDownMenuItem<int>> items = <DropDownMenuItem<int>>[];
+    for (int i = 0; i < 20; ++i)
+      items.add(new DropDownMenuItem<int>(value: i, child: new Text('$i')));
+
+    void handleChanged(int newValue) {
+      value = newValue;
+    }
+
+    DropDownButton<int> button = new DropDownButton<int>(
+      value: value,
+      onChanged: handleChanged,
+      items: items
+    );
+
+    tester.pumpWidget(
+      new MaterialApp(
+        home: new Material(
+          child: new Align(
+            alignment: FractionalOffset.topCenter,
+            child:button
+          )
+        )
+      )
+    );
+
+    tester.tap(find.text('4'));
+    tester.pump();
+    tester.pump(const Duration(seconds: 1)); // finish the menu animation
+
+    tester.tap(find.byConfig(button));
+
+    // 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);
+
+    // TODO(abarth): Remove these calls to pump once navigator cleans up its
+    // pop transitions.
+    tester.pump();
+    tester.pump(const Duration(seconds: 1)); // finish the menu animation
+
+  });
+}