Unverified Commit dee75839 authored by Greg Spencer's avatar Greg Spencer Committed by GitHub

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.
parent 9423a012
......@@ -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<T> extends StatefulWidget {
class _DropdownButtonState<T> extends State<DropdownButton<T>> with WidgetsBindingObserver {
int _selectedIndex;
_DropdownRoute<T> _dropdownRoute;
Orientation _lastOrientation;
@override
void initState() {
super.initState();
_updateSelectedIndex();
WidgetsBinding.instance.addObserver(this);
}
@override
......@@ -921,16 +922,10 @@ class _DropdownButtonState<T> extends State<DropdownButton<T>> 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<T> extends State<DropdownButton<T>> 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.
......
......@@ -124,9 +124,12 @@ abstract class WidgetsBindingObserver {
/// }
///
/// class _MetricsReactorState extends State<MetricsReactor> 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; });
......
......@@ -661,10 +661,10 @@ class _ModalScopeState<T> extends State<_ModalScope<T>> {
// 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<bool>(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,
......
......@@ -43,9 +43,11 @@ Widget buildFrame({
List<String> 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<TestApp> {
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);
});
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment