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

Fix done button click not blur in iOS keyboard #31718

Merged
merged 12 commits into from
Mar 16, 2022

Conversation

JunbinDeng
Copy link
Contributor

And besides I found the following issue is related to TextField focus:

This commit resolves iOS virtual keyboard glitch and TextField focus issue in Safari. When tapping the done button above the keyboard, TextField blurs and closes the keyboard right away instead of refocusing and reopening afterward.

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 Hixie said 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.

@flutter-dashboard flutter-dashboard bot added the platform-web Code specifically for the web engine label Feb 28, 2022
@JunbinDeng JunbinDeng marked this pull request as draft March 3, 2022 01:07
@JunbinDeng JunbinDeng marked this pull request as ready for review March 3, 2022 01:08
@justinmc justinmc requested review from justinmc and removed request for justinmc March 3, 2022 22:27
@justinmc
Copy link
Contributor

justinmc commented Mar 3, 2022

CC @mdebbar

@mdebbar mdebbar self-requested a review March 8, 2022 15:56
Copy link
Contributor

@mdebbar mdebbar left a comment

Choose a reason for hiding this comment

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

Could you elaborate a little more about how this PR fixes the clicks on "done"?

@JunbinDeng
Copy link
Contributor Author

Could you elaborate a little more about how this PR fixes the clicks on "done"?

Thanks for reviewing. In the beginning, I found that taping the 'done' button above the soft input in iOS Safari was not working correctly as the keyboard closed and opened up again.

When pressing the 'done' button, the event listener 'onblur' should trigger a callback then the keyboard should be closed normally. The reason why the keyboard kept opening up was that the window was still on focus.

I was trying to remove everything except 'owner.sendTextConnectionClosedToFrameworkIfAny' in the 'onblur' listener callback. But a new problem occurred, which was tapping the TextInput could not bring up the keyboard occasionally.

I figured out the 'onblur' listener callback was triggered, as long as the listener had set up after a while. It should call by mistake.

Having tested several times, I found that the intervals between the 'onblur' event listener registered and callback triggered were below 200ms.

It was considered a fast callback, which should regain focus instead of blur.

Please let me know if there's any other way I can do better.

@mdebbar
Copy link
Contributor

mdebbar commented Mar 10, 2022

Thanks for the explanation!

I'm still trying to understand the "fast callback". Here's my understanding of the sequence of things:

The onblur event listener is registered when the text field is created (which happens when you tap on it for the first time). And the onblur event is triggered when you click the "done" button.

Why does the duration between these two things matter? And in what situation does the onblur trigger immediately after subscription?

@JunbinDeng
Copy link
Contributor Author

JunbinDeng commented Mar 11, 2022

Thanks for the explanation!

I'm still trying to understand the "fast callback". Here's my understanding of the sequence of things:

The onblur event listener is registered when the text field is created (which happens when you tap on it for the first time). And the onblur event is triggered when you click the "done" button.

Why does the duration between these two things matter? And in what situation does the onblur trigger immediately after subscription?

It could be more clear to deduce matters in this way.

The current situation:

  1. Tap the TextField
  2. Set up the blur listener
  3. Trigger the blur listener callback immediately (should not happen)
  4. Yes, the window has the focus
  5. Keep showing the keyboard

The current keyboard action:

  1. Press the 'done' button
  2. The keyboard is hiding
  3. Trigger the blur listener callback
  4. Yes, the window has the focus
  5. The keyboard shows up (unable to hide)

If we apply the 'fast callback' solution, then:

  1. Tap the TextField
  2. Set up the blur listener
  3. Trigger the blur listener callback immediately (should not happen)
  4. Yes, a 'fast callback'
  5. Keep showing the keyboard

The solved keyboard action:

  1. Press the 'done' button
  2. The keyboard is hiding
  3. Trigger the blur listener callback
  4. No, not a 'fast callback'
  5. The keyboard hides

Hopefully can answer your questions.

Copy link
Contributor

@mdebbar mdebbar left a comment

Choose a reason for hiding this comment

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

Thanks for the great explanation! I added a suggestion on how to incorporate it in the doc comments. Feel free to re-phrase it if needed.

@JunbinDeng
Copy link
Contributor Author

Thanks for the great explanation! I added a suggestion on how to incorporate it in the doc comments. Feel free to re-phrase it if needed.

I appreciate the time and effort you have spent to share your insightful comments, which will be helpful to improve code readability.

@mdebbar mdebbar added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Mar 15, 2022
@mdebbar
Copy link
Contributor

mdebbar commented Mar 15, 2022

Thanks for contributing this fix to the flutter web engine!

@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • Please get at least one approved review if you are already a member or two member reviews if you are not a member before re-applying this label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

@fluttergithubbot fluttergithubbot removed the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Mar 15, 2022
Copy link
Contributor

@yjbanov yjbanov left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Just some small nits for my opinion on making the docs more readable, but it's up to you.

@JunbinDeng
Copy link
Contributor Author

LGTM 👍

Just some small nits for my opinion on making the docs more readable, but it's up to you.

Thanks for your suggestion. I've done it.

@mdebbar mdebbar added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Mar 16, 2022
@fluttergithubbot fluttergithubbot merged commit a00ba24 into flutter:main Mar 16, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 17, 2022
zanderso pushed a commit to flutter/flutter that referenced this pull request Mar 17, 2022
* 09d7bcc Optionally specify the target dir in tools/gn (flutter/engine#32065)

* a00ba24 Fix done button click not blur in iOS keyboard (flutter/engine#31718)

* 81547d1 Add a display list op to clear to transformation stack. (flutter/engine#32050)

* 2309bcc Add WASM target in gn (flutter/engine#31670)

* 852e800 [web] Remove the --passfail flag when calling goldctl in post-submit (flutter/engine#32071)

* eb1c50d Fix issues with nested gradients in html renderer. (flutter/engine#31887)

* fb0fd74 Update the magic number for JPEG to just FF D8 FF. (flutter/engine#32076)

* 233c17c Wrap the global timeline event handler callback in a std::atomic (flutter/engine#32073)

* dfde2aa Roll Dart SDK from 24bf86f16411 to 5bc905e69609 (9 revisions) (flutter/engine#32075)
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 waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants