Unverified Commit 8daa165d authored by Greg Spencer's avatar Greg Spencer Committed by GitHub

Make disabled buttons/chips/text fields not be focusable. (#38726)

This changes the behavior of text fields, Material buttons, and Chips so that if they are disabled they lose focus. Before this change, it was possible to disable a control and then use focus traversal to reach it anyhow, and in the case of text fields, enter text into a disabled field.

Fixes #33985
parent 0ebcfe10
...@@ -332,6 +332,7 @@ class _RawMaterialButtonState extends State<RawMaterialButton> { ...@@ -332,6 +332,7 @@ class _RawMaterialButtonState extends State<RawMaterialButton> {
final Widget result = Focus( final Widget result = Focus(
focusNode: widget.focusNode, focusNode: widget.focusNode,
canRequestFocus: widget.enabled,
onFocusChange: _handleFocusedChanged, onFocusChange: _handleFocusedChanged,
autofocus: widget.autofocus, autofocus: widget.autofocus,
child: ConstrainedBox( child: ConstrainedBox(
......
...@@ -1728,6 +1728,7 @@ class _RawChipState extends State<RawChip> with TickerProviderStateMixin<RawChip ...@@ -1728,6 +1728,7 @@ class _RawChipState extends State<RawChip> with TickerProviderStateMixin<RawChip
onFocusChange: _handleFocus, onFocusChange: _handleFocus,
focusNode: widget.focusNode, focusNode: widget.focusNode,
autofocus: widget.autofocus, autofocus: widget.autofocus,
canRequestFocus: widget.isEnabled,
child: Material( child: Material(
elevation: isTapping ? pressElevation : elevation, elevation: isTapping ? pressElevation : elevation,
shadowColor: widget.selected ? selectedShadowColor : shadowColor, shadowColor: widget.selected ? selectedShadowColor : shadowColor,
......
...@@ -313,6 +313,7 @@ class IconButton extends StatelessWidget { ...@@ -313,6 +313,7 @@ class IconButton extends StatelessWidget {
child: Focus( child: Focus(
focusNode: focusNode, focusNode: focusNode,
autofocus: autofocus, autofocus: autofocus,
canRequestFocus: onPressed != null,
child: InkResponse( child: InkResponse(
onTap: onPressed, onTap: onPressed,
child: result, child: result,
......
...@@ -686,6 +686,8 @@ class _TextFieldState extends State<TextField> with AutomaticKeepAliveClientMixi ...@@ -686,6 +686,8 @@ class _TextFieldState extends State<TextField> with AutomaticKeepAliveClientMixi
bool get selectionEnabled => widget.selectionEnabled; bool get selectionEnabled => widget.selectionEnabled;
// End of API for TextSelectionGestureDetectorBuilderDelegate. // End of API for TextSelectionGestureDetectorBuilderDelegate.
bool get _isEnabled => widget.enabled ?? widget.decoration?.enabled ?? true;
InputDecoration _getEffectiveDecoration() { InputDecoration _getEffectiveDecoration() {
final MaterialLocalizations localizations = MaterialLocalizations.of(context); final MaterialLocalizations localizations = MaterialLocalizations.of(context);
final ThemeData themeData = Theme.of(context); final ThemeData themeData = Theme.of(context);
...@@ -755,9 +757,11 @@ class _TextFieldState extends State<TextField> with AutomaticKeepAliveClientMixi ...@@ -755,9 +757,11 @@ class _TextFieldState extends State<TextField> with AutomaticKeepAliveClientMixi
void initState() { void initState() {
super.initState(); super.initState();
_selectionGestureDetectorBuilder = _TextFieldSelectionGestureDetectorBuilder(state: this); _selectionGestureDetectorBuilder = _TextFieldSelectionGestureDetectorBuilder(state: this);
if (widget.controller == null) if (widget.controller == null) {
_controller = TextEditingController(); _controller = TextEditingController();
} }
_effectiveFocusNode.canRequestFocus = _isEnabled;
}
@override @override
void didUpdateWidget(TextField oldWidget) { void didUpdateWidget(TextField oldWidget) {
...@@ -766,11 +770,7 @@ class _TextFieldState extends State<TextField> with AutomaticKeepAliveClientMixi ...@@ -766,11 +770,7 @@ class _TextFieldState extends State<TextField> with AutomaticKeepAliveClientMixi
_controller = TextEditingController.fromValue(oldWidget.controller.value); _controller = TextEditingController.fromValue(oldWidget.controller.value);
else if (widget.controller != null && oldWidget.controller == null) else if (widget.controller != null && oldWidget.controller == null)
_controller = null; _controller = null;
final bool isEnabled = widget.enabled ?? widget.decoration?.enabled ?? true; _effectiveFocusNode.canRequestFocus = _isEnabled;
final bool wasEnabled = oldWidget.enabled ?? oldWidget.decoration?.enabled ?? true;
if (wasEnabled && !isEnabled) {
_effectiveFocusNode.unfocus();
}
if (_effectiveFocusNode.hasFocus && widget.readOnly != oldWidget.readOnly) { if (_effectiveFocusNode.hasFocus && widget.readOnly != oldWidget.readOnly) {
if(_effectiveController.selection.isCollapsed) { if(_effectiveController.selection.isCollapsed) {
_showSelectionHandles = !widget.readOnly; _showSelectionHandles = !widget.readOnly;
...@@ -1045,7 +1045,7 @@ class _TextFieldState extends State<TextField> with AutomaticKeepAliveClientMixi ...@@ -1045,7 +1045,7 @@ class _TextFieldState extends State<TextField> with AutomaticKeepAliveClientMixi
onEnter: _handleMouseEnter, onEnter: _handleMouseEnter,
onExit: _handleMouseExit, onExit: _handleMouseExit,
child: IgnorePointer( child: IgnorePointer(
ignoring: !(widget.enabled ?? widget.decoration?.enabled ?? true), ignoring: !_isEnabled,
child: _selectionGestureDetectorBuilder.buildGestureDetector( child: _selectionGestureDetectorBuilder.buildGestureDetector(
behavior: HitTestBehavior.translucent, behavior: HitTestBehavior.translucent,
child: child, child: child,
......
...@@ -391,7 +391,8 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier { ...@@ -391,7 +391,8 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
set skipTraversal(bool value) { set skipTraversal(bool value) {
if (value != _skipTraversal) { if (value != _skipTraversal) {
_skipTraversal = value; _skipTraversal = value;
_notify(); _manager?._dirtyNodes?.add(this);
_manager?._markNeedsUpdate();
} }
} }
...@@ -423,7 +424,8 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier { ...@@ -423,7 +424,8 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
if (!_canRequestFocus) { if (!_canRequestFocus) {
unfocus(); unfocus();
} }
_notify(); _manager?._dirtyNodes?.add(this);
_manager?._markNeedsUpdate();
} }
} }
......
...@@ -345,7 +345,7 @@ class _ShortcutsState extends State<Shortcuts> { ...@@ -345,7 +345,7 @@ class _ShortcutsState extends State<Shortcuts> {
@override @override
Widget build(BuildContext context) { Widget build(BuildContext context) {
return Focus( return Focus(
skipTraversal: true, canRequestFocus: false,
onKey: _handleOnKey, onKey: _handleOnKey,
child: _ShortcutsMarker( child: _ShortcutsMarker(
manager: manager, manager: manager,
......
...@@ -1837,4 +1837,71 @@ void main() { ...@@ -1837,4 +1837,71 @@ void main() {
// Teardown. // Teardown.
await gesture.removePointer(); await gesture.removePointer();
}); });
testWidgets('loses focus when disabled', (WidgetTester tester) async {
final FocusNode focusNode = FocusNode(debugLabel: 'InputChip');
await tester.pumpWidget(
_wrapForChip(
child: InputChip(
focusNode: focusNode,
autofocus: true,
shape: const RoundedRectangleBorder(borderRadius: BorderRadius.all(Radius.circular(0.0))),
avatar: const CircleAvatar(child: Text('A')),
label: const Text('Chip A'),
onPressed: () { },
),
),
);
await tester.pump();
expect(focusNode.hasPrimaryFocus, isTrue);
await tester.pumpWidget(
_wrapForChip(
child: InputChip(
focusNode: focusNode,
autofocus: true,
shape: const RoundedRectangleBorder(borderRadius: BorderRadius.all(Radius.circular(0.0))),
avatar: const CircleAvatar(child: Text('A')),
label: const Text('Chip A'),
onPressed: null,
),
),
);
await tester.pump();
expect(focusNode.hasPrimaryFocus, isFalse);
});
testWidgets('cannot be traversed to when disabled', (WidgetTester tester) async {
final FocusNode focusNode1 = FocusNode(debugLabel: 'InputChip 1');
final FocusNode focusNode2 = FocusNode(debugLabel: 'InputChip 2');
await tester.pumpWidget(
_wrapForChip(
child: Column(
children: <Widget>[
InputChip(
focusNode: focusNode1,
autofocus: true,
label: const Text('Chip A'),
onPressed: () { },
),
InputChip(
focusNode: focusNode2,
autofocus: true,
label: const Text('Chip B'),
onPressed: null,
),
],
),
),
);
await tester.pump();
expect(focusNode1.hasPrimaryFocus, isTrue);
expect(focusNode2.hasPrimaryFocus, isFalse);
expect(focusNode1.nextFocus(), isTrue);
await tester.pump();
expect(focusNode1.hasPrimaryFocus, isTrue);
expect(focusNode2.hasPrimaryFocus, isFalse);
});
} }
...@@ -332,13 +332,81 @@ void main() { ...@@ -332,13 +332,81 @@ void main() {
semantics.dispose(); semantics.dispose();
}); });
testWidgets('IconButton loses focus when disabled.', (WidgetTester tester) async {
final FocusNode focusNode = FocusNode(debugLabel: 'IconButton');
await tester.pumpWidget(
wrap(
child: IconButton(
focusNode: focusNode,
autofocus: true,
onPressed: () {},
icon: const Icon(Icons.link),
),
),
);
await tester.pump();
expect(focusNode.hasPrimaryFocus, isTrue);
await tester.pumpWidget(
wrap(
child: IconButton(
focusNode: focusNode,
autofocus: true,
onPressed: null,
icon: const Icon(Icons.link),
),
),
);
await tester.pump();
expect(focusNode.hasPrimaryFocus, isFalse);
});
testWidgets("Disabled IconButton can't be traversed to when disabled.", (WidgetTester tester) async {
final FocusNode focusNode1 = FocusNode(debugLabel: 'IconButton 1');
final FocusNode focusNode2 = FocusNode(debugLabel: 'IconButton 2');
await tester.pumpWidget(
wrap(
child: Column(
children: <Widget>[
IconButton(
focusNode: focusNode1,
autofocus: true,
onPressed: () {},
icon: const Icon(Icons.link),
),
IconButton(
focusNode: focusNode2,
onPressed: null,
icon: const Icon(Icons.link),
),
],
),
),
);
await tester.pump();
expect(focusNode1.hasPrimaryFocus, isTrue);
expect(focusNode2.hasPrimaryFocus, isFalse);
expect(focusNode1.nextFocus(), isTrue);
await tester.pump();
expect(focusNode1.hasPrimaryFocus, isTrue);
expect(focusNode2.hasPrimaryFocus, isFalse);
});
} }
Widget wrap({ Widget child }) { Widget wrap({ Widget child }) {
return Directionality( return DefaultFocusTraversal(
policy: ReadingOrderTraversalPolicy(),
child: Directionality(
textDirection: TextDirection.ltr, textDirection: TextDirection.ltr,
child: Material( child: Material(
child: Center(child: child), child: Center(child: child),
), ),
),
); );
} }
...@@ -237,6 +237,78 @@ void main() { ...@@ -237,6 +237,78 @@ void main() {
expect(box, paints..rect(color: focusColor)); expect(box, paints..rect(color: focusColor));
}); });
testWidgets('$RawMaterialButton loses focus when disabled.', (WidgetTester tester) async {
final FocusNode focusNode = FocusNode(debugLabel: 'RawMaterialButton');
await tester.pumpWidget(
MaterialApp(
home: Center(
child: RawMaterialButton(
autofocus: true,
focusNode: focusNode,
onPressed: () {},
child: Container(width: 100, height: 100, color: const Color(0xffff0000)),
),
),
),
);
await tester.pump();
expect(focusNode.hasPrimaryFocus, isTrue);
await tester.pumpWidget(
MaterialApp(
home: Center(
child: RawMaterialButton(
focusNode: focusNode,
onPressed: null,
child: Container(width: 100, height: 100, color: const Color(0xffff0000)),
),
),
),
);
await tester.pump();
expect(focusNode.hasPrimaryFocus, isFalse);
});
testWidgets("Disabled $RawMaterialButton can't be traversed to when disabled.", (WidgetTester tester) async {
final FocusNode focusNode1 = FocusNode(debugLabel: '$RawMaterialButton 1');
final FocusNode focusNode2 = FocusNode(debugLabel: '$RawMaterialButton 2');
await tester.pumpWidget(
MaterialApp(
home: Center(
child: Column(
children: <Widget>[
RawMaterialButton(
autofocus: true,
focusNode: focusNode1,
onPressed: () {},
child: Container(width: 100, height: 100, color: const Color(0xffff0000)),
),
RawMaterialButton(
autofocus: true,
focusNode: focusNode2,
onPressed: null,
child: Container(width: 100, height: 100, color: const Color(0xffff0000)),
),
],
),
),
),
);
await tester.pump();
expect(focusNode1.hasPrimaryFocus, isTrue);
expect(focusNode2.hasPrimaryFocus, isFalse);
expect(focusNode1.nextFocus(), isTrue);
await tester.pump();
expect(focusNode1.hasPrimaryFocus, isTrue);
expect(focusNode2.hasPrimaryFocus, isFalse);
});
testWidgets('$RawMaterialButton handles hover', (WidgetTester tester) async { testWidgets('$RawMaterialButton handles hover', (WidgetTester tester) async {
const Key key = Key('test'); const Key key = Key('test');
const Color hoverColor = Color(0xff00ff00); const Color hoverColor = Color(0xff00ff00);
......
...@@ -3391,6 +3391,83 @@ void main() { ...@@ -3391,6 +3391,83 @@ void main() {
semantics.dispose(); semantics.dispose();
}); });
testWidgets('TextField loses focus when disabled', (WidgetTester tester) async {
final FocusNode focusNode = FocusNode(debugLabel: 'TextField');
await tester.pumpWidget(
MaterialApp(
home: Material(
child: Center(
child: TextField(
focusNode: focusNode,
autofocus: true,
maxLength: 10,
enabled: true,
),
),
),
),
);
await tester.pump();
expect(focusNode.hasPrimaryFocus, isTrue);
await tester.pumpWidget(
MaterialApp(
home: Material(
child: Center(
child: TextField(
focusNode: focusNode,
autofocus: true,
maxLength: 10,
enabled: false,
),
),
),
),
);
await tester.pump();
expect(focusNode.hasPrimaryFocus, isFalse);
});
testWidgets("Disabled TextField can't be traversed to when disabled.", (WidgetTester tester) async {
final FocusNode focusNode1 = FocusNode(debugLabel: 'TextField 1');
final FocusNode focusNode2 = FocusNode(debugLabel: 'TextField 2');
await tester.pumpWidget(
MaterialApp(
home: Material(
child: Center(
child: Column(
children: <Widget>[
TextField(
focusNode: focusNode1,
autofocus: true,
maxLength: 10,
enabled: true,
),
TextField(
focusNode: focusNode2,
maxLength: 10,
enabled: false,
),
],
),
),
),
),
);
await tester.pump();
expect(focusNode1.hasPrimaryFocus, isTrue);
expect(focusNode2.hasPrimaryFocus, isFalse);
expect(focusNode1.nextFocus(), isTrue);
await tester.pump();
expect(focusNode1.hasPrimaryFocus, isTrue);
expect(focusNode2.hasPrimaryFocus, isFalse);
});
void sendFakeKeyEvent(Map<String, dynamic> data) { void sendFakeKeyEvent(Map<String, dynamic> data) {
ServicesBinding.instance.defaultBinaryMessenger.handlePlatformMessage( ServicesBinding.instance.defaultBinaryMessenger.handlePlatformMessage(
SystemChannels.keyEvent.name, SystemChannels.keyEvent.name,
......
...@@ -184,36 +184,11 @@ void main() { ...@@ -184,36 +184,11 @@ void main() {
); );
expect(_validateCalled, 1); expect(_validateCalled, 1);
await tester.showKeyboard(find.byType(TextField));
await tester.enterText(find.byType(TextField), 'a'); await tester.enterText(find.byType(TextField), 'a');
await tester.pump(); await tester.pump();
expect(_validateCalled, 2); expect(_validateCalled, 2);
}); });
testWidgets('validate is not called if widget is disabled', (WidgetTester tester) async {
int _validateCalled = 0;
await tester.pumpWidget(
MaterialApp(
home: Material(
child: Center(
child: TextFormField(
enabled: false,
autovalidate: true,
validator: (String value) { _validateCalled += 1; return null; },
),
),
),
),
);
expect(_validateCalled, 0);
await tester.showKeyboard(find.byType(TextField));
await tester.enterText(find.byType(TextField), 'a');
await tester.pump();
expect(_validateCalled, 0);
});
testWidgets('validate is called if widget is enabled', (WidgetTester tester) async { testWidgets('validate is called if widget is enabled', (WidgetTester tester) async {
int _validateCalled = 0; int _validateCalled = 0;
...@@ -232,7 +207,6 @@ void main() { ...@@ -232,7 +207,6 @@ void main() {
); );
expect(_validateCalled, 1); expect(_validateCalled, 1);
await tester.showKeyboard(find.byType(TextField));
await tester.enterText(find.byType(TextField), 'a'); await tester.enterText(find.byType(TextField), 'a');
await tester.pump(); await tester.pump();
expect(_validateCalled, 2); expect(_validateCalled, 2);
......
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