-
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
[RNMobile] Handle undefined block case in block actions menu #30920
[RNMobile] Handle undefined block case in block actions menu #30920
Conversation
@illusaen I saw that you recently introduced changes here related to the transform blocks (related PR), I'd appreciate if you could review these changes because I did some modifications there. |
Size Change: 0 B Total Size: 1.46 MB ℹ️ View Unchanged
|
@fluiddot This looks good; I don't think it impacts transforms at all 👍 |
const selectedBlockClientId = first( getSelectedBlockClientIds() ); | ||
const selectedBlock = selectedBlockClientId |
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.
Since the name is singular, I changed these values to be an object instead of array.
? first( getBlocksByClientId( selectedBlockClientId ) ) | ||
: undefined; | ||
const selectedBlockPossibleTransformations = selectedBlock | ||
? getBlockTransformItems( [ selectedBlock ], rootClientId ) |
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.
getBlockTransformItems
requires to pass an array of blocks.
@geriux I added you as a reviewer because I saw you reviewed other PRs related to this part, let me know if it’s ok for you or if I should reference someone else, thanks 🙇 . |
I'll check it out 😃 |
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! Tested it on both iOS/Android and all good. Thanks for handling this!
gutenberg-mobile
PR: wordpress-mobile/gutenberg-mobile#3389Description
Problem
I was testing the undo/redo functionality with the debugger enabled and I realised that when undoing a block creation an exception is raised in the blocks actions menu component.
Exception:
TypeError: Cannot read property 'title' of undefined
packages/block-editor/src/components/block-mobile-toolbar/block-actions-menu.native.js#L275
This exception is not leading to a crash fortunately but it could produce undesired side effects as well as make the debugging experience a bit hard.
I checked further and the cause is that the block can be
undefined
right after the undo action is executed. Some of the calculations made in thewithSelect
block are not checking this case so they produce the TypeError exceptions.Solution
I checked the logic of that part and I added conditions for handling the undefined block case.
How has this been tested?
NOTE: The exception can only be triggered enabling the debugging mode via the RN developer menu.
Sources
tab and enablePause on exception
(see attached screenshot)4. Tap on the initial paragraph block 5. Add some text 6. Duplicate the block 7. Undo the action 8. Observe that a exception is raised pointing to `block-actions-menu.native.js` file
Screenshots
block-actions-menu-exception.mp4
Types of changes
Bug fix
Checklist:
*.native.js
files for terms that need renaming or removal).