[RNMobile] Avoid keyboard dismiss when interacting text blocks #57070
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fluiddot
added
Mobile App - i.e. Android or iOS
|
Size Change: -762 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 17dcda5. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7221803754
|
This function will help us to deal with the special case of unfocusing an Aztec view upon unmounting.
This was previously handled in the `RichText` component.
geriux
approved these changes
Dec 18, 2023
There was a problem hiding this comment.
LGTM! This is a major improvement to the writing flow experience 🤩
The code changes look good to me, I'm happy that we were able to achieve this with not that many code changes or refactors 🚀
I've tested all test cases scenarios including the writing flow without any issues or regressions.
Devices used:
- iPhone 15 Pro (OS 17.1.2)
- Samsung Galaxy A23 5G (OS Android 13)
- Redmi Note 8T (OS Android 11)
Really nice work! 👏
I think to simplify things we could merge #57069 into this branch and then merge this one into |
) * React Native Bridge - Android - Introduces showAndroidSoftKeyboard to show the keyboard if there's a focused TextInput * Mobile - Bottom Sheet - Adds usage of showAndroidSoftKeyboard when closing the Modal so it shows the Keyboard on Android for focused TextInputs * React Native Bridge - Android - Introduces hideAndroidSoftKeyboard to hide the keyboard without triggering blur events * React Native Bridge - Remove console warnings for unsupported methods, as their names are self-explanatory. * Update showAndroidSoftKeyboard to take into account when the window focus changed, when we show the Modals these are shown on top of the editor activity. It also adds an option to delay this for full screen modals * Mobile - BottomSheet - Enable hardwareAccelerated and useNativeDriverForBackdrop props to improve performance on Android * Update snapshots * Removes hasWindowFocus condition as it is not being called hence not needed * Refactor showAndroidSoftKeyboard to split into several functions, it also removes the delay functionality as it is no longer needed. It fixes an issue where mKeyboardRunnable was not being set. It removes the delay logic from the Bottom Sheet component and the bridge. * Updates createShowKeyboardRunnable to get the activity within the runnable instead of getting it as an param * Remove unneeded check
|
The changes from #57069 are now merged into this branch 🎉 |
|
Before heading to merge the PR, I'll update the changelog to reflect the fixes. |
fluiddot
deleted the
rnmobile/fix/avoid-keyboard-dismiss-when-interacting-text-blocks
branch
|
Hi @fluiddot, I ran into a crash on Android from these changes. Screen: screen-20231226-171440.mp4I get the error from anywhere on the post with or without any existing content. I'm running the app locally on a Pixel 8 with WP-Android checked out at wordpress-mobile/WordPress-Android@70eb2d9 . I don't get the error after reverting the PR merge commit. |
|
Hey @jhnstn 👋 I'll investigate this issue, thanks for reporting! |
|
@jhnstn This issue is happening because we haven't released an alpha version of Gutenberg Mobile and these changes include native code. If you build Gutenberg Mobile from source it should fix it, you can do this by changing your local Let us know if this fixes the problem until we create an alpha tag. |
|
Thanks @geriux ! setting up the local GB path fixed it |
4 tasks
artemiomorales
pushed a commit
that referenced
this pull request
Jan 4, 2024
* Use debounce in Aztec's blur function * Execute `focus` UI block before other blocks * Add `hideAndroidSoftKeyboard` to RN bridge * Add `blurOnUnmount` to Aztec input state manager. This function will help us to deal with the special case of unfocusing an Aztec view upon unmounting. * Dismiss keyboard when Aztec view unmounts This was previously handled in the `RichText` component. * Fix unit test related to `AztecInputState` after adding debounce to `blur` function * Remove console warning from `hideAndroidSoftKeyboard` * Update inline comments of `blurOnUnmount` function * [Mobile] - Android - Bring the Keyboard back when closing modals (#57069) * React Native Bridge - Android - Introduces showAndroidSoftKeyboard to show the keyboard if there's a focused TextInput * Mobile - Bottom Sheet - Adds usage of showAndroidSoftKeyboard when closing the Modal so it shows the Keyboard on Android for focused TextInputs * React Native Bridge - Android - Introduces hideAndroidSoftKeyboard to hide the keyboard without triggering blur events * React Native Bridge - Remove console warnings for unsupported methods, as their names are self-explanatory. * Update showAndroidSoftKeyboard to take into account when the window focus changed, when we show the Modals these are shown on top of the editor activity. It also adds an option to delay this for full screen modals * Mobile - BottomSheet - Enable hardwareAccelerated and useNativeDriverForBackdrop props to improve performance on Android * Update snapshots * Removes hasWindowFocus condition as it is not being called hence not needed * Refactor showAndroidSoftKeyboard to split into several functions, it also removes the delay functionality as it is no longer needed. It fixes an issue where mKeyboardRunnable was not being set. It removes the delay logic from the Bottom Sheet component and the bridge. * Updates createShowKeyboardRunnable to get the activity within the runnable instead of getting it as an param * Remove unneeded check * Update `react-native-editor` changelog --------- Co-authored-by: Gerardo Pacheco <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related PRs:
What?
This PR solves the following cases that are affected by the keyboard’s disruptive hide-and-show behavior:
enterkey in a text-based block.backspacekey in a text-based block.However, there's a remaining case that this PR won't cover:
We haven't found a solution to this yet, as a next step, we'd need to explore further the actions performed during block creation and merging in the List block.
Why?
This PR will greatly improve the usability when writing content, as the hide-and-show behavior impacts negatively the editing experience.
How?
Solve
blur/focusevent conflictsEach
RichtTextcomponent instance triggersblur/focusevents based on its state (e.g. will be unmounted, gains/loses selection) without considering other instances states. This leads to conflicts betweenblurandfocusevents in different cases like pressingenteron a Paragraph block to insert a new paragraph.Here is an example of the sequence of events generated when pressing
enter:blurevent. This comes from thecomponentDidUpdatefunction due to changing its selection state from selected to unselected.bluris the culprit of the keyboard being dismissed, as it executed first.focusevent when it's mounted (i.e. from thecomponentDidMountfunction).focusevent makes the keyboard to open.focusa second time. However, this event is innocuous. This comes from thecomponentDidUpdatefunction due to changing its selection state from unselected to selected.blurevent. This comes from the_onBlurcallback triggered by Aztec because the text input lost focus. This event is innocuous.To solve this conflict, this PR introduces the usage of
debouncein theblurevent (b6f237f). It will ensure thatfocusevents take precedence whenblurevents have been triggered at the same time, as well as cancel them to avoid the keyboard to be dismissed.iOS - Keyboard dismisses automatically when text input is removed
Another case to solve is when two blocks are merged. The editor internally replaces the blocks which implies removing one of them. On iOS, this presents a problem because the keyboard will be automatically dismissed. No
blurevents are generated so a different approach was taken.The solution (a71b1c5) implied tweaking the order of native commands invoked for
blurandfocusevents (reference). Thefocusevent triggered by the new block created (i.e. the one that contains the merged content) happens at the same time the other block is removed. So, the workaround for this case is to ensure thatfocusevents are prepended in the pending UI blocks list of theUIManager(reference 1 , reference 2).Android - Avoid triggering
blurevents on unmounted instancesAs mentioned below, when
RichText/AztecViewis unmounted ablurevent is triggered. On Android, this behavior in combination with usingdebounceleads to an exception because we try toblura text input that no longer exists. To address this, a new function has been introduced in the Aztec input state (blurOnUnmount) that is only called for the component unmounting case (634867f). This function has two purposes:blurcalls to avoid exceptions.blurevent. Note that the call to hide the keyboard usesdebounce. This is needed to avoid dismissing the keyboard if afocusevent happens at the same time.Other changes
As a side note, the
blurevent triggered when unmounting theRichTextcomponent has been moved to theAztecViewcomponent as seems a better fit for this logic (d3fb486).Testing Instructions
Note
It's recommended to use the installable builds for testing:
1 - Post title
2 - Insert a new block by pressing the
enterkey in a text-based block3 - Merge blocks by pressing the
backspacekey in a text-based block (iOS only)4 - Automatically dismiss the keyboard when selecting non-text-based-blocks
5 - Manually dismiss the keyboard
6 - The keyboard is dismissed and restored when opening modals
NOTE: On Android, this case will be covered by #57069.
6 - Writing flow
It's recommended to cover the writing flow test suite by manually testing.
Testing Instructions for Keyboard
N/A
Screenshots or screencast
ios-avoid-keyboard-dismiss-when-interacting-text-blocks.MP4
android-avoid-keyboard-dismiss-when-interacting-text-blocks.mp4