-
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] - BlockList - Block component refactor #50108
[Mobile] - BlockList - Block component refactor #50108
Conversation
Size Change: 0 B Total Size: 1.38 MB ℹ️ View Unchanged
|
Flaky tests detected in 35057dd. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4937207534
|
88dc039
to
f9083d0
Compare
…alues that the editor uses
… move old class component into functional component. It also uses the same approach as the web editor for with applyWithSelect and applyWithDispatch. It also updates the code to avoid nesting too many views, and updating memoized code like the mergedStyle constant
…own file and fix an issue where the dashed border wasn't showing for inner blocks
…adds two simple integration tests
… this change simplifies using an extra View
… use the deep param
af1679c
to
deac926
Compare
…after simplifying the View wrappers around blocks
…eeds to be calculated and stored in the component's state to reflect changes
…ecent changes in the tree structure
… element for all blocks
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. I wanted to leave some initial thoughts and questions while I tested the changes.
packages/block-editor/src/components/block-list/block.native.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-list/test/block-invalid-warning.native.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-list/test/block-invalid-warning.native.js
Outdated
Show resolved
Hide resolved
…ntBlockSelected to reflect it does a deep check
…omment and place the rest under the Assert section
…the structure now will be always available except that then values would be undefined for non block-based themes
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.
Great work!
I verified the test plan passed on an iPhone SE and Samsung Galaxy S20 FE 5G. I also performed a few rudimentary tests of render counts to verify no major performance regressions were introduced in the manner.
…ist-block # Conflicts: # packages/react-native-editor/CHANGELOG.md
Closes wordpress-mobile/gutenberg-mobile#5616
Closes #45415
Related PRs:
What?
This PR refactors the code of the
Block
component andBlockInvalidWarning
.Why?
To improve maintainability by updating the code to move away from using class components in favor of functional components.
How?
First, it updates the
BlockInvalidWarning
component by only passing theclientId
as a prop, so it will handle getting other data it needs. It also adds a few simple tests to make sure block recovery works as expected.It adds a new
BlockOutline
component that extracts the logic of showing the block outlines into its own file, if we decide in the future to remove them, we can just remove this component and where it's rendered.The behavior of tapping on nested blocks changes, for inner blocks you can now access directly into that nested block instead of tapping multiple times depending on how many levels the block is nested in. Because of this, a few test helpers needed to be updated to reflect how tapping on blocks now works and the updated component tree.
Block component
mergedStyle
to avoid adding other attributes in the future without testing well it doesn't degrade performance.applyWithSelect
,applyWithDispatch
, andcompose
approaches from the web editor. I thought about finding a way to share this logic but maybe it's something we can work on in the future.Testing Instructions
Test adding blocks, nesting blocks, updating their alignment e.g. Full-width, wide with. Drag & Drop a few blocks.
CI should give us also quite a bit of confidence with the changes as well.
Testing Instructions for Keyboard
N/A
Screenshots or screencast
TapBefore.mov
TapAfter.mov