Unverified Commit 0028887a authored by Greg Spencer's avatar Greg Spencer Committed by GitHub

Don't allow Disabled InkWells to be focusable (#43848)

Makes sure that disabled InkWell/InkResponse and widgets that use them don't allow themselves to be focused.

ListTile, PopupMenu, and Stepper were not setting canRequestFocus properly on the InkWell, and InkWell was allowing focus even if it was disabled (it was basically just relying on the containing widget to set canRequestFocus properly). Now InkWell must both be enabled (have an onTap or similar) and have canRequestFocus set to true.
parent f798cb6d
......@@ -773,11 +773,12 @@ class _InkResponseState<T extends InkResponse> extends State<T> with AutomaticKe
_highlights[type]?.color = getHighlightColorForType(type);
}
_currentSplash?.color = widget.splashColor ?? Theme.of(context).splashColor;
final bool canRequestFocus = enabled && widget.canRequestFocus;
return Actions(
actions: _actionMap,
child: Focus(
focusNode: widget.focusNode,
canRequestFocus: widget.canRequestFocus,
canRequestFocus: canRequestFocus,
onFocusChange: _handleFocusUpdate,
autofocus: widget.autofocus,
child: MouseRegion(
......
......@@ -880,6 +880,7 @@ class ListTile extends StatelessWidget {
return InkWell(
onTap: enabled ? onTap : null,
onLongPress: enabled ? onLongPress : null,
canRequestFocus: enabled,
child: Semantics(
selected: selected,
enabled: enabled,
......
......@@ -323,6 +323,7 @@ class PopupMenuItemState<T, W extends PopupMenuItem<T>> extends State<W> {
return InkWell(
onTap: widget.enabled ? handleTap : null,
canRequestFocus: widget.enabled,
child: item,
);
}
......@@ -1090,6 +1091,7 @@ class _PopupMenuButtonState<T> extends State<PopupMenuButton<T>> {
message: widget.tooltip ?? MaterialLocalizations.of(context).showMenuTooltip,
child: InkWell(
onTap: widget.enabled ? showButtonMenu : null,
canRequestFocus: widget.enabled,
child: widget.child,
),
);
......
......@@ -593,6 +593,7 @@ class _StepperState extends State<Stepper> with TickerProviderStateMixin {
if (widget.onStepTapped != null)
widget.onStepTapped(i);
} : null,
canRequestFocus: widget.steps[i].state != StepState.disabled,
child: _buildVerticalHeader(i),
),
_buildVerticalBody(i),
......@@ -610,6 +611,7 @@ class _StepperState extends State<Stepper> with TickerProviderStateMixin {
if (widget.onStepTapped != null)
widget.onStepTapped(i);
} : null,
canRequestFocus: widget.steps[i].state != StepState.disabled,
child: Row(
children: <Widget>[
Container(
......
......@@ -1620,7 +1620,6 @@ void main() {
TestSemantics(
label: 'test',
textDirection: TextDirection.ltr,
flags: <SemanticsFlag>[SemanticsFlag.isFocusable],
),
],
),
......
......@@ -322,4 +322,92 @@ void main() {
semantics.dispose();
});
testWidgets("ink response doesn't focus when disabled", (WidgetTester tester) async {
WidgetsBinding.instance.focusManager.highlightStrategy = FocusHighlightStrategy.alwaysTouch;
final FocusNode focusNode = FocusNode(debugLabel: 'Ink Focus');
final GlobalKey childKey = GlobalKey();
await tester.pumpWidget(
Material(
child: Directionality(
textDirection: TextDirection.ltr,
child: InkWell(
autofocus: true,
onTap: () {},
onLongPress: () {},
onHover: (bool hover) {},
focusNode: focusNode,
child: Container(key: childKey),
),
),
),
);
await tester.pumpAndSettle();
expect(focusNode.hasPrimaryFocus, isTrue);
await tester.pumpWidget(
Material(
child: Directionality(
textDirection: TextDirection.ltr,
child: InkWell(
focusNode: focusNode,
child: Container(key: childKey),
),
),
),
);
await tester.pumpAndSettle();
expect(focusNode.hasPrimaryFocus, isFalse);
});
testWidgets("ink response doesn't hover when disabled", (WidgetTester tester) async {
WidgetsBinding.instance.focusManager.highlightStrategy = FocusHighlightStrategy.alwaysTouch;
final FocusNode focusNode = FocusNode(debugLabel: 'Ink Focus');
final GlobalKey childKey = GlobalKey();
bool hovering = false;
await tester.pumpWidget(
Material(
child: Directionality(
textDirection: TextDirection.ltr,
child: Container(
width: 100,
height: 100,
child: InkWell(
autofocus: true,
onTap: () {},
onLongPress: () {},
onHover: (bool value) { hovering = value; },
focusNode: focusNode,
child: Container(key: childKey),
),
),
),
),
);
await tester.pumpAndSettle();
expect(focusNode.hasPrimaryFocus, isTrue);
final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
addTearDown(gesture.removePointer);
await gesture.moveTo(tester.getCenter(find.byKey(childKey)));
await tester.pumpAndSettle();
expect(hovering, isTrue);
await tester.pumpWidget(
Material(
child: Directionality(
textDirection: TextDirection.ltr,
child: Container(
width: 100,
height: 100,
child: InkWell(
focusNode: focusNode,
onHover: (bool value) { hovering = value; },
child: Container(key: childKey),
),
),
),
),
);
await tester.pumpAndSettle();
expect(focusNode.hasPrimaryFocus, isFalse);
});
}
......@@ -349,16 +349,20 @@ void main() {
child: MediaQuery(
data: const MediaQueryData(),
child: Column(
children: const <Widget>[
ListTile(
children: <Widget>[
const ListTile(
title: Text('one'),
),
ListTile(
title: Text('two'),
selected: true,
title: const Text('two'),
onTap: () {},
),
ListTile(
const ListTile(
title: Text('three'),
selected: true,
),
const ListTile(
title: Text('four'),
enabled: false,
),
],
......@@ -377,26 +381,32 @@ void main() {
flags: <SemanticsFlag>[
SemanticsFlag.hasEnabledState,
SemanticsFlag.isEnabled,
SemanticsFlag.isFocusable,
],
label: 'one',
),
TestSemantics.rootChild(
flags: <SemanticsFlag>[
SemanticsFlag.isSelected,
SemanticsFlag.hasEnabledState,
SemanticsFlag.isEnabled,
SemanticsFlag.isFocusable,
],
actions: <SemanticsAction>[SemanticsAction.tap],
label: 'two',
),
TestSemantics.rootChild(
flags: <SemanticsFlag>[
SemanticsFlag.isSelected,
SemanticsFlag.hasEnabledState,
SemanticsFlag.isFocusable,
SemanticsFlag.isEnabled,
],
label: 'three',
),
TestSemantics.rootChild(
flags: <SemanticsFlag>[
SemanticsFlag.hasEnabledState,
],
label: 'four',
),
],
),
ignoreTransform: true,
......@@ -1127,4 +1137,51 @@ void main() {
expect(tester.getRect(find.byType(Placeholder).at(0)), const Rect.fromLTWH(800.0 - 16.0 - 24.0, 16.0, 24.0, 56.0));
expect(tester.getRect(find.byType(Placeholder).at(1)), const Rect.fromLTWH(800.0 - 16.0 - 24.0, 88.0 + 16.0, 24.0, 56.0));
});
testWidgets('ListTile only accepts focus when enabled', (WidgetTester tester) async {
final GlobalKey childKey = GlobalKey();
await tester.pumpWidget(
MaterialApp(
home: Material(
child: ListView(
children: <Widget>[
ListTile(
title: Text('A', key: childKey),
dense: true,
enabled: true,
onTap: () {},
),
],
),
),
),
);
await tester.pump(); // Let the focus take effect.
final FocusNode tileNode = Focus.of(childKey.currentContext);
tileNode.requestFocus();
await tester.pump(); // Let the focus take effect.
expect(Focus.of(childKey.currentContext, nullOk: true).hasPrimaryFocus, isTrue);
expect(tileNode.hasPrimaryFocus, isTrue);
await tester.pumpWidget(
MaterialApp(
home: Material(
child: ListView(
children: <Widget>[
ListTile(
title: Text('A', key: childKey),
dense: true,
enabled: false,
onTap: () {},
),
],
),
),
),
);
expect(tester.binding.focusManager.primaryFocus, isNot(equals(tileNode)));
expect(Focus.of(childKey.currentContext, nullOk: true).hasPrimaryFocus, isFalse);
});
}
......@@ -175,6 +175,123 @@ void main() {
expect(onCanceledCalled, isFalse);
});
testWidgets('disabled PopupMenuButton is not focusable', (WidgetTester tester) async {
final Key popupButtonKey = UniqueKey();
final GlobalKey childKey = GlobalKey();
bool itemBuilderCalled = false;
bool onSelectedCalled = false;
await tester.pumpWidget(
MaterialApp(
home: Material(
child: Column(
children: <Widget>[
PopupMenuButton<int>(
key: popupButtonKey,
child: Container(key: childKey),
enabled: false,
itemBuilder: (BuildContext context) {
itemBuilderCalled = true;
return <PopupMenuEntry<int>>[
const PopupMenuItem<int>(
value: 1,
child: Text('Tap me please!'),
),
];
},
onSelected: (int selected) => onSelectedCalled = true,
),
],
),
),
),
);
Focus.of(childKey.currentContext, nullOk: true).requestFocus();
await tester.pump();
expect(Focus.of(childKey.currentContext, nullOk: true).hasPrimaryFocus, isFalse);
expect(itemBuilderCalled, isFalse);
expect(onSelectedCalled, isFalse);
});
testWidgets('PopupMenuItem is only focusable when enabled', (WidgetTester tester) async {
final Key popupButtonKey = UniqueKey();
final GlobalKey childKey = GlobalKey();
bool itemBuilderCalled = false;
await tester.pumpWidget(
MaterialApp(
home: Material(
child: Column(
children: <Widget>[
PopupMenuButton<int>(
key: popupButtonKey,
itemBuilder: (BuildContext context) {
itemBuilderCalled = true;
return <PopupMenuEntry<int>>[
PopupMenuItem<int>(
enabled: true,
value: 1,
child: Text('Tap me please!', key: childKey),
),
];
},
),
],
),
),
),
);
// Open the popup to build and show the menu contents.
await tester.tap(find.byKey(popupButtonKey));
await tester.pumpAndSettle();
final FocusNode childNode = Focus.of(childKey.currentContext, nullOk: true);
// Now that the contents are shown, request focus on the child text.
childNode.requestFocus();
await tester.pumpAndSettle();
expect(itemBuilderCalled, isTrue);
// Make sure that the focus went where we expected it to.
expect(childNode.hasPrimaryFocus, isTrue);
itemBuilderCalled = false;
// Close the popup.
await tester.tap(find.byKey(popupButtonKey));
await tester.pumpAndSettle();
await tester.pumpWidget(
MaterialApp(
home: Material(
child: Column(
children: <Widget>[
PopupMenuButton<int>(
key: popupButtonKey,
itemBuilder: (BuildContext context) {
itemBuilderCalled = true;
return <PopupMenuEntry<int>>[
PopupMenuItem<int>(
enabled: false,
value: 1,
child: Text('Tap me please!', key: childKey),
),
];
},
),
],
),
),
),
);
await tester.pumpAndSettle();
// Open the popup again to rebuild the contents with enabled == false.
await tester.tap(find.byKey(popupButtonKey));
await tester.pumpAndSettle();
expect(itemBuilderCalled, isTrue);
expect(Focus.of(childKey.currentContext, nullOk: true).hasPrimaryFocus, isFalse);
});
testWidgets('PopupMenuButton is horizontal on iOS', (WidgetTester tester) async {
Widget build(TargetPlatform platform) {
return MaterialApp(
......
......@@ -605,4 +605,56 @@ void main() {
expect(find.text('Text After Stepper'), findsNothing);
});
testWidgets("Vertical Stepper can't be focused when disabled.", (WidgetTester tester) async {
await tester.pumpWidget(
MaterialApp(
home: Material(
child: Stepper(
currentStep: 0,
type: StepperType.vertical,
steps: const <Step>[
Step(
title: Text('Step 0'),
state: StepState.disabled,
content: Text('Text 0'),
),
],
),
),
),
);
await tester.pump();
final FocusNode disabledNode = Focus.of(tester.element(find.text('Step 0')), nullOk: true, scopeOk: true);
disabledNode.requestFocus();
await tester.pump();
expect(disabledNode.hasPrimaryFocus, isFalse);
});
testWidgets("Horizontal Stepper can't be focused when disabled.", (WidgetTester tester) async {
await tester.pumpWidget(
MaterialApp(
home: Material(
child: Stepper(
currentStep: 0,
type: StepperType.horizontal,
steps: const <Step>[
Step(
title: Text('Step 0'),
state: StepState.disabled,
content: Text('Text 0'),
),
],
),
),
),
);
await tester.pump();
final FocusNode disabledNode = Focus.of(tester.element(find.text('Step 0')), nullOk: true, scopeOk: true);
disabledNode.requestFocus();
await tester.pump();
expect(disabledNode.hasPrimaryFocus, isFalse);
});
}
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