Skip to content

Commit

Permalink
Reland: Request focus if accessibility focus is given to a Focus widg…
Browse files Browse the repository at this point in the history
…et (#142942) (#149840)

## Description

This attempts to re-land #142942 after being reverted in flutter/flutter#149741 because it broke the iOS [platform view UI integration test](https://github.com/flutter/flutter/blob/master/dev/integration_tests/ios_platform_view_tests/ios/PlatformViewUITests/PlatformViewUITests.m?rgh-link-date=2024-06-06T19%3A47%3A27Z).

The changes here from the original are that in the Focus widget we no longer set the `onFocus` for the `Semantics` if the platform is iOS.  It was not intended to do anything on iOS anyhow.

Also, I updated the matchers to not actually do anything yet with the SemanticsAction.focus matching, so that this can be landed without breaking customer tests, and once they have been updated to correctly look for the focus action, we can land a PR that will turn it on.

## Related Issues
 - flutter/flutter#149838
 - flutter/flutter#83809
 - flutter/flutter#149842

## Tests
 - Updated framework tests to look for the appropriate things using the matchers, even though it doesn't actually test for them yet.
  • Loading branch information
gspencergoog authored Jun 12, 2024
1 parent 0c692b8 commit d68e05b
Show file tree
Hide file tree
Showing 59 changed files with 329 additions and 81 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ void main() {
isEnabled: true,
isFocusable: true,
hasTapAction: true,
hasFocusAction: true,
label: 'Update border shape',
));

Expand All @@ -29,6 +30,7 @@ void main() {
isEnabled: true,
isFocusable: true,
hasTapAction: true,
hasFocusAction: true,
label: 'Reset chips',
));

Expand Down
9 changes: 9 additions & 0 deletions packages/flutter/lib/src/widgets/focus_scope.dart
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,15 @@ class _FocusState extends State<Focus> {
Widget child = widget.child;
if (widget.includeSemantics) {
child = Semantics(
// Automatically request the focus for a focusable widget when it
// receives an input focus action from the semantics. Nothing is needed
// for losing the focus because if focus is lost, that means another
// node will gain focus and take focus from this widget.
// TODO(gspencergoog): Allow this to be set on iOS once the issue is
// addressed: https://github.com/flutter/flutter/issues/150030
onFocus: defaultTargetPlatform != TargetPlatform.iOS && _couldRequestFocus
? focusNode.requestFocus
: null,
focusable: _couldRequestFocus,
focused: _hadPrimaryFocus,
child: widget.child,
Expand Down
10 changes: 7 additions & 3 deletions packages/flutter/test/cupertino/checkbox_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ void main() {
hasEnabledState: true,
isEnabled: true,
hasTapAction: true,
hasFocusAction: true,
isFocusable: true,
));

Expand All @@ -54,6 +55,7 @@ void main() {
isChecked: true,
isEnabled: true,
hasTapAction: true,
hasFocusAction: true,
isFocusable: true,
));

Expand All @@ -73,6 +75,7 @@ void main() {
hasEnabledState: true,
// isFocusable is delayed by 1 frame.
isFocusable: true,
hasFocusAction: true,
));

await tester.pump();
Expand Down Expand Up @@ -178,6 +181,7 @@ void main() {
hasEnabledState: true,
isEnabled: true,
hasTapAction: true,
hasFocusAction: true,
isFocusable: true,
));
handle.dispose();
Expand Down Expand Up @@ -247,7 +251,7 @@ void main() {
SemanticsFlag.isFocusable,
SemanticsFlag.isCheckStateMixed,
],
actions: <SemanticsAction>[SemanticsAction.tap],
actions: <SemanticsAction>[SemanticsAction.tap, SemanticsAction.focus],
), hasLength(1));

await tester.pumpWidget(
Expand All @@ -268,7 +272,7 @@ void main() {
SemanticsFlag.isChecked,
SemanticsFlag.isFocusable,
],
actions: <SemanticsAction>[SemanticsAction.tap],
actions: <SemanticsAction>[SemanticsAction.tap, SemanticsAction.focus],
), hasLength(1));

await tester.pumpWidget(
Expand All @@ -288,7 +292,7 @@ void main() {
SemanticsFlag.isEnabled,
SemanticsFlag.isFocusable,
],
actions: <SemanticsAction>[SemanticsAction.tap],
actions: <SemanticsAction>[SemanticsAction.tap, SemanticsAction.focus],
), hasLength(1));

semantics.dispose();
Expand Down
4 changes: 4 additions & 0 deletions packages/flutter/test/cupertino/radio_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ void main() {
],
actions: <SemanticsAction>[
SemanticsAction.tap,
SemanticsAction.focus,
],
),
);
Expand All @@ -172,6 +173,7 @@ void main() {
hasEnabledState: true,
isEnabled: true,
hasTapAction: true,
hasFocusAction: true,
isFocusable: true,
isInMutuallyExclusiveGroup: true,
));
Expand All @@ -191,6 +193,7 @@ void main() {
hasEnabledState: true,
isEnabled: true,
hasTapAction: true,
hasFocusAction: true,
isFocusable: true,
isInMutuallyExclusiveGroup: true,
isChecked: true,
Expand All @@ -211,6 +214,7 @@ void main() {
hasEnabledState: true,
isFocusable: true,
isInMutuallyExclusiveGroup: true,
hasFocusAction: true,
));

await tester.pump();
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter/test/cupertino/route_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1945,7 +1945,7 @@ void main() {
await tester.pumpAndSettle();

expect(semantics, isNot(includesNodeWith(
actions: <SemanticsAction>[SemanticsAction.tap],
actions: <SemanticsAction>[SemanticsAction.tap, SemanticsAction.focus],
label: 'Dismiss',
)));
debugDefaultTargetPlatformOverride = null;
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter/test/cupertino/text_field_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2621,7 +2621,7 @@ void main() {
),
);

expect(semantics, isNot(includesNodeWith(actions: <SemanticsAction>[SemanticsAction.tap])));
expect(semantics, isNot(includesNodeWith(actions: <SemanticsAction>[SemanticsAction.tap, SemanticsAction.focus])));

semantics.dispose();
});
Expand Down
2 changes: 2 additions & 0 deletions packages/flutter/test/material/back_button_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ void main() {
hasEnabledState: true,
isEnabled: true,
hasTapAction: true,
hasFocusAction: true,
isFocusable: true,
));
handle.dispose();
Expand Down Expand Up @@ -258,6 +259,7 @@ void main() {
hasEnabledState: true,
isEnabled: true,
hasTapAction: true,
hasFocusAction: true,
isFocusable: true,
));
handle.dispose();
Expand Down
14 changes: 12 additions & 2 deletions packages/flutter/test/material/bottom_navigation_bar_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2111,6 +2111,7 @@ void main() {
isFocusable: true,
isSelected: true,
hasTapAction: true,
hasFocusAction: true,
),
);
expect(
Expand All @@ -2120,6 +2121,7 @@ void main() {
textDirection: TextDirection.ltr,
isFocusable: true,
hasTapAction: true,
hasFocusAction: true,
),
);
expect(
Expand All @@ -2129,6 +2131,7 @@ void main() {
textDirection: TextDirection.ltr,
isFocusable: true,
hasTapAction: true,
hasFocusAction: true,
),
);
});
Expand Down Expand Up @@ -2165,6 +2168,7 @@ void main() {
isFocusable: true,
isSelected: true,
hasTapAction: true,
hasFocusAction: true,
),
);
expect(
Expand All @@ -2174,6 +2178,7 @@ void main() {
textDirection: TextDirection.ltr,
isFocusable: true,
hasTapAction: true,
hasFocusAction: true,
),
);
expect(
Expand All @@ -2183,6 +2188,7 @@ void main() {
textDirection: TextDirection.ltr,
isFocusable: true,
hasTapAction: true,
hasFocusAction: true,
),
);
});
Expand Down Expand Up @@ -2515,6 +2521,7 @@ void main() {
isFocusable: true,
isSelected: true,
hasTapAction: true,
hasFocusAction: true,
),
);
expect(
Expand All @@ -2524,6 +2531,7 @@ void main() {
textDirection: TextDirection.ltr,
isFocusable: true,
hasTapAction: true,
hasFocusAction: true,
),
);
});
Expand Down Expand Up @@ -2558,6 +2566,7 @@ void main() {
isFocusable: true,
isSelected: true,
hasTapAction: true,
hasFocusAction: true,
),
);
expect(
Expand All @@ -2567,6 +2576,7 @@ void main() {
textDirection: TextDirection.ltr,
isFocusable: true,
hasTapAction: true,
hasFocusAction: true,
),
);
});
Expand Down Expand Up @@ -2747,13 +2757,13 @@ void main() {
SemanticsFlag.isSelected,
SemanticsFlag.isFocusable,
],
actions: <SemanticsAction>[SemanticsAction.tap],
actions: <SemanticsAction>[SemanticsAction.tap, SemanticsAction.focus],
label: 'A\nTab 1 of 2',
textDirection: TextDirection.ltr,
),
TestSemantics(
flags: <SemanticsFlag>[SemanticsFlag.isFocusable],
actions: <SemanticsAction>[SemanticsAction.tap],
actions: <SemanticsAction>[SemanticsAction.tap, SemanticsAction.focus],
label: 'B\nTab 2 of 2',
textDirection: TextDirection.ltr,
),
Expand Down
Loading

0 comments on commit d68e05b

Please sign in to comment.