From 0c6e3416d8d4176f6e19980b48c736402410f58e Mon Sep 17 00:00:00 2001 From: Hixie Date: Tue, 15 Dec 2015 16:52:58 -0800 Subject: [PATCH] Canceling a dropdown menu selects null value Previously we didn't distinguish between canceling the menu (which popped the route with no return value, i.e. null) and explicitly selecting an item whose value is null (pop with value null). Both fired onChange(null). Now we box the return value so we can distinguish the two cases. I would have preferred to just disallow "null" as a value but it turns out people like being able to use "null" as a value for much the same reason we do, and it seems best for us to pay the complexity cost of boxing than having everyone else do it. :-) --- .../flutter/lib/src/material/dropdown.dart | 32 +++++++++++++++---- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/packages/flutter/lib/src/material/dropdown.dart b/packages/flutter/lib/src/material/dropdown.dart index b4984e04f5..1b6a829404 100644 --- a/packages/flutter/lib/src/material/dropdown.dart +++ b/packages/flutter/lib/src/material/dropdown.dart @@ -95,7 +95,10 @@ class _DropDownMenu extends StatusTransitionComponent { padding: _kMenuHorizontalPadding, child: route.items[itemIndex] ), - onTap: () => Navigator.pop(context, route.items[itemIndex].value) + onTap: () => Navigator.pop( + context, + new _DropDownRouteResult(route.items[itemIndex].value) + ) ) )); } @@ -143,9 +146,24 @@ class _DropDownMenu extends StatusTransitionComponent { } } -class _DropDownRoute extends PopupRoute { +// We box the return value so that the return value can be null. Otherwise, +// canceling the route (which returns null) would get confused with actually +// returning a real null value. +class _DropDownRouteResult { + const _DropDownRouteResult(this.result); + final T result; + bool operator ==(dynamic other) { + if (other is! _DropDownRouteResult) + return false; + final _DropDownRouteResult typedOther = other; + return result == typedOther.result; + } + int get hashCode => result.hashCode; +} + +class _DropDownRoute extends PopupRoute<_DropDownRouteResult> { _DropDownRoute({ - Completer completer, + Completer<_DropDownRouteResult> completer, this.items, this.selectedIndex, this.rect, @@ -246,7 +264,7 @@ class _DropDownButtonState extends State> { void _handleTap() { final RenderBox renderBox = indexedStackKey.currentContext.findRenderObject(); final Rect rect = renderBox.localToGlobal(Point.origin) & renderBox.size; - final Completer completer = new Completer(); + final Completer completer = new Completer<_DropDownRouteResult>(); Navigator.push(context, new _DropDownRoute( completer: completer, items: config.items, @@ -254,11 +272,11 @@ class _DropDownButtonState extends State> { rect: _kMenuHorizontalPadding.inflateRect(rect), elevation: config.elevation )); - completer.future.then((T newValue) { - if (!mounted) + completer.future.then((_DropDownRouteResult newValue) { + if (!mounted || newValue == null) return; if (config.onChanged != null) - config.onChanged(newValue); + config.onChanged(newValue.result); }); }