-
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
[Mobile] - Refactor BlockListItem #49765
Conversation
103608a
to
8b5a27a
Compare
Flaky tests detected in 869ccb76d6a61e4b50e226c622d3b9e4cbb2f01c. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4798861456
|
Size Change: 0 B Total Size: 1.37 MB ℹ️ View Unchanged
|
8d93ea5
to
7b98040
Compare
faf5d51
to
b81414c
Compare
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.
This looks good so far. 👏🏻 I have not performed any manual tests, only reviewed the code itself.
🎉
7b98040
to
33d4472
Compare
5d44504
to
869ccb7
Compare
This branch has been updated with the feature branch, it also includes an extra commit 869ccb7 that adds two missing props. |
869ccb7
to
88dc039
Compare
…uses the new useEditorWrapperStyles and simplifies usage of some props and selectors
… down through restProps
…nt, rootClientId) passed down to the Block component
88dc039
to
f9083d0
Compare
Hey @dcalhoun 👋 this PR is now targeting |
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 did a bit of manual testing on an iPhone 14 Pro simulator, iPad Pro simulator, and Pixel 3a emulator. I did not encounter any issues.
* @param {boolean} props.isStackedHorizontally Whether the block is stacked horizontally. | ||
* @param {number} props.marginHorizontal The horizontal margin. | ||
* @param {number} props.marginVertical The vertical margin. | ||
* @param {Function} props.onAddBlock On Add block callback. |
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.
Suggestion to match casing of other descriptions.
* @param {Function} props.onAddBlock On Add block callback. | |
* @param {Function} props.onAddBlock On add block callback. |
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.
Sure! Updated in 4d3b846
* @param {boolean} props.isStackedHorizontally Whether the block is stacked horizontally. | ||
* @param {number} props.marginHorizontal The horizontal margin. | ||
* @param {number} props.marginVertical The vertical margin. | ||
* @param {Function} props.onAddBlock On Add block callback. |
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.
Suggestion to match casing of other descriptions.
* @param {Function} props.onAddBlock On Add block callback. | |
* @param {Function} props.onAddBlock On add block callback. |
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.
Updated in 4d3b846
# Conflicts: # packages/block-editor/src/components/block-list/block-list-item.native.js
Closes wordpress-mobile/gutenberg-mobile#5615
Fixes wordpress-mobile/gutenberg-mobile#4081
Related PRs:
What?
This PR refactors the
BlockListItem
component by updating the code into a functional component, using the new useEditorWrapperStyles hook.Why?
To keep the code updated with the latest code practices and to simplify its logic.
How?
BlockListItemContent
, andBlockListItem
Block
component, this way we know which props are being received and passed down easilyTesting Instructions
CI checks should pass for both Gutenberg and Gutenberg mobile.
Testing Instructions for Keyboard
N/A
Screenshots or screencast
N/A