Request focus if SemanticsAction.focus is sent to a focusable widget
#142942
Conversation
|
This change surprised me. At least on Android (and I think on iOS as well) a11y focus and input focus are two separate concepts and you can totally move a11y focus away from e.g. a textfield while the textfield retains input focus. |
This is a good point. This behavior should be limited to desktop form factor. It is true that on mobile accessibility focus does not automatically grab the input focus. On desktop (at least I tried on macOS, and desktop web) it does. |
How do on screen IMEs work on Desktop in a11y mode if moving the a11y focus to a key on the onscreen keyboard removes the input focus from the textfield you want to type into? |
|
Haven't tried IMEs specifically, but moving accessibility into a different text fields does stop editing the previous field and starts editing the newly focused field. |
|
Is there an actual user of this UI that we can ask about this behavior? It does seem like we're making a change to a UX experience that we don't really use ourselves. I'd hate to mess with the accessibility system's usability without checking in with at least one actual user. |
8bd671c to
7f94913
Compare
|
Okay, I think this is updated to match Yegor's change. I added in some missing matchers for the semantics testing too. |
There was a problem hiding this comment.
Should this be _couldRequestFocus ? focusNode.requestFocus : null? It's strange that the condition that controls the availability of the onFocus callback is not exactly the same as the one used to control the focusable property below. If the two disagree, we could end up in a situation where the semantics thinks a widget is focusable while the framework does not, or vice versa.
If it is important to use separate conditions, it would also be good to have a test that covers it, and I may have to change my engine PR to do the right thing.
There was a problem hiding this comment.
Yes, that's much clearer. In reality, it makes no difference, since _couldRequestFocus is just equal to focusNode.canRequestFocus, and the requestFocus method exits early if the node already has focus.
There was a problem hiding this comment.
The comment still refers to "accessibility focus". Since we switched from didGainAccessibilityFocus to focus, it should say just "focus", or to be more precise "input focus request from semantics".
There was a problem hiding this comment.
Good point, fixed.
|
Perhaps this solved the problem with the accessibility of the edit fields? |
18e0c7a to
3c36672
Compare
Unlikely, since this PR hasn't landed yet. But if it's fixed, we'll try not to break it with this PR. |
3c36672 to
d6d2625
Compare
|
auto label is removed for flutter/flutter/142942, due to - The status or check suite Mac framework_tests_impeller has failed. Please fix the issues identified (or deflake) before re-applying this label. |
…ble widget (#142942)" (#149741) Reverts: #142942 Initiated by: zanderso Reason for reverting: Seems to have affected iOS platform view focus: https://ci.chromium.org/ui/p/flutter/builders/prod/Mac_ios%20native_platform_view_ui_tests_ios/10626/overview Original PR Author: gspencergoog Reviewed By: {yjbanov, goderbauer, chunhtai} This change reverts the following previous change: ## Description This causes the `Focus` widget to request focus on its focus node if the accessibility system (screen reader) focuses a widget via the `SemanticsAction.focus` action. ## Related Issues - #83809 ## Tests - Added a test to make sure that focus is requested when `SemanticsAction.focus` is sent by the engine.
flutter/flutter@c246ecd...27e0656 2024-06-05 98614782+auto-submit[bot]@users.noreply.github.com Reverts "TreeSliver & associated classes (#147171)" (flutter/flutter#149754) 2024-06-05 p.trost93@gmail.com Feature: Add AnimatedList with separators (flutter/flutter#144899) 2024-06-05 yjbanov@google.com make output of flutter run web tests verbose (flutter/flutter#149694) 2024-06-05 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Request focus if `SemanticsAction.focus` is sent to a focusable widget (#142942)" (flutter/flutter#149741) 2024-06-05 engine-flutter-autoroll@skia.org Roll Flutter Engine from e88469090fed to 11a32d43e3f6 (2 revisions) (flutter/flutter#149699) 2024-06-05 gspencergoog@users.noreply.github.com Request focus if `SemanticsAction.focus` is sent to a focusable widget (flutter/flutter#142942) 2024-06-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from 3dd40156afb6 to e88469090fed (2 revisions) (flutter/flutter#149695) 2024-06-04 katelovett@google.com TreeSliver & associated classes (flutter/flutter#147171) 2024-06-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from fe00f023666c to 3dd40156afb6 (3 revisions) (flutter/flutter#149692) 2024-06-04 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.25.7 to 3.25.8 (flutter/flutter#149691) 2024-06-04 47866232+chunhtai@users.noreply.github.com Prepares semantics_update_test for upcoming heading level changes (flutter/flutter#149671) 2024-06-04 34871572+gmackall@users.noreply.github.com Identify and re-throw our dependency checking errors in `flutter.groovy` (flutter/flutter#149609) 2024-06-04 hans.muller@gmail.com Scrollbar thumb drag gestures now produce one start and one end scroll notification (flutter/flutter#146654) 2024-06-04 15619084+vashworth@users.noreply.github.com Disable sandboxing for macOS apps and tests in CI (flutter/flutter#149618) 2024-06-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from e211c43f3dc1 to fe00f023666c (3 revisions) (flutter/flutter#149680) 2024-06-04 gspencergoog@users.noreply.github.com Allow `find.byTooltip` to use a RegEx (flutter/flutter#149348) 2024-06-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from a6aa5d826649 to e211c43f3dc1 (8 revisions) (flutter/flutter#149658) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC dit@google.com,rmistry@google.com,stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Reland: Request focus if SemanticsAction.focus is sent to a focusable widget (flutter#142942)
Reland: Request focus if SemanticsAction.focus is sent to a focusable widget (flutter#142942)
…et (#142942) (#149840) ## Description This attempts to re-land #142942 after being reverted in #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 - #149838 - #83809 - #149842 ## Tests - Updated framework tests to look for the appropriate things using the matchers, even though it doesn't actually test for them yet.
…et (flutter#142942) (flutter#149840) ## Description This attempts to re-land flutter#142942 after being reverted in 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#149838 - flutter#83809 - 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.
…et (flutter#142942) (flutter#149840) ## Description This attempts to re-land flutter#142942 after being reverted in 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#149838 - flutter#83809 - 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.
Adds `onFocus` support to Cupertino and Material text field widgets (similar to #142942).
Adds `onFocus` support to Cupertino and Material text field widgets (similar to flutter#142942).
Description
This causes the
Focuswidget to request focus on its focus node if the accessibility system (screen reader) focuses a widget via theSemanticsAction.focusaction.Related Issues
Tests
SemanticsAction.focusis sent by the engine.