[RNMobile] Fix a crash related to Reanimated when closing the editor #52320
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
Seems there's some kind of incompatibility on calling a JS function from a worklet invoked from a gesture handler. For this reason, the logic to set the dropping insertion point has been updated. It now uses a Reanimated's shared value to keep the dragging over position and `useDerivedValue` hook to listen for changes.
fluiddot
added
[Type] Bug
|
Size Change: 0 B Total Size: 1.42 MB ℹ️ View Unchanged
|
|
Flaky tests detected in 7ce0719. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5462626319
|
dcalhoun
reviewed
Jul 5, 2023
packages/block-editor/src/components/use-block-drop-zone/index.native.js
Show resolved
Hide resolved
packages/block-editor/src/components/use-block-drop-zone/index.native.js
Show resolved
Hide resolved
packages/block-editor/src/components/use-block-drop-zone/index.native.js
Show resolved
Hide resolved
packages/block-editor/src/components/use-block-drop-zone/index.native.js
Show resolved
Hide resolved
dcalhoun
approved these changes
Jul 5, 2023
There was a problem hiding this comment.
Approving based on #52320 (review), but feel free to seek additional review before merging if desired.
Thank you @dcalhoun for reviewing the PR 🙇 , I appreciate it. I'm ok with one approval, although it would be great if @geriux could take a quick look since was originally involved in these changes, at least to confirm that this approach makes sense. Thanks! |
|
@dcalhoun let me know if you could also take a look at the companion Gutenberg Mobile PR. It just updates the Gutenberg reference. Thanks 🙇 ! |
geriux
reviewed
Jul 6, 2023
Comment on lines
+173
to
+174
| Math.abs( x - prevX ) >= UPDATE_TARGET_BLOCK_INDEX_THRESHOLD || | ||
| Math.abs( y - prevY ) >= UPDATE_TARGET_BLOCK_INDEX_THRESHOLD |
geriux
approved these changes
Jul 6, 2023
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 PR: wordpress-mobile/gutenberg-mobile#5938
What?
When working on the RN upgrade 0.71.11, we spotted a very similar crash to this one. Now that it has been fixed in the RN upgrade, we could also incorporate it into
trunkin case it solves other crashes related to Reanimated before the upgrade lands.Why?
This PR aims to fix several crashes we identified in the app, but that we couldn't reproduce back then:
How?
Seems there's some kind of incompatibility on calling a JS function from a Reanimated's worklet (reference) invoked from a gesture handler (reference). For this reason, the logic to set the dropping insertion point has been updated. It now uses a Reanimated's shared value to keep the dragging over position and
useDerivedValuehook to listen for changes.More info about the fix can be found here.
Testing Instructions
As we couldn't find a consistent way to reproduce the Reanimated crashes, we won't be able to test that this PR fixes them. However, we can ensure that the drag & drop functionality works.
8. Observe that when dropping the block, it's added to the same position as indicated by the insertion indicator.Testing Instructions for Keyboard
N/A
Screenshots or screencast
N/A