Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Do not move focus when the semantics text strategy is deactivated. #54979

Closed
wants to merge 2 commits into from

Conversation

tugorez
Copy link
Contributor

@tugorez tugorez commented Sep 5, 2024

When the text field is deactivated, the non-semantics text strategy refocuses on the Flutter view. This is necessary because there's no assurance that the next interactive widget will be supported by a DOM node. If not, focus will shift outside the Flutter view. With semantics enabled, the app ensures every interactive widget has a corresponding DOM node, eliminating the need to refocus on the Flutter view. In fact, the view shouldn't be focusable with semantics enabled to avoid disrupting the interaction sequence.

This PR should be the last step before landing #54966

I compiled https://github.com/flutter/flutter/tree/master/dev/a11y_assessments with these changes. App can be found at https://tugorez.com/flutter_focus_web/

flutter/flutter#153022

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 and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

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

@github-actions github-actions bot added the platform-web Code specifically for the web engine label Sep 5, 2024
@tugorez tugorez requested review from yjbanov and mdebbar September 5, 2024 21:02
// Keep this consistent with how DefaultTextEditingStrategy does it. As of
// right now, the only difference is that semantic text fields do not
// participate in form autofill.
DefaultTextEditingStrategy.scheduleFocusFlutterView(activeDomElement, activeDomElementView);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TextEditingStrategy.disable is used by TextInput.clearClient, TextInput.hide, and in one case by TextInput.setClient. The focus move (which previously was just domElement.blur()) seems to have the intention to close the virtual keyboard when the framework demands it. If we do not do anything here, does it mean that there are situations when the framework would detach and move onto the next widget, but the browser will stay focused on the text field and virtual keyboard showing? I remember we had bugs where the virtual keyboard would show up, but typing into it would not enter any text, because the <input> is not connected to the framework.

Copy link
Contributor Author

@tugorez tugorez Sep 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a great point Yegor!

If I understand correctly, the deactivated call suggests either app transitioned "flutter" focus to another widget, shifting the "browser" focus, or the user interacted outside the app, causing it to lose focus. In both cases the focus will be moved away from the text input causing the keyboard to be hidding. Is there a use case I might be missing?

@tugorez tugorez mentioned this pull request Sep 6, 2024
8 tasks
@tugorez tugorez closed this Sep 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
platform-web Code specifically for the web engine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants