-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Add boolean contentStyle and clientId check to Column Edit InnerBlocks #47234
Conversation
Size Change: +858 B (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
Flaky tests detected in e756d94. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/3966976731
|
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! I agree including this change seems like the best option until we find out how to reproduce it or if there are other side effects from the React Native upgrade.
I tested this on both iOS and Android, I tried nesting Columns blocks and other blocks to check if there were no regressions, I also tested passing undefined
as parentWidth
and added some inner blocks and I found no issues. I also tested changing the alignments of the block.
@fluiddot @dcalhoun Given that this change was cherry-picked into the 1.87.2 release of Gutenberg Mobile, can this PR now be closed, or should we await the E2E test fixes? |
@derekblank Yes, we can close this PR as this change was merged via #47257. |
@derekblank I've just noticed that the commit that was cherry-picked in |
If I am not mistaken, the Closing this PR as it is no longer necessary. Feel free to reopen if needed. 🙇🏻 |
Oh, I overlooked that commit 🤦 , my bad. Yep, let's keep this PR closed, sorry for the noise 🙏 . |
What?
This PR adds a boolean check to values passed to the
<InnerBlocks />
component of block-library/src/column/edit.native.js.Related items:
Why?
This is an attempt to provide a fix for wordpress-mobile/gutenberg-mobile#5387, which investigates an issue reported via Sentry:
RCTFatalException: Unhandled JS Exception: TypeError: undefined is not an object (evaluating 'b[W].width')
.Currently, we have not found a way to replicate this issue in production. Based on the nature of the stack trace, it's possible that a width value may be
null
orundefined
when being passed to<InnerBlocks />
, and is possibly an unintended side-effect of internal React Native changes made in the upgrade between 0.66.2 and 0.69.4. Until a more specific resolution can be found, this fix is proposed as the user impact rate of this crash is currently 0.098%. The intent is that the boolean checks may preventnull
orundefined
values from being passed to other components, and if that is true, we would see the user impact threshold decrease in Sentry as evidence.How?
Checks for both the value of
contentStyle
andcontentStyle[ clientId ]
before passing theparentWidth
value to the component:Testing Instructions