From dee75839087fb9cdfab342dd26587caa51b31eaf Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Mon, 14 Oct 2019 08:00:11 -0700 Subject: [PATCH] Only dismiss dropdowns if the orientation changes, not the size. (#42482) This changes the DropDownButton so that instead of dismissing itself when any metrics change occurs, it only dismisses itself when the orientation changes. This gets around the fact that we can't currently have a dropdown and a text field on the same page because the keyboard disappearing when the dropdown gets focus causes a metrics change, and the dropdown immediately disappears when activated. It still will cause the keyboard to jump up and down between controls, but that's a larger issue. At least now we can use the two together again. --- .../flutter/lib/src/material/dropdown.dart | 28 +++++++++++++------ packages/flutter/lib/src/widgets/binding.dart | 5 ++-- packages/flutter/lib/src/widgets/routes.dart | 4 +-- .../flutter/test/material/dropdown_test.dart | 24 ++++++++++++---- 4 files changed, 44 insertions(+), 17 deletions(-) diff --git a/packages/flutter/lib/src/material/dropdown.dart b/packages/flutter/lib/src/material/dropdown.dart index 6effd9aa69..2ccd63cd74 100644 --- a/packages/flutter/lib/src/material/dropdown.dart +++ b/packages/flutter/lib/src/material/dropdown.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'dart:math' as math; +import 'dart:ui' show window; import 'package:flutter/rendering.dart'; import 'package:flutter/widgets.dart'; @@ -906,12 +907,12 @@ class DropdownButton extends StatefulWidget { class _DropdownButtonState extends State> with WidgetsBindingObserver { int _selectedIndex; _DropdownRoute _dropdownRoute; + Orientation _lastOrientation; @override void initState() { super.initState(); _updateSelectedIndex(); - WidgetsBinding.instance.addObserver(this); } @override @@ -921,16 +922,10 @@ class _DropdownButtonState extends State> with WidgetsBindi super.dispose(); } - // Typically called because the device's orientation has changed. - // Defined by WidgetsBindingObserver - @override - void didChangeMetrics() { - _removeDropdownRoute(); - } - void _removeDropdownRoute() { _dropdownRoute?._dismiss(); _dropdownRoute = null; + _lastOrientation = null; } @override @@ -1036,10 +1031,27 @@ class _DropdownButtonState extends State> with WidgetsBindi bool get _enabled => widget.items != null && widget.items.isNotEmpty && widget.onChanged != null; + Orientation _getOrientation(BuildContext context) { + Orientation result = MediaQuery.of(context, nullOk: true)?.orientation; + if (result == null) { + // If there's no MediaQuery, then use the window aspect to determine + // orientation. + final Size size = window.physicalSize; + result = size.width > size.height ? Orientation.landscape : Orientation.portrait; + } + return result; + } + @override Widget build(BuildContext context) { assert(debugCheckHasMaterial(context)); assert(debugCheckHasMaterialLocalizations(context)); + final Orientation newOrientation = _getOrientation(context); + _lastOrientation ??= newOrientation; + if (newOrientation != _lastOrientation) { + _removeDropdownRoute(); + _lastOrientation = newOrientation; + } // The width of the button and the menu are defined by the widest // item and the width of the hint. diff --git a/packages/flutter/lib/src/widgets/binding.dart b/packages/flutter/lib/src/widgets/binding.dart index 94bc479e55..c9ecc96e42 100644 --- a/packages/flutter/lib/src/widgets/binding.dart +++ b/packages/flutter/lib/src/widgets/binding.dart @@ -124,9 +124,12 @@ abstract class WidgetsBindingObserver { /// } /// /// class _MetricsReactorState extends State with WidgetsBindingObserver { + /// Size _lastSize; + /// /// @override /// void initState() { /// super.initState(); + /// _lastSize = WidgetsBinding.instance.window.physicalSize; /// WidgetsBinding.instance.addObserver(this); /// } /// @@ -136,8 +139,6 @@ abstract class WidgetsBindingObserver { /// super.dispose(); /// } /// - /// Size _lastSize; - /// /// @override /// void didChangeMetrics() { /// setState(() { _lastSize = WidgetsBinding.instance.window.physicalSize; }); diff --git a/packages/flutter/lib/src/widgets/routes.dart b/packages/flutter/lib/src/widgets/routes.dart index e47c524c15..b33992d8b3 100644 --- a/packages/flutter/lib/src/widgets/routes.dart +++ b/packages/flutter/lib/src/widgets/routes.dart @@ -661,10 +661,10 @@ class _ModalScopeState extends State<_ModalScope> { // only necessary to rebuild the IgnorePointer widget and set // the focus node's ability to focus. AnimatedBuilder( - animation: widget.route.navigator.userGestureInProgressNotifier, + animation: widget.route.navigator?.userGestureInProgressNotifier ?? ValueNotifier(false), builder: (BuildContext context, Widget child) { final bool ignoreEvents = widget.route.animation?.status == AnimationStatus.reverse || - widget.route.navigator.userGestureInProgress; + (widget.route.navigator?.userGestureInProgress ?? false); focusScopeNode.canRequestFocus = !ignoreEvents; return IgnorePointer( ignoring: ignoreEvents, diff --git a/packages/flutter/test/material/dropdown_test.dart b/packages/flutter/test/material/dropdown_test.dart index a89fd0bd75..1fa94e75f0 100644 --- a/packages/flutter/test/material/dropdown_test.dart +++ b/packages/flutter/test/material/dropdown_test.dart @@ -43,9 +43,11 @@ Widget buildFrame({ List items = menuItems, Alignment alignment = Alignment.center, TextDirection textDirection = TextDirection.ltr, + Size mediaSize, }) { return TestApp( textDirection: textDirection, + mediaSize: mediaSize, child: Material( child: Align( alignment: alignment, @@ -131,9 +133,10 @@ Widget buildFormFrame({ } class TestApp extends StatefulWidget { - const TestApp({ this.textDirection, this.child }); + const TestApp({ this.textDirection, this.child, this.mediaSize }); final TextDirection textDirection; final Widget child; + final Size mediaSize; @override _TestAppState createState() => _TestAppState(); } @@ -148,7 +151,7 @@ class _TestAppState extends State { DefaultMaterialLocalizations.delegate, ], child: MediaQuery( - data: MediaQueryData.fromWindow(window), + data: MediaQueryData.fromWindow(window).copyWith(size: widget.mediaSize), child: Directionality( textDirection: widget.textDirection, child: Navigator( @@ -937,13 +940,24 @@ void main() { expect(menuRect.bottomRight, const Offset(800.0, 600.0)); }); - testWidgets('Dropdown menus are dismissed on screen orientation changes', (WidgetTester tester) async { - await tester.pumpWidget(buildFrame(onChanged: onChanged)); + testWidgets('Dropdown menus are dismissed on screen orientation changes, but not on keyboard hide', (WidgetTester tester) async { + await tester.pumpWidget(buildFrame(onChanged: onChanged, mediaSize: const Size(800, 600))); await tester.tap(find.byType(dropdownButtonType)); await tester.pumpAndSettle(); expect(find.byType(ListView), findsOneWidget); - window.onMetricsChanged(); + // Show a keyboard (simulate by shortening the height). + await tester.pumpWidget(buildFrame(onChanged: onChanged, mediaSize: const Size(800, 300))); + await tester.pump(); + expect(find.byType(ListView, skipOffstage: false), findsOneWidget); + + // Hide a keyboard again (simulate by increasing the height). + await tester.pumpWidget(buildFrame(onChanged: onChanged, mediaSize: const Size(800, 600))); + await tester.pump(); + expect(find.byType(ListView, skipOffstage: false), findsOneWidget); + + // Rotate the device (simulate by changing the aspect ratio). + await tester.pumpWidget(buildFrame(onChanged: onChanged, mediaSize: const Size(600, 800))); await tester.pump(); expect(find.byType(ListView, skipOffstage: false), findsNothing); });