[Mobile] Unselect blocks using the hardware back button (Android) #57279
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
added 4 commits
December 20, 2023 18:53
… hardware back button
geriux
added
[Type] Task
|
Size Change: 0 B Total Size: 1.69 MB ℹ️ View Unchanged
|
|
Flaky tests detected in ce40897. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7299706667
|
12 tasks
geriux
removed
[Status] In Progress
fluiddot
reviewed
Dec 22, 2023
There was a problem hiding this comment.
I've gone through the testing instructions and worked as expected. However, I noticed the following minor issues. The first would be the only one that we might consider fixing before merging. The other two could be addressed (if needed) separately.
Back button doesn't navigate back from Post settings screen
Seems an issue related to this PR as I can't reproduce it in version 23.9-rc-2.
android-back-from-post-settings.mp4
Keyboard opens after navigating back from Help & Support
This is unrelated to this PR as I've been able to reproduce in trunk. My gut feeling is that it might be a regression introduced in #57069. I'll open a GitHub issue as a follow-up.
android-back-from-help-and-support.mp4
Selecting and unselecting inner blocks
The current behavior looks good to me, but I wanted to share that my first impression was that navigating back would select the parent instead of unselecting the block. Absolutely not a blocker but I wonder what behavior would users expect 🤔.
added 4 commits
December 22, 2023 13:11
…e's another activity instanced and the editor is in the background
…null when hiding the soft keyboard. It also adds a check to not attempt to show the soft keyboard if there's no element set in currentFocusedElement
Thank you for testing the build!
Nice catch! I've updated the code to track when it should intercept the back button and when it shouldn't, for this case, when the activity of the editor isn't visible it shouldn't intercept it. Addressed in ce40897 and wordpress-mobile/WordPress-Android@0960294 post-settings-screen.mov
Sorry for introducing this bug 😓 I have a potential fix in 194c1f7 let me know what you think. help-and-support.mov
I'm hesitant about this, in cases where there are a few nesting levels, I worry that users might think they are stuck and unable to exit the editor after several presses of the back button. 😅 What do you think about shipping it as is and seeing if we receive any feedback about it? Thanks again for testing! |
@geriux Sure, my comment was merely an observation and shouldn't block at all the PR. I'll review the PR and once approved, we could merge it as is. |
fluiddot
approved these changes
Jan 2, 2024
There was a problem hiding this comment.
LGTM 🎊 !
I checked the issues mentioned in #57279 (review) and confirmed that it's working as expected.
- Back button doesn't navigate back from Post settings screen ✅
- Keyboard opens after navigating back from Help & Support ✅
Comment on lines
+801
to
+807
| mShouldHandleBackPress = false; | ||
| mReactInstanceManager.onHostDestroy(activity); | ||
| mRnReactNativeGutenbergBridgePackage.getRNReactNativeGutenbergBridgeModule().notifyModalClosed(); | ||
| } | ||
|
|
||
| public void onDestroy(Activity activity) { | ||
| mShouldHandleBackPress = false; |
There was a problem hiding this comment.
NOTE: I debugged the app and seems onPause is called before detaching and destroying the activity, hence mShouldHandleBackPress would be always false at this point and we won't need to set it to false again. However, in case there might be other scenarios where onPause wouldn't be invoked, let's keep it as is.
…'s a focused element
# Conflicts: # packages/react-native-editor/CHANGELOG.md
…its state (#57486) * Ensure `blurOnUnmount` updates internal input state * Add unit test to cover `blurOnUnmount` logic
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 adds a new functionality on Android to allow unselecting blocks by using the hardware back button of the device.
Why?
To prevent users from accidentally closing the editor when trying to unselect a block.
How?
A few native changes were needed to make this implementation work on the host Android app:
A new event listener was added to be able to communicate to the main app when it should trigger the default back button behavior instead of the implementations added within the React Native app. This listener is triggered within
invokeDefaultOnBackPressedwhich will bubble up the event to theEditPostActivity.javafile.I used this React Native guide as a reference on how to implement the back button code.
On the React side, I added a BackHandler listener that will intercept when the back button is pressed and it will decide if it should take over the event or if it should skip it and trigger
invokeDefaultOnBackPressed.Testing Instructions
Note
Use the build from this PR to go through the testing cases.
Unselecting a block
Closing a modal
Selecting and unselecting inner blocks
Testing Instructions for Keyboard
N/A
Screenshots or screencast
BackButton_TestCase1.mov
BackButton_TestCase2.mov
BackButton_TestCase3.mov