-
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] - Adds useEditorWrapperStyles hook #49616
[Mobile] - Adds useEditorWrapperStyles hook #49616
Conversation
Size Change: 0 B Total Size: 1.37 MB ℹ️ View Unchanged
|
Flaky tests detected in 7b98040. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4691711842
|
4125399
to
f9b9328
Compare
8d93ea5
to
7b98040
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.
Thanks for providing such great context in the PR description. Very helpful.
packages/block-editor/src/hooks/test/use-editor-wrapper-styles.native.js
Show resolved
Hide resolved
packages/block-editor/src/hooks/use-editor-wrapper-styles.native.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/hooks/use-editor-wrapper-styles.native.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/hooks/use-editor-wrapper-styles.native.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/hooks/use-editor-wrapper-styles.native.scss
Outdated
Show resolved
Hide resolved
…om marginHorizontal values
… styles to the JavaScript side and update tests
…ject param instead of first level params
7b98040
to
33d4472
Compare
Hey @dcalhoun 👋 I've addressed the first round of feedback, let me know what you think! I also updated the feature |
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.
The latest changes look great. My one comment is a suggestion, not a request.
packages/block-editor/src/hooks/use-editor-wrapper-styles.native.js
Outdated
Show resolved
Hide resolved
… canvasStyles logic
Thank you for the review and feedback! |
* Mobile - Adds initial useEditorWrapperStyles hook * Mobile - Update useEditorWrapperStyles hook to take into account custom marginHorizontal values * Mobile - useEditorWrapperStyles hook - Remove helper tests and exports * Mobile - useEditorWrapperStyles - Move max width values from the SCSS styles to the JavaScript side and update tests * Mobile - useEditorWrapperStyles - Update helpers to use an options object param instead of first level params * Mobile - useEditorWrapperStyles - Update wrapperStyles logic to unify canvasStyles logic
Closes wordpress-mobile/gutenberg-mobile#5614
What?
This PR introduces a new hook
useEditorWrapperStyles
which will replace the usage of the ReadableContentView component in a separate PR. It will also handle the margins and max-width that will be used for the block wrappers within theBlockList
.Why?
Currently, the ReadableContentView component uses listeners to detect window dimension changes to then partly handle the alignment of the content depending on the
align
prop. This component is used in different places like the EditorHeader, EmptyListComponent, and BlockListItem.Having this in the
BlockListItem
for example when having different blocks that use InnerBlocks means that there will be quite a lot of listeners added when is not needed.Nesting too many
View
components is also a problem as we've reached the maximum in the past so we want to avoid that too.The BlockListItem component also handles margin and style data depending on the screen and the alignment so this hook will take care of that too.
How?
The hook will use there functions:
getWideWidthStyles
-> It will handle the styles logic forwide
alignments.getFullWidthStyles
-> It will handle the styles logic forfull
alignments.getBlockMargin
-> It will handle the horizontal margins for a block taking into account its parents.useEditorWrapperStyles
Will return the
wrapperStyles
and themargin
.Usage examples:
For the EditorHeader component:
For the BlockListItem component
Testing Instructions
CI checks should pass.
An integration PR will follow up this one.
Testing Instructions for Keyboard
N/A
Screenshots or screencast
N/A