Unverified Commit 19778f9e authored by Greg Spencer's avatar Greg Spencer Committed by GitHub

Make selected item get focus when dropdown is opened (#43722)

As it stands, dropdowns currently do not focus the item that is selected, so if you select something on a dropdown, and then close it, and re-open it, then the new item is not auto-focused. This PR changes that so that selected value is focused by default when the dropdown is re-opened.
parent 4cc8ce06
......@@ -100,15 +100,109 @@ class _DropdownScrollBehavior extends ScrollBehavior {
ScrollPhysics getScrollPhysics(BuildContext context) => const ClampingScrollPhysics();
}
// The widget that is the button wrapping the menu items.
class _DropdownMenuItemButton<T> extends StatefulWidget {
const _DropdownMenuItemButton({
Key key,
@required this.padding,
@required this.route,
@required this.buttonRect,
@required this.constraints,
@required this.itemIndex,
}) : super(key: key);
final _DropdownRoute<T> route;
final EdgeInsets padding;
final Rect buttonRect;
final BoxConstraints constraints;
final int itemIndex;
@override
_DropdownMenuItemButtonState<T> createState() => _DropdownMenuItemButtonState<T>();
}
class _DropdownMenuItemButtonState<T> extends State<_DropdownMenuItemButton<T>> {
void _handleFocusChange(bool focused) {
bool inTraditionalMode;
switch (FocusManager.instance.highlightMode) {
case FocusHighlightMode.touch:
inTraditionalMode = false;
break;
case FocusHighlightMode.traditional:
inTraditionalMode = true;
break;
}
if (focused && inTraditionalMode) {
final _MenuLimits menuLimits = widget.route.getMenuLimits(
widget.buttonRect, widget.constraints.maxHeight, widget.itemIndex);
widget.route.scrollController.animateTo(
menuLimits.scrollOffset,
curve: Curves.easeInOut,
duration: const Duration(milliseconds: 100),
);
}
}
void _handleOnTap() {
Navigator.pop(
context,
_DropdownRouteResult<T>(widget.route.items[widget.itemIndex].item.value),
);
}
static final Map<LogicalKeySet, Intent> _webShortcuts =<LogicalKeySet, Intent>{
LogicalKeySet(LogicalKeyboardKey.enter): const Intent(SelectAction.key),
};
@override
Widget build(BuildContext context) {
CurvedAnimation opacity;
final double unit = 0.5 / (widget.route.items.length + 1.5);
if (widget.itemIndex == widget.route.selectedIndex) {
opacity = CurvedAnimation(parent: widget.route.animation, curve: const Threshold(0.0));
} else {
final double start = (0.5 + (widget.itemIndex + 1) * unit).clamp(0.0, 1.0);
final double end = (start + 1.5 * unit).clamp(0.0, 1.0);
opacity = CurvedAnimation(parent: widget.route.animation, curve: Interval(start, end));
}
Widget child = FadeTransition(
opacity: opacity,
child: InkWell(
autofocus: widget.itemIndex == widget.route.selectedIndex,
child: Container(
padding: widget.padding,
child: widget.route.items[widget.itemIndex],
),
onTap: _handleOnTap,
onFocusChange: _handleFocusChange,
),
);
if (kIsWeb) {
// On the web, enter doesn't select things, *except* in a <select>
// element, which is what a dropdown emulates.
child = Shortcuts(
shortcuts: _webShortcuts,
child: child,
);
}
return child;
}
}
class _DropdownMenu<T> extends StatefulWidget {
const _DropdownMenu({
Key key,
this.padding,
this.route,
this.buttonRect,
this.constraints,
}) : super(key: key);
final _DropdownRoute<T> route;
final EdgeInsets padding;
final Rect buttonRect;
final BoxConstraints constraints;
@override
_DropdownMenuState<T> createState() => _DropdownMenuState<T>();
......@@ -118,12 +212,6 @@ class _DropdownMenuState<T> extends State<_DropdownMenu<T>> {
CurvedAnimation _fadeOpacity;
CurvedAnimation _resize;
// On the web, enter doesn't select things, *except* in a <select>
// element, which is what a dropdown emulates.
static final Map<LogicalKeySet, Intent> _webShortcuts =<LogicalKeySet, Intent>{
LogicalKeySet(LogicalKeyboardKey.enter): const Intent(SelectAction.key),
};
@override
void initState() {
super.initState();
......@@ -156,40 +244,16 @@ class _DropdownMenuState<T> extends State<_DropdownMenu<T>> {
assert(debugCheckHasMaterialLocalizations(context));
final MaterialLocalizations localizations = MaterialLocalizations.of(context);
final _DropdownRoute<T> route = widget.route;
final double unit = 0.5 / (route.items.length + 1.5);
final List<Widget> children = <Widget>[];
for (int itemIndex = 0; itemIndex < route.items.length; ++itemIndex) {
CurvedAnimation opacity;
if (itemIndex == route.selectedIndex) {
opacity = CurvedAnimation(parent: route.animation, curve: const Threshold(0.0));
} 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 = CurvedAnimation(parent: route.animation, curve: Interval(start, end));
}
Widget child = FadeTransition(
opacity: opacity,
child: InkWell(
child: Container(
padding: widget.padding,
child: route.items[itemIndex],
),
onTap: () => Navigator.pop(
context,
_DropdownRouteResult<T>(route.items[itemIndex].item.value),
),
final List<Widget> children = <Widget>[
for (int itemIndex = 0; itemIndex < route.items.length; ++itemIndex)
_DropdownMenuItemButton<T>(
route: widget.route,
padding: widget.padding,
buttonRect: widget.buttonRect,
constraints: widget.constraints,
itemIndex: itemIndex,
),
);
if (kIsWeb) {
// On the web, enter doesn't select things, *except* in a <select>
// element, which is what a dropdown emulates.
child = Shortcuts(
shortcuts: _webShortcuts,
child: child,
);
}
children.add(child);
}
];
return FadeTransition(
opacity: _fadeOpacity,
......@@ -201,7 +265,7 @@ class _DropdownMenuState<T> extends State<_DropdownMenu<T>> {
resize: _resize,
// This offset is passed as a callback, not a value, because it must
// be retrieved at paint time (after layout), not at build time.
getSelectedItemOffset: route.getSelectedItemOffset,
getSelectedItemOffset: () => route.getItemOffset(route.selectedIndex),
),
child: Semantics(
scopesRoute: true,
......@@ -260,7 +324,7 @@ class _DropdownMenuRouteLayout<T> extends SingleChildLayoutDelegate {
@override
Offset getPositionForChild(Size size, Size childSize) {
final _MenuLimits menuLimits = route.getMenuLimits(buttonRect, size.height);
final _MenuLimits menuLimits = route.getMenuLimits(buttonRect, size.height, route.selectedIndex);
assert(() {
final Rect container = Offset.zero & size;
......@@ -382,12 +446,12 @@ class _DropdownRoute<T> extends PopupRoute<_DropdownRouteResult<T>> {
navigator?.removeRoute(this);
}
double getSelectedItemOffset() {
double getItemOffset(int index) {
double offset = kMaterialListPadding.top;
if (items.isNotEmpty && selectedIndex > 0) {
if (items.isNotEmpty && index > 0) {
assert(items.length == itemHeights?.length);
offset += itemHeights
.sublist(0, selectedIndex)
.sublist(0, index)
.reduce((double total, double height) => total + height);
}
return offset;
......@@ -397,11 +461,11 @@ class _DropdownRoute<T> extends PopupRoute<_DropdownRouteResult<T>> {
// for the ListView that contains the menu items. The vertical center of the
// selected item is aligned with the button's vertical center, as far as
// that's possible given availableHeight.
_MenuLimits getMenuLimits(Rect buttonRect, double availableHeight) {
_MenuLimits getMenuLimits(Rect buttonRect, double availableHeight, int index) {
final double maxMenuHeight = availableHeight - 2.0 * _kMenuItemHeight;
final double buttonTop = buttonRect.top;
final double buttonBottom = math.min(buttonRect.bottom, availableHeight);
final double selectedItemOffset = getSelectedItemOffset();
final double selectedItemOffset = getItemOffset(index);
// If the button is placed on the bottom or top of the screen, its top or
// bottom may be less than [_kMenuItemHeight] from the edge of the screen.
......@@ -481,7 +545,7 @@ class _DropdownRoutePage<T> extends StatelessWidget {
// Otherwise the initialScrollOffset is just a rough approximation based on
// treating the items as if their heights were all equal to kMinInteractveDimension.
if (route.scrollController == null) {
final _MenuLimits menuLimits = route.getMenuLimits(buttonRect, constraints.maxHeight);
final _MenuLimits menuLimits = route.getMenuLimits(buttonRect, constraints.maxHeight, selectedIndex);
route.scrollController = ScrollController(initialScrollOffset: menuLimits.scrollOffset);
}
......@@ -489,6 +553,8 @@ class _DropdownRoutePage<T> extends StatelessWidget {
Widget menu = _DropdownMenu<T>(
route: route,
padding: padding.resolve(textDirection),
buttonRect: buttonRect,
constraints: constraints,
);
if (theme != null)
......@@ -520,7 +586,7 @@ class _DropdownRoutePage<T> extends StatelessWidget {
// each menu item. These sizes are used to compute the offset of the selected
// item so that _DropdownRoutePage can align the vertical center of the
// selected item lines up with the vertical center of the dropdown button,
// as closely as posible.
// as closely as possible.
class _MenuItem<T> extends SingleChildRenderObjectWidget {
const _MenuItem({
Key key,
......@@ -978,6 +1044,7 @@ class _DropdownButtonState<T> extends State<DropdownButton<T>> with WidgetsBindi
FocusNode get focusNode => widget.focusNode ?? _internalNode;
bool _hasPrimaryFocus = false;
Map<LocalKey, ActionFactory> _actionMap;
FocusHighlightMode _focusHighlightMode;
// Only used if needed to create _internalNode.
FocusNode _createFocusNode() {
......@@ -996,12 +1063,16 @@ class _DropdownButtonState<T> extends State<DropdownButton<T>> with WidgetsBindi
if (!kIsWeb) ActivateAction.key: _createAction,
};
focusNode.addListener(_handleFocusChanged);
final FocusManager focusManager = WidgetsBinding.instance.focusManager;
_focusHighlightMode = focusManager.highlightMode;
focusManager.addHighlightModeListener(_handleFocusHighlightModeChange);
}
@override
void dispose() {
WidgetsBinding.instance.removeObserver(this);
_removeDropdownRoute();
WidgetsBinding.instance.focusManager.removeHighlightModeListener(_handleFocusHighlightModeChange);
focusNode.removeListener(_handleFocusChanged);
_internalNode?.dispose();
super.dispose();
......@@ -1021,6 +1092,15 @@ class _DropdownButtonState<T> extends State<DropdownButton<T>> with WidgetsBindi
}
}
void _handleFocusHighlightModeChange(FocusHighlightMode mode) {
if (!mounted) {
return;
}
setState(() {
_focusHighlightMode = mode;
});
}
@override
void didUpdateWidget(DropdownButton<T> oldWidget) {
super.didUpdateWidget(oldWidget);
......@@ -1152,6 +1232,16 @@ class _DropdownButtonState<T> extends State<DropdownButton<T>> with WidgetsBindi
return result;
}
bool get _showHighlight {
switch (_focusHighlightMode) {
case FocusHighlightMode.touch:
return false;
case FocusHighlightMode.traditional:
return _hasPrimaryFocus;
}
return null;
}
@override
Widget build(BuildContext context) {
assert(debugCheckHasMaterial(context));
......@@ -1219,10 +1309,12 @@ class _DropdownButtonState<T> extends State<DropdownButton<T>> with WidgetsBindi
Widget result = DefaultTextStyle(
style: _textStyle,
child: Container(
decoration: BoxDecoration(
color:_hasPrimaryFocus ? widget.focusColor ?? Theme.of(context).focusColor : null,
borderRadius: const BorderRadius.all(Radius.circular(4.0)),
),
decoration: _showHighlight
? BoxDecoration(
color: widget.focusColor ?? Theme.of(context).focusColor,
borderRadius: const BorderRadius.all(Radius.circular(4.0)),
)
: null,
padding: padding.resolve(Directionality.of(context)),
height: widget.isDense ? _denseButtonHeight : null,
child: Row(
......
......@@ -7,6 +7,7 @@ import 'dart:ui' show window;
import 'package:flutter_test/flutter_test.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter/gestures.dart';
import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter/services.dart';
......@@ -1059,7 +1060,10 @@ void main() {
TestSemantics(
label: 'one',
textDirection: TextDirection.ltr,
flags: <SemanticsFlag>[SemanticsFlag.isFocusable],
flags: <SemanticsFlag>[
SemanticsFlag.isFocused,
SemanticsFlag.isFocusable,
],
tags: <SemanticsTag>[const SemanticsTag('RenderViewport.twoPane')],
actions: <SemanticsAction>[SemanticsAction.tap],
),
......@@ -1916,6 +1920,7 @@ void main() {
});
testWidgets('DropdownButton can be focused, and has focusColor', (WidgetTester tester) async {
tester.binding.focusManager.highlightStrategy = FocusHighlightStrategy.alwaysTraditional;
final UniqueKey buttonKey = UniqueKey();
final FocusNode focusNode = FocusNode(debugLabel: 'DropdownButton');
await tester.pumpWidget(buildFrame(buttonKey: buttonKey, onChanged: onChanged, focusNode: focusNode, autofocus: true));
......@@ -1940,9 +1945,6 @@ void main() {
testWidgets('DropdownButton is activated with the enter/space key', (WidgetTester tester) async {
final FocusNode focusNode = FocusNode(debugLabel: 'DropdownButton');
String value = 'one';
void didChangeValue(String newValue) {
value = newValue;
}
Widget buildFrame() {
return MaterialApp(
......@@ -1953,7 +1955,11 @@ void main() {
return DropdownButton<String>(
focusNode: focusNode,
autofocus: true,
onChanged: didChangeValue,
onChanged: (String newValue) {
setState(() {
value = newValue;
});
},
value: value,
itemHeight: null,
items: menuItems.map<DropdownMenuItem<String>>((String item) {
......@@ -1981,9 +1987,7 @@ void main() {
await tester.pump(const Duration(seconds: 1)); // finish the menu animation
expect(value, equals('one'));
await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown); // Focus 'one'
await tester.pump();
await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown); // Focus 'two'
await tester.sendKeyEvent(LogicalKeyboardKey.tab); // Focus 'two'
await tester.pump();
await tester.sendKeyEvent(LogicalKeyboardKey.enter); // Select 'two', should work on web too.
await tester.pump();
......@@ -1993,4 +1997,201 @@ void main() {
expect(value, equals('two'));
});
testWidgets('Selected element is focused when dropdown is opened', (WidgetTester tester) async {
final FocusNode focusNode = FocusNode(debugLabel: 'DropdownButton');
String value = 'one';
await tester.pumpWidget(MaterialApp(
home: Scaffold(
body: Center(
child: StatefulBuilder(
builder: (BuildContext context, StateSetter setState) {
return DropdownButton<String>(
focusNode: focusNode,
autofocus: true,
onChanged: (String newValue) {
setState(() {
value = newValue;
});
},
value: value,
itemHeight: null,
items: menuItems.map<DropdownMenuItem<String>>((String item) {
return DropdownMenuItem<String>(
key: ValueKey<String>(item),
value: item,
child: Text(item, key: ValueKey<String>('Text $item')),
);
}).toList(),
);
},
),
),
),
));
await tester.pump(); // Pump a frame for autofocus to take effect.
expect(focusNode.hasPrimaryFocus, isTrue);
await tester.sendKeyEvent(LogicalKeyboardKey.enter);
await tester.pump();
await tester.pump(const Duration(seconds: 1)); // finish the menu open animation
expect(value, equals('one'));
expect(Focus.of(tester.element(find.byKey(const ValueKey<String>('one')).last)).hasPrimaryFocus, isTrue);
await tester.sendKeyEvent(LogicalKeyboardKey.tab); // Focus 'two'
await tester.pump();
await tester.sendKeyEvent(LogicalKeyboardKey.enter); // Select 'two' and close the dropdown.
await tester.pump();
await tester.pump(const Duration(seconds: 1)); // finish the menu close animation
expect(value, equals('two'));
// Now make sure that "two" is focused when we re-open the dropdown.
await tester.sendKeyEvent(LogicalKeyboardKey.enter);
await tester.pump();
await tester.pump(const Duration(seconds: 1)); // finish the menu open animation
expect(value, equals('two'));
final Element element = tester.element(find.byKey(const ValueKey<String>('two')).last);
final FocusNode node = Focus.of(element);
expect(node.hasFocus, isTrue);
}, skip: kIsWeb);
testWidgets('Selected element is correctly focused with dropdown that more items than fit on the screen', (WidgetTester tester) async {
final FocusNode focusNode = FocusNode(debugLabel: 'DropdownButton');
int value = 1;
final List<int> hugeMenuItems = List<int>.generate(50, (int index) => index);
await tester.pumpWidget(
MaterialApp(
home: Scaffold(
body: Center(
child: StatefulBuilder(
builder: (BuildContext context, StateSetter setState) {
return DropdownButton<int>(
focusNode: focusNode,
autofocus: true,
onChanged: (int newValue) {
setState(() {
value = newValue;
});
},
value: value,
itemHeight: null,
items: hugeMenuItems.map<DropdownMenuItem<int>>((int item) {
return DropdownMenuItem<int>(
key: ValueKey<int>(item),
value: item,
child: Text(item.toString(), key: ValueKey<String>('Text $item')),
);
}).toList(),
);
},
),
),
),
),
);
await tester.pump(); // Pump a frame for autofocus to take effect.
expect(focusNode.hasPrimaryFocus, isTrue);
await tester.sendKeyEvent(LogicalKeyboardKey.enter);
await tester.pump();
await tester.pump(const Duration(seconds: 1)); // finish the menu open animation
expect(value, equals(1));
expect(Focus.of(tester.element(find.byKey(const ValueKey<int>(1)).last)).hasPrimaryFocus, isTrue);
for (int i = 0; i < 41; ++i) {
await tester.sendKeyEvent(LogicalKeyboardKey.tab); // Move to the next one.
await tester.pumpAndSettle(const Duration(milliseconds: 200)); // Wait for it to animate the menu.
}
await tester.sendKeyEvent(LogicalKeyboardKey.enter); // Select '42' and close the dropdown.
await tester.pumpAndSettle(const Duration(seconds: 1)); // Finish the menu close animation
expect(value, equals(42));
// Now make sure that "42" is focused when we re-open the dropdown.
await tester.sendKeyEvent(LogicalKeyboardKey.enter);
await tester.pump();
await tester.pump(const Duration(seconds: 1)); // finish the menu open animation
expect(value, equals(42));
final Element element = tester.element(find.byKey(const ValueKey<int>(42)).last);
final FocusNode node = Focus.of(element);
expect(node.hasFocus, isTrue);
}, skip: kIsWeb);
testWidgets("Having a focused element doesn't interrupt scroll when flung by touch", (WidgetTester tester) async {
final FocusNode focusNode = FocusNode(debugLabel: 'DropdownButton');
int value = 1;
final List<int> hugeMenuItems = List<int>.generate(100, (int index) => index);
await tester.pumpWidget(
MaterialApp(
home: Scaffold(
body: Center(
child: StatefulBuilder(
builder: (BuildContext context, StateSetter setState) {
return DropdownButton<int>(
focusNode: focusNode,
autofocus: true,
onChanged: (int newValue) {
setState(() {
value = newValue;
});
},
value: value,
itemHeight: null,
items: hugeMenuItems.map<DropdownMenuItem<int>>((int item) {
return DropdownMenuItem<int>(
key: ValueKey<int>(item),
value: item,
child: Text(item.toString(), key: ValueKey<String>('Text $item')),
);
}).toList(),
);
},
),
),
),
),
);
await tester.pump(); // Pump a frame for autofocus to take effect.
expect(focusNode.hasPrimaryFocus, isTrue);
await tester.sendKeyEvent(LogicalKeyboardKey.enter);
await tester.pumpAndSettle();
expect(value, equals(1));
expect(Focus.of(tester.element(find.byKey(const ValueKey<int>(1)).last)).hasPrimaryFocus, isTrue);
// Move to an item very far down the menu.
for (int i = 0; i < 90; ++i) {
await tester.sendKeyEvent(LogicalKeyboardKey.tab); // Move to the next one.
await tester.pumpAndSettle(); // Wait for it to animate the menu.
}
expect(Focus.of(tester.element(find.byKey(const ValueKey<int>(91)).last)).hasPrimaryFocus, isTrue);
// Scroll back to the top using touch, and make sure we end up there.
final Finder menu = find.byWidgetPredicate((Widget widget) {
return widget.runtimeType.toString().startsWith('_DropdownMenu<');
});
final Rect menuRect = tester.getRect(menu).shift(tester.getTopLeft(menu));
for (int i = 0; i < 10; ++i) {
await tester.fling(menu, Offset(0.0, menuRect.height), 10.0);
}
await tester.pumpAndSettle();
// Make sure that we made it to the top and something didn't stop the
// scroll.
expect(find.byKey(const ValueKey<int>(1)), findsNWidgets(2));
expect(
tester.getRect(find.byKey(const ValueKey<int>(1)).last),
equals(const Rect.fromLTRB(372.0, 104.0, 436.0, 152.0)),
);
// Scrolling to the top again has removed the one the focus was on from the
// tree, causing it to lose focus.
expect(Focus.of(tester.element(find.byKey(const ValueKey<int>(91)).last)).hasPrimaryFocus, isFalse);
}, skip: kIsWeb);
}
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