-
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
Fix increasingly big canvas in the post editor when editing patterns #62360
Fix increasingly big canvas in the post editor when editing patterns #62360
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Size Change: +1.46 kB (+0.08%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
} else if ( baseStyles.includes( TYPEWRITER_STYLE ) ) { | ||
baseStyles = baseStyles.filter( | ||
( style ) => style !== TYPEWRITER_STYLE | ||
); |
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.
So first, I think this fix is fine and we can ship it as is if you like.
But I have a couple thoughts:
Removing the style shouldn't be needed. I think the issue is that we're providing the value of the setting by reading the exact same settings. I think this hook shouldn't be using the "editorSettings" from the editor package. Instead it should be using the "settings" prop of the Layout component.
The second thing is that I think that this code should probably move within the editor package somewhere. Because I believe this padding should probably be applied to the editor every time "posts/pages" are rendered in post-only mode regardless of whether it's the post editor or site editor or some other page.
Both of these comments can be addressed separately though.
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 the speedy review.
I can follow up on that, though it'll be next week as it's end of day for me now and I'll only be back working on Monday.
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.
Right, why does the TYPEWRITER_STYLE
need to be removed if this is the only place where it's adding it to the baseStyles
?
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.
It looks like it's due to editorSettings.styles
being mutated, so it should be an easy fix to prevent that. I've pushed some commits to the PR.
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.
I've gone ahead and merged the PR, but happy to revisit in a follow-up if there's any further feedback.
…62360) * Fix big canvas in the post editor when editing patterns * Avoid adding multiple padding rules * Revert "Avoid adding multiple padding rules" This reverts commit 91edeb9. * Revert "Fix big canvas in the post editor when editing patterns" This reverts commit 37326a9. * Ensure `useEditorStyles` recomputes whenever the postType changes * Fix mutation of `editorSettings.styles` resulting in editor styles incorrectly persisting and being duplicated ---- Co-authored-by: talldan <talldanwp@git.wordpress.org> Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: ellatrix <ellatrix@git.wordpress.org>
…62360) * Fix big canvas in the post editor when editing patterns * Avoid adding multiple padding rules * Revert "Avoid adding multiple padding rules" This reverts commit 91edeb9. * Revert "Fix big canvas in the post editor when editing patterns" This reverts commit 37326a9. * Ensure `useEditorStyles` recomputes whenever the postType changes * Fix mutation of `editorSettings.styles` resulting in editor styles incorrectly persisting and being duplicated ---- Co-authored-by: talldan <talldanwp@git.wordpress.org> Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: ellatrix <ellatrix@git.wordpress.org>
…ordPress#62360) * Fix big canvas in the post editor when editing patterns * Avoid adding multiple padding rules * Revert "Avoid adding multiple padding rules" This reverts commit 91edeb9. * Revert "Fix big canvas in the post editor when editing patterns" This reverts commit 37326a9. * Ensure `useEditorStyles` recomputes whenever the postType changes * Fix mutation of `editorSettings.styles` resulting in editor styles incorrectly persisting and being duplicated ---- Co-authored-by: talldan <talldanwp@git.wordpress.org> Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: ellatrix <ellatrix@git.wordpress.org>
This was cherry-picked to the wp/6.6 branch. |
What?
There seems to have been a bug introduced lately to editing patterns in the post editor. The editor canvas has lots of empty space at the bottom that grows increasingly bigger as more content is added.
This PR fixes it.
How?
I tracked it down to a
padding-bottom: 40vh
that's added in theuseEditorStyles
hook. It shouldn't be added for patterns, but it looks like the React hook is missing thepostType
dependency.Then a second issue is that some code needs to be added to remove the styles when already added.
Thirdly, I added an extra check to make sure multiple padding rules aren't being added.
Testing Instructions
Screenshots or screencast
Before
After