[Mobile] - Android - Bring the Keyboard back when closing modals #57069
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
geriux
added
[Status] In Progress
|
Size Change: 0 B Total Size: 1.71 MB ℹ️ View Unchanged
|
|
Flaky tests detected in ef48c7f. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7213191517
|
geriux
force-pushed
the
feat/android-restore-keyboard-when-closing-modals
branch
from
acf6641
to
b2d6ead
Compare
added 4 commits
December 14, 2023 19:40
… show the keyboard if there's a focused TextInput
…osing the Modal so it shows the Keyboard on Android for focused TextInputs
… hide the keyboard without triggering blur events
…, as their names are self-explanatory.
geriux
force-pushed
the
feat/android-restore-keyboard-when-closing-modals
branch
from
5bd2dcc
to
ef48c7f
Compare
geriux
changed the base branch from
trunk
to
rnmobile/fix/avoid-keyboard-dismiss-when-interacting-text-blocks
fluiddot
approved these changes
Dec 15, 2023
...java/org/wordpress/mobile/ReactNativeGutenbergBridge/RNReactNativeGutenbergBridgeModule.java
Outdated
Show resolved
Hide resolved
added 2 commits
December 15, 2023 16:11
…ocus 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
…ForBackdrop props to improve performance on Android
Thank you for the review and for testing these changes! I introduced a few changes related to the keyboard logic and I also added two props for the Modal component that I think it'd improve the Android performance a bit. Let me know if you have any questions about it. Thanks! |
fluiddot
reviewed
Dec 15, 2023
...java/org/wordpress/mobile/ReactNativeGutenbergBridge/RNReactNativeGutenbergBridgeModule.java
Outdated
Show resolved
Hide resolved
...java/org/wordpress/mobile/ReactNativeGutenbergBridge/RNReactNativeGutenbergBridgeModule.java
Outdated
Show resolved
Hide resolved
...java/org/wordpress/mobile/ReactNativeGutenbergBridge/RNReactNativeGutenbergBridgeModule.java
Outdated
Show resolved
Hide resolved
fluiddot
approved these changes
Dec 15, 2023
There was a problem hiding this comment.
LGTM 🎊 ! I've tested the latest changes and worked as expected, thanks @geriux for addressing my suggestions 🙇 !
I added some minor comments but we could skip them to merge the PR. Alternatively, since the PR will be merged into a feature branch, we could address some of them directly there.
I introduced a few changes related to the keyboard logic and I also added two props for the Modal component that I think it'd improve the Android performance a bit. Let me know if you have any questions about it. Thanks!
That's great, do you foresee we should check modals in other scenarios aside the keyboard issue due to this change?
I've tested in an actual device with a local build and can't really tell if the performance improved when showing/hiding modals. In any case, I presume that this will be more visible when testing installable builds.
...java/org/wordpress/mobile/ReactNativeGutenbergBridge/RNReactNativeGutenbergBridgeModule.java
Outdated
Show resolved
Hide resolved
added 2 commits
December 18, 2023 12:37
…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.
fluiddot
reviewed
Dec 18, 2023
...java/org/wordpress/mobile/ReactNativeGutenbergBridge/RNReactNativeGutenbergBridgeModule.java
Outdated
Show resolved
Hide resolved
…nable instead of getting it as an param
fluiddot
reviewed
Dec 18, 2023
...java/org/wordpress/mobile/ReactNativeGutenbergBridge/RNReactNativeGutenbergBridgeModule.java
Outdated
Show resolved
Hide resolved
|
@fluiddot Thank you for the review and feedback! I'll merge this PR once the checks pass 🚀 |
geriux
merged commit 71b3409
into
rnmobile/fix/avoid-keyboard-dismiss-when-interacting-text-blocks
52 checks passed
fluiddot
added a commit
that referenced
this pull request
Dec 18, 2023
* 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 <gerardo.pacheco@automattic.com>
This was referenced Dec 19, 2023
Text stops being selected after opening highlight color and link bottom sheets (Only Android)
#47564
Closed
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 <gerardo.pacheco@automattic.com>
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.
This PR is part of #57070
What?
It adds new implementations in the bridge for Android, so we can control hiding and showing the keyboard in some cases.
Why?
To improve the writing flow experience, we'll handle the keyboard manually in some cases.
How?
It adds two methods
showAndroidSoftKeyboardandhideAndroidSoftKeyboardin the React Native Bridge package, this functionality will be shared for both the demo app and the host apps as the functionality is kept entirely inRNReactNativeGutenbergBridgeModule.The
showAndroidSoftKeyboardmethod is introduced in theBottomSheetcomponent, to prevent not displaying the Keyboard when these modals are dismissed and a TextInput was focused prior to opening the Modal.This is only needed in Android since iOS handles this internally.
It's used in three parts of the code:
componentWillUnmountsome Modals get removed completely when they're hidden, which prevents other callbacks to be triggered likeonModalHide.onHardwareButtonPresswhen a user dismisses the modal by using the hardware back button on Android.onCloseBottomSheettriggered when the modal is hidden either by swiping down, or tapping on the Modal backdrop.Testing Instructions
Case 1 - Opening the block settings of a Paragraph block
Case 2 - Changing the text color of a Paragraph block
Case 3 - Changing the text alignment
Case 4 - Opening the block inserter when focused on a Paragraph block
Testing Instructions for Keyboard
N/A
Screenshots or screencast
Case1_Keyboard.mov
Case2_Keyboard.mov
Case3_Keyboard.mov
Case4_Keyboard.mov