Unverified Commit 2e8af2c3 authored by chunhtai's avatar chunhtai Committed by GitHub

Unifies text field focus management in desktops (#129652)

related https://github.com/flutter/flutter/issues/128709

engine PR: https://github.com/flutter/engine/pull/43279

The web engine requires a way to unfocus textfield, It comes to nature
to me that we should leverage didGain/didLose a11y focus action. I also
unifies the action handler of all desktop platforms

## Pre-launch Checklist

- [ ] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [ ] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [ ] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [ ] I signed the [CLA].
- [ ] I listed at least one issue that this PR fixes in the description
above.
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [ ] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [ ] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
parent ab86c79c
...@@ -1290,6 +1290,7 @@ class _TextFieldState extends State<TextField> with RestorationMixin implements ...@@ -1290,6 +1290,7 @@ class _TextFieldState extends State<TextField> with RestorationMixin implements
Color? autocorrectionTextRectColor; Color? autocorrectionTextRectColor;
Radius? cursorRadius = widget.cursorRadius; Radius? cursorRadius = widget.cursorRadius;
VoidCallback? handleDidGainAccessibilityFocus; VoidCallback? handleDidGainAccessibilityFocus;
VoidCallback? handleDidLoseAccessibilityFocus;
switch (theme.platform) { switch (theme.platform) {
case TargetPlatform.iOS: case TargetPlatform.iOS:
...@@ -1320,6 +1321,9 @@ class _TextFieldState extends State<TextField> with RestorationMixin implements ...@@ -1320,6 +1321,9 @@ class _TextFieldState extends State<TextField> with RestorationMixin implements
_effectiveFocusNode.requestFocus(); _effectiveFocusNode.requestFocus();
} }
}; };
handleDidLoseAccessibilityFocus = () {
_effectiveFocusNode.unfocus();
};
case TargetPlatform.android: case TargetPlatform.android:
case TargetPlatform.fuchsia: case TargetPlatform.fuchsia:
...@@ -1337,6 +1341,15 @@ class _TextFieldState extends State<TextField> with RestorationMixin implements ...@@ -1337,6 +1341,15 @@ class _TextFieldState extends State<TextField> with RestorationMixin implements
cursorOpacityAnimates ??= false; cursorOpacityAnimates ??= false;
cursorColor = _hasError ? _errorColor : widget.cursorColor ?? selectionStyle.cursorColor ?? theme.colorScheme.primary; cursorColor = _hasError ? _errorColor : widget.cursorColor ?? selectionStyle.cursorColor ?? theme.colorScheme.primary;
selectionColor = selectionStyle.selectionColor ?? theme.colorScheme.primary.withOpacity(0.40); selectionColor = selectionStyle.selectionColor ?? theme.colorScheme.primary.withOpacity(0.40);
handleDidGainAccessibilityFocus = () {
// Automatically activate the TextField when it receives accessibility focus.
if (!_effectiveFocusNode.hasFocus && _effectiveFocusNode.canRequestFocus) {
_effectiveFocusNode.requestFocus();
}
};
handleDidLoseAccessibilityFocus = () {
_effectiveFocusNode.unfocus();
};
case TargetPlatform.windows: case TargetPlatform.windows:
forcePressEnabled = false; forcePressEnabled = false;
...@@ -1351,6 +1364,9 @@ class _TextFieldState extends State<TextField> with RestorationMixin implements ...@@ -1351,6 +1364,9 @@ class _TextFieldState extends State<TextField> with RestorationMixin implements
_effectiveFocusNode.requestFocus(); _effectiveFocusNode.requestFocus();
} }
}; };
handleDidLoseAccessibilityFocus = () {
_effectiveFocusNode.unfocus();
};
} }
Widget child = RepaintBoundary( Widget child = RepaintBoundary(
...@@ -1478,6 +1494,7 @@ class _TextFieldState extends State<TextField> with RestorationMixin implements ...@@ -1478,6 +1494,7 @@ class _TextFieldState extends State<TextField> with RestorationMixin implements
_requestKeyboard(); _requestKeyboard();
}, },
onDidGainAccessibilityFocus: handleDidGainAccessibilityFocus, onDidGainAccessibilityFocus: handleDidGainAccessibilityFocus,
onDidLoseAccessibilityFocus: handleDidLoseAccessibilityFocus,
child: child, child: child,
); );
}, },
......
...@@ -605,6 +605,9 @@ void main() { ...@@ -605,6 +605,9 @@ void main() {
const Widget flexibleSpace = Text('FlexibleSpace'); const Widget flexibleSpace = Text('FlexibleSpace');
TestSemantics buildExpected({ required String routeName }) { TestSemantics buildExpected({ required String routeName }) {
final bool isDesktop = debugDefaultTargetPlatformOverride == TargetPlatform.macOS ||
debugDefaultTargetPlatformOverride == TargetPlatform.windows ||
debugDefaultTargetPlatformOverride == TargetPlatform.linux;
return TestSemantics.root( return TestSemantics.root(
children: <TestSemantics>[ children: <TestSemantics>[
TestSemantics( TestSemantics(
...@@ -651,9 +654,10 @@ void main() { ...@@ -651,9 +654,10 @@ void main() {
debugDefaultTargetPlatformOverride != TargetPlatform.macOS) SemanticsFlag.namesRoute, debugDefaultTargetPlatformOverride != TargetPlatform.macOS) SemanticsFlag.namesRoute,
], ],
actions: <SemanticsAction>[ actions: <SemanticsAction>[
if (debugDefaultTargetPlatformOverride == TargetPlatform.macOS || if (isDesktop)
debugDefaultTargetPlatformOverride == TargetPlatform.windows)
SemanticsAction.didGainAccessibilityFocus, SemanticsAction.didGainAccessibilityFocus,
if (isDesktop)
SemanticsAction.didLoseAccessibilityFocus,
SemanticsAction.tap, SemanticsAction.tap,
SemanticsAction.setSelection, SemanticsAction.setSelection,
SemanticsAction.setText, SemanticsAction.setText,
...@@ -748,6 +752,9 @@ void main() { ...@@ -748,6 +752,9 @@ void main() {
group('contributes semantics', () { group('contributes semantics', () {
TestSemantics buildExpected({ required String routeName }) { TestSemantics buildExpected({ required String routeName }) {
final bool isDesktop = debugDefaultTargetPlatformOverride == TargetPlatform.macOS ||
debugDefaultTargetPlatformOverride == TargetPlatform.windows ||
debugDefaultTargetPlatformOverride == TargetPlatform.linux;
return TestSemantics.root( return TestSemantics.root(
children: <TestSemantics>[ children: <TestSemantics>[
TestSemantics( TestSemantics(
...@@ -791,9 +798,10 @@ void main() { ...@@ -791,9 +798,10 @@ void main() {
debugDefaultTargetPlatformOverride != TargetPlatform.macOS) SemanticsFlag.namesRoute, debugDefaultTargetPlatformOverride != TargetPlatform.macOS) SemanticsFlag.namesRoute,
], ],
actions: <SemanticsAction>[ actions: <SemanticsAction>[
if (debugDefaultTargetPlatformOverride == TargetPlatform.macOS || if (isDesktop)
debugDefaultTargetPlatformOverride == TargetPlatform.windows)
SemanticsAction.didGainAccessibilityFocus, SemanticsAction.didGainAccessibilityFocus,
if (isDesktop)
SemanticsAction.didLoseAccessibilityFocus,
SemanticsAction.tap, SemanticsAction.tap,
SemanticsAction.setSelection, SemanticsAction.setSelection,
SemanticsAction.setText, SemanticsAction.setText,
......
...@@ -655,7 +655,7 @@ void main() { ...@@ -655,7 +655,7 @@ void main() {
expect(editableText.cursorOpacityAnimates, false); expect(editableText.cursorOpacityAnimates, false);
}); });
testWidgets('Activates the text field when receives semantics focus on Mac, Windows', (WidgetTester tester) async { testWidgets('Activates the text field when receives semantics focus on desktops', (WidgetTester tester) async {
final SemanticsTester semantics = SemanticsTester(tester); final SemanticsTester semantics = SemanticsTester(tester);
final SemanticsOwner semanticsOwner = tester.binding.pipelineOwner.semanticsOwner!; final SemanticsOwner semanticsOwner = tester.binding.pipelineOwner.semanticsOwner!;
final FocusNode focusNode = FocusNode(); final FocusNode focusNode = FocusNode();
...@@ -686,6 +686,7 @@ void main() { ...@@ -686,6 +686,7 @@ void main() {
actions: <SemanticsAction>[ actions: <SemanticsAction>[
SemanticsAction.tap, SemanticsAction.tap,
SemanticsAction.didGainAccessibilityFocus, SemanticsAction.didGainAccessibilityFocus,
SemanticsAction.didLoseAccessibilityFocus,
], ],
textDirection: TextDirection.ltr, textDirection: TextDirection.ltr,
), ),
...@@ -705,8 +706,12 @@ void main() { ...@@ -705,8 +706,12 @@ void main() {
semanticsOwner.performAction(4, SemanticsAction.didGainAccessibilityFocus); semanticsOwner.performAction(4, SemanticsAction.didGainAccessibilityFocus);
await tester.pumpAndSettle(); await tester.pumpAndSettle();
expect(focusNode.hasFocus, isTrue); expect(focusNode.hasFocus, isTrue);
semanticsOwner.performAction(4, SemanticsAction.didLoseAccessibilityFocus);
await tester.pumpAndSettle();
expect(focusNode.hasFocus, isFalse);
semantics.dispose(); semantics.dispose();
}, variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.macOS, TargetPlatform.windows })); }, variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.macOS, TargetPlatform.windows, TargetPlatform.linux }));
testWidgets('TextField passes onEditingComplete to EditableText', (WidgetTester tester) async { testWidgets('TextField passes onEditingComplete to EditableText', (WidgetTester tester) async {
void onEditingComplete() { } void onEditingComplete() { }
......
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