-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[Mobile] - Prevent passing potential false
values to the onPress
prop
#60595
Conversation
…AddBlock and onToggle
…he media options reference directly
Size Change: 0 B Total Size: 1.75 MB ℹ️ View Unchanged
|
false
values to the onPress
propfalse
values to the onPress
prop
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 @geriux. 🚀
# Conflicts: # packages/react-native-editor/CHANGELOG.md
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. |
@geriux for my own education, would you mind sharing how you identified these specific lines to change/guard? |
Of course! Well, I'm still waiting to see if we don't get any new reports since I couldn't reproduce it, but hopefully, this would address it. From the crash reports, the code line being referenced was the following:
So I then looked through all of the code base and the usage of the My gut feeling is that there are cases where users tap several times potentially on low-end devices while UI items are being re-rendered causing the app to crash when there's an invalid value set in the My idea with this PR was to see if this could be the case, if we still get crash reports then it would have to be something unrelated to the Thanks for asking about it and let me know if you have any other questions! |
What?
This PR aims to fix a potential crash where it appears an
onPress
prop has afalse
value.Why?
There are a few crash events on low-end devices where the editor crashes in the Pressability component of React Native. After several different investigations, we haven't found reproducible steps of this problem.
How?
This PR prevents passing optional values to the
onPress
prop of theButtonBlockAppender
component andMediaEdit
button in the Cover block.With these changes, a function will always be passed with checks to prevent passing potential
false
values.Testing Instructions
Note
It's recommended to smoke test the following as there are no reproducible steps
ButtonBlockAppender - Test case 1
+
button to add a blockButtonBlockAppender - Test case 2
+
button to add a Columns block+
appender button and add a blockButtonBlockAppender - Test case 3
+
button to add a Buttons block+
appender button and add another Button blockCover Block MediaEdit
+
button and add a Cover blockTesting Instructions for Keyboard
N/A
Screenshots or screencast
ButtonBlockAppender
ButtonBlockAppender.mov
Cover Block MediaEdit
MediaEdit.mov