Unverified Commit 49f620d8 authored by Nitesh Sharma's avatar Nitesh Sharma Committed by GitHub

Fix dual focus issue in CheckboxListTile, RadioListTile and SwitchListTile (#143213)

These widgets can now only receive focus once when tabbing through the focus tree.
parent ace3e58f
...@@ -475,7 +475,8 @@ class CheckboxListTile extends StatelessWidget { ...@@ -475,7 +475,8 @@ class CheckboxListTile extends StatelessWidget {
switch (_checkboxType) { switch (_checkboxType) {
case _CheckboxType.material: case _CheckboxType.material:
control = Checkbox( control = ExcludeFocus(
child: Checkbox(
value: value, value: value,
onChanged: enabled ?? true ? onChanged : null, onChanged: enabled ?? true ? onChanged : null,
mouseCursor: mouseCursor, mouseCursor: mouseCursor,
...@@ -492,9 +493,11 @@ class CheckboxListTile extends StatelessWidget { ...@@ -492,9 +493,11 @@ class CheckboxListTile extends StatelessWidget {
side: side, side: side,
isError: isError, isError: isError,
semanticLabel: checkboxSemanticLabel, semanticLabel: checkboxSemanticLabel,
),
); );
case _CheckboxType.adaptive: case _CheckboxType.adaptive:
control = Checkbox.adaptive( control = ExcludeFocus(
child: Checkbox.adaptive(
value: value, value: value,
onChanged: enabled ?? true ? onChanged : null, onChanged: enabled ?? true ? onChanged : null,
mouseCursor: mouseCursor, mouseCursor: mouseCursor,
...@@ -511,6 +514,7 @@ class CheckboxListTile extends StatelessWidget { ...@@ -511,6 +514,7 @@ class CheckboxListTile extends StatelessWidget {
side: side, side: side,
isError: isError, isError: isError,
semanticLabel: checkboxSemanticLabel, semanticLabel: checkboxSemanticLabel,
),
); );
} }
......
...@@ -452,7 +452,8 @@ class RadioListTile<T> extends StatelessWidget { ...@@ -452,7 +452,8 @@ class RadioListTile<T> extends StatelessWidget {
final Widget control; final Widget control;
switch (_radioType) { switch (_radioType) {
case _RadioType.material: case _RadioType.material:
control = Radio<T>( control = ExcludeFocus(
child: Radio<T>(
value: value, value: value,
groupValue: groupValue, groupValue: groupValue,
onChanged: onChanged, onChanged: onChanged,
...@@ -465,9 +466,11 @@ class RadioListTile<T> extends StatelessWidget { ...@@ -465,9 +466,11 @@ class RadioListTile<T> extends StatelessWidget {
hoverColor: hoverColor, hoverColor: hoverColor,
overlayColor: overlayColor, overlayColor: overlayColor,
splashRadius: splashRadius, splashRadius: splashRadius,
),
); );
case _RadioType.adaptive: case _RadioType.adaptive:
control = Radio<T>.adaptive( control = ExcludeFocus(
child: Radio<T>.adaptive(
value: value, value: value,
groupValue: groupValue, groupValue: groupValue,
onChanged: onChanged, onChanged: onChanged,
...@@ -481,6 +484,7 @@ class RadioListTile<T> extends StatelessWidget { ...@@ -481,6 +484,7 @@ class RadioListTile<T> extends StatelessWidget {
overlayColor: overlayColor, overlayColor: overlayColor,
splashRadius: splashRadius, splashRadius: splashRadius,
useCupertinoCheckmarkStyle: useCupertinoCheckmarkStyle, useCupertinoCheckmarkStyle: useCupertinoCheckmarkStyle,
),
); );
} }
......
...@@ -511,7 +511,8 @@ class SwitchListTile extends StatelessWidget { ...@@ -511,7 +511,8 @@ class SwitchListTile extends StatelessWidget {
final Widget control; final Widget control;
switch (_switchListTileType) { switch (_switchListTileType) {
case _SwitchListTileType.adaptive: case _SwitchListTileType.adaptive:
control = Switch.adaptive( control = ExcludeFocus(
child: Switch.adaptive(
value: value, value: value,
onChanged: onChanged, onChanged: onChanged,
activeColor: activeColor, activeColor: activeColor,
...@@ -534,10 +535,12 @@ class SwitchListTile extends StatelessWidget { ...@@ -534,10 +535,12 @@ class SwitchListTile extends StatelessWidget {
mouseCursor: mouseCursor, mouseCursor: mouseCursor,
splashRadius: splashRadius, splashRadius: splashRadius,
overlayColor: overlayColor, overlayColor: overlayColor,
),
); );
case _SwitchListTileType.material: case _SwitchListTileType.material:
control = Switch( control = ExcludeFocus(
child: Switch(
value: value, value: value,
onChanged: onChanged, onChanged: onChanged,
activeColor: activeColor, activeColor: activeColor,
...@@ -559,6 +562,7 @@ class SwitchListTile extends StatelessWidget { ...@@ -559,6 +562,7 @@ class SwitchListTile extends StatelessWidget {
mouseCursor: mouseCursor, mouseCursor: mouseCursor,
splashRadius: splashRadius, splashRadius: splashRadius,
overlayColor: overlayColor, overlayColor: overlayColor,
),
); );
} }
......
...@@ -1194,6 +1194,41 @@ void main() { ...@@ -1194,6 +1194,41 @@ void main() {
handle.dispose(); handle.dispose();
}); });
testWidgets('CheckboxListTile.control widget should not request focus on traversal', (WidgetTester tester) async {
final GlobalKey firstChildKey = GlobalKey();
final GlobalKey secondChildKey = GlobalKey();
await tester.pumpWidget(
MaterialApp(
home: Material(
child: Column(
children: <Widget>[
CheckboxListTile(
value: true,
onChanged: (bool? value) {},
title: Text('Hey', key: firstChildKey),
),
CheckboxListTile(
value: true,
onChanged: (bool? value) {},
title: Text('There', key: secondChildKey),
),
],
),
),
),
);
await tester.pump();
Focus.of(firstChildKey.currentContext!).requestFocus();
await tester.pump();
expect(Focus.of(firstChildKey.currentContext!).hasPrimaryFocus, isTrue);
Focus.of(firstChildKey.currentContext!).nextFocus();
await tester.pump();
expect(Focus.of(firstChildKey.currentContext!).hasPrimaryFocus, isFalse);
expect(Focus.of(secondChildKey.currentContext!).hasPrimaryFocus, isTrue);
});
} }
class _SelectedGrabMouseCursor extends MaterialStateMouseCursor { class _SelectedGrabMouseCursor extends MaterialStateMouseCursor {
......
...@@ -1260,6 +1260,43 @@ void main() { ...@@ -1260,6 +1260,43 @@ void main() {
expect(tester.getSize(find.byType(Radio<bool>)), const Size(48.0, 48.0)); expect(tester.getSize(find.byType(Radio<bool>)), const Size(48.0, 48.0));
}); });
testWidgets('RadioListTile.control widget should not request focus on traversal', (WidgetTester tester) async {
final GlobalKey firstChildKey = GlobalKey();
final GlobalKey secondChildKey = GlobalKey();
await tester.pumpWidget(
MaterialApp(
home: Material(
child: Column(
children: <Widget>[
RadioListTile<bool>(
value: true,
groupValue: true,
onChanged: (bool? value) {},
title: Text('Hey', key: firstChildKey),
),
RadioListTile<bool>(
value: true,
groupValue: true,
onChanged: (bool? value) {},
title: Text('There', key: secondChildKey),
),
],
),
),
),
);
await tester.pump();
Focus.of(firstChildKey.currentContext!).requestFocus();
await tester.pump();
expect(Focus.of(firstChildKey.currentContext!).hasPrimaryFocus, isTrue);
Focus.of(firstChildKey.currentContext!).nextFocus();
await tester.pump();
expect(Focus.of(firstChildKey.currentContext!).hasPrimaryFocus, isFalse);
expect(Focus.of(secondChildKey.currentContext!).hasPrimaryFocus, isTrue);
});
testWidgets('RadioListTile.adaptive shows the correct radio platform widget', (WidgetTester tester) async { testWidgets('RadioListTile.adaptive shows the correct radio platform widget', (WidgetTester tester) async {
Widget buildApp(TargetPlatform platform) { Widget buildApp(TargetPlatform platform) {
return MaterialApp( return MaterialApp(
......
...@@ -359,7 +359,19 @@ void main() { ...@@ -359,7 +359,19 @@ void main() {
final ListTile listTile = tester.widget(find.byType(ListTile)); final ListTile listTile = tester.widget(find.byType(ListTile));
// When controlAffinity is ListTileControlAffinity.leading, the position of // When controlAffinity is ListTileControlAffinity.leading, the position of
// Switch is at leading edge and SwitchListTile.secondary at trailing edge. // Switch is at leading edge and SwitchListTile.secondary at trailing edge.
expect(listTile.leading.runtimeType, Switch);
// Find the ExcludeFocus widget within the ListTile's leading
final ExcludeFocus excludeFocusWidget = tester.widget(
find.byWidgetPredicate((Widget widget) => listTile.leading == widget && widget is ExcludeFocus),
);
// Assert that the ExcludeFocus widget is not null
expect(excludeFocusWidget, isNotNull);
// Assert that the child of ExcludeFocus is Switch
expect(excludeFocusWidget.child.runtimeType, Switch);
// Assert that the trailing is Icon
expect(listTile.trailing.runtimeType, Icon); expect(listTile.trailing.runtimeType, Icon);
}); });
...@@ -379,8 +391,20 @@ void main() { ...@@ -379,8 +391,20 @@ void main() {
// By default, value of controlAffinity is ListTileControlAffinity.platform, // By default, value of controlAffinity is ListTileControlAffinity.platform,
// where the position of SwitchListTile.secondary is at leading edge and Switch // where the position of SwitchListTile.secondary is at leading edge and Switch
// at trailing edge. This also covers test for ListTileControlAffinity.trailing. // at trailing edge. This also covers test for ListTileControlAffinity.trailing.
// Find the ExcludeFocus widget within the ListTile's trailing
final ExcludeFocus excludeFocusWidget = tester.widget(
find.byWidgetPredicate((Widget widget) => listTile.trailing == widget && widget is ExcludeFocus),
);
// Assert that the ExcludeFocus widget is not null
expect(excludeFocusWidget, isNotNull);
// Assert that the child of ExcludeFocus is Switch
expect(excludeFocusWidget.child.runtimeType, Switch);
// Assert that the leading is Icon
expect(listTile.leading.runtimeType, Icon); expect(listTile.leading.runtimeType, Icon);
expect(listTile.trailing.runtimeType, Switch);
}); });
testWidgets('SwitchListTile respects shape', (WidgetTester tester) async { testWidgets('SwitchListTile respects shape', (WidgetTester tester) async {
...@@ -1632,4 +1656,39 @@ void main() { ...@@ -1632,4 +1656,39 @@ void main() {
paints..rrect()..rrect(color: hoveredTrackColor, style: PaintingStyle.stroke) paints..rrect()..rrect(color: hoveredTrackColor, style: PaintingStyle.stroke)
); );
}); });
testWidgets('SwitchListTile.control widget should not request focus on traversal', (WidgetTester tester) async {
final GlobalKey firstChildKey = GlobalKey();
final GlobalKey secondChildKey = GlobalKey();
await tester.pumpWidget(
MaterialApp(
home: Material(
child: Column(
children: <Widget>[
SwitchListTile(
value: true,
onChanged: (bool? value) {},
title: Text('Hey', key: firstChildKey),
),
SwitchListTile(
value: true,
onChanged: (bool? value) {},
title: Text('There', key: secondChildKey),
),
],
),
),
),
);
await tester.pump();
Focus.of(firstChildKey.currentContext!).requestFocus();
await tester.pump();
expect(Focus.of(firstChildKey.currentContext!).hasPrimaryFocus, isTrue);
Focus.of(firstChildKey.currentContext!).nextFocus();
await tester.pump();
expect(Focus.of(firstChildKey.currentContext!).hasPrimaryFocus, isFalse);
expect(Focus.of(secondChildKey.currentContext!).hasPrimaryFocus, isTrue);
});
} }
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