Fix LinkPicker freeze when virtual keyboard is hidden #37782
Merged
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
When a devices virtual keyboard is hidden, e.g. when a hardware keyboard is connected, dismissing the `LinkPicker` resulted in the application freezing. The freeze likely originates from an unconsumed `LayoutAnimation` registered during resizing of the `BottomSheet`. To address this issue, we now avoid registering a `LayoutAnimation` whenever the changes to the header are sub-pixel values, which can result in the `LayoutAnimation` going unconsumed. https://git.io/J9q2G Long-term, we should likely consider refactoring the `BottomSheet` to holistically avoid stacking `LayoutAnimations`: https://git.io/J9q2l
dcalhoun
added
Mobile App - i.e. Android or iOS
2 tasks
|
Size Change: 0 B Total Size: 1.13 MB ℹ️ View Unchanged
|
fluiddot
reviewed
Jan 7, 2022
This allows individual tests to pass a unique, top-level testID for the BottomSheet and have it propagate to the header as well, which may make querying easier. Co-authored-by: Carlos Garcia <fluiddot@gmail.com>
fluiddot
approved these changes
Jan 7, 2022
There was a problem hiding this comment.
LGTM 🎊 !
Thanks @dcalhoun for addressing the issue and introducing a test for the bottom sheet component 🙇 !
I tested the following cases which all of them passed:
- Use an external keyboard on Android device ✅
- Disable software keyboard on iOS simulator ✅
- Use an external keyboard on iOS device ✅
And verified that the unit tests passed too ✅ .
|
@dcalhoun what do you think about creating a GB-mobile PR to include the fix in Gutenberg Mobile? |
|
Additionally, we might consider adding an entry with this fix in the |
@fluiddot yep, wordpress-mobile/gutenberg-mobile#4443 already exists. 👍🏻
Yes, that makes sense. 👍🏻 Even though this particular bug will never land in a public-facing release of the WordPress apps, I suppose it is worth noting it does technically exist in the |
dcalhoun
deleted the
bug/fix-link-picker-freeze-when-virtual-keyboard-is-hidden
branch
mkevins
pushed a commit
that referenced
this pull request
Jan 17, 2022
* Fix LinkPicker freeze when virtual keyboard is hidden When a devices virtual keyboard is hidden, e.g. when a hardware keyboard is connected, dismissing the `LinkPicker` resulted in the application freezing. The freeze likely originates from an unconsumed `LayoutAnimation` registered during resizing of the `BottomSheet`. To address this issue, we now avoid registering a `LayoutAnimation` whenever the changes to the header are sub-pixel values, which can result in the `LayoutAnimation` going unconsumed. https://git.io/J9q2G Long-term, we should likely consider refactoring the `BottomSheet` to holistically avoid stacking `LayoutAnimations`: https://git.io/J9q2l * Support unique BottomSheet header testID This allows individual tests to pass a unique, top-level testID for the BottomSheet and have it propagate to the header as well, which may make querying easier. Co-authored-by: Carlos Garcia <fluiddot@gmail.com> * Fix indentation issue in bottom sheet component * Add change log entry * Add pull request reference to change log entry Co-authored-by: Carlos Garcia <fluiddot@gmail.com>
mkevins
added a commit
that referenced
this pull request
Jan 19, 2022
* Release script: Update react-native-editor version to 1.69.0 * Release script: Update with changes from 'npm run core preios' * Update native editor changelog for v1.69.0 * Release script: Update react-native-editor version to 1.69.1 * Release script: Update with changes from 'npm run core preios' * Mobile - Gallery block - Fix issue migrating old format and missing fixedHeight context, use of images prop instead of the attributes (#37889) * [Mobile] - RichText - Use parsed font size values when comparing new changes (#37951) * Mobile - RichText - Use parsed font size values when comparing new changes to avoid not matching values that generate a render loop * Mobile - E2E - Move typography test to full tests instead of canary * Mobile - RichText - Avoid using the fontSize value from the props if there's a font size set from the styles * Mobile - E2E - Remove test and move some test cases into the current integration tests * Mobile - RichTest - Update tests * Fix LinkPicker freeze when virtual keyboard is hidden (#37782) * Fix LinkPicker freeze when virtual keyboard is hidden When a devices virtual keyboard is hidden, e.g. when a hardware keyboard is connected, dismissing the `LinkPicker` resulted in the application freezing. The freeze likely originates from an unconsumed `LayoutAnimation` registered during resizing of the `BottomSheet`. To address this issue, we now avoid registering a `LayoutAnimation` whenever the changes to the header are sub-pixel values, which can result in the `LayoutAnimation` going unconsumed. https://git.io/J9q2G Long-term, we should likely consider refactoring the `BottomSheet` to holistically avoid stacking `LayoutAnimations`: https://git.io/J9q2l * Support unique BottomSheet header testID This allows individual tests to pass a unique, top-level testID for the BottomSheet and have it propagate to the header as well, which may make querying easier. Co-authored-by: Carlos Garcia <fluiddot@gmail.com> * Fix indentation issue in bottom sheet component * Add change log entry * Add pull request reference to change log entry Co-authored-by: Carlos Garcia <fluiddot@gmail.com> * Update release notes * Add back missing dep in package-lock Co-authored-by: Gerardo Pacheco <gerardo.pacheco@automattic.com> Co-authored-by: David Calhoun <438664+dcalhoun@users.noreply.github.com> Co-authored-by: Carlos Garcia <fluiddot@gmail.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.
Description
Fixes #37773.
When a devices virtual keyboard is hidden, e.g. when a hardware keyboard is connected, dismissing the
LinkPickerresulted in the application freezing. The freeze likely originates from an unconsumedLayoutAnimationregistered during resizing of theBottomSheet.To address this issue, we now avoid registering a
LayoutAnimationwhenever the changes to the header are sub-pixel values, which can result in theLayoutAnimationgoing unconsumed: https://git.io/J9q2G Long-term, we should likely consider refactoring theBottomSheetto holistically avoid stackingLayoutAnimations: https://git.io/J9q2lHow has this been tested?
*These specific context were not tested due to lack of necessary hardware.
Screenshots
n/a
Types of changes
Bug fix
Checklist:
*.native.jsfiles for terms that need renaming or removal).