-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RNMobile] Prevent JavascriptException on scrollToSection
#59110
[RNMobile] Prevent JavascriptException on scrollToSection
#59110
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Size Change: 0 B Total Size: 1.7 MB ℹ️ View Unchanged
|
Flaky tests detected in 2d0a1c9. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7925267275
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🎊 ! Thanks @derekblank for addressing the issue 🙇 !
I couldn't manage to reproduce it without changing the code. My gut feeling is that the following part might be related, although rotating the device and adding a block at the same time works without any issues. The fact that it only affected a few users makes me think that the scenario for the crash to happen is an edge case.
gutenberg/packages/components/src/mobile/keyboard-aware-flat-list/use-scroll.native.js
Lines 53 to 57 in 2d0a1c9
// When the orientation changes, the ScrollView measurements | |
// need to be re-calculated. | |
useEffect( () => { | |
scrollViewMeasurements.current = null; | |
}, [ isLandscape ] ); |
Thanks for taking a look!
I agree that the linked code seems like the source of |
What?
Fixes:
Why?
Prevents fatal JavascriptException from firing when
scrollToSection
is invoked.How?
Checks if the property
current
exists on thescrollViewMeasurements
within thescrollToSection
hook.Testing Instructions
To test in general, create a situation where the app invokes
scrollToSection
hook, and observe the app does not crash. This can be done by following the Testing Instruction steps from #57273, or by the steps below: