Comment Pagination Next: Add Border and Spacing Support #64310
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shail-mehta
added
[Type] Enhancement
shail-mehta
requested review from
ajitbohra,
ryanwelcher,
juanmaguitar,
fabiankaegy and
ndiego
as code owners
|
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: +3 kB (+0.17%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Thanks for putting this PR together along with #64308 @shail-mehta 👍
As noted over on #64308, I think we might need to make the new block support controls optional so they don't display by default.
I've left a couple of suggested tweaks below.
Comment on lines
42
to
45
| "spacing": { | ||
| "margin": true, | ||
| "padding": true | ||
| }, |
There was a problem hiding this comment.
Suggested change
| "spacing": { | |
| "margin": true, | |
| "padding": true | |
| }, | |
| "spacing": { | |
| "margin": true, | |
| "padding": true, | |
| "__experimentalDefaultControls": { | |
| "margin": false, | |
| "padding": false | |
| } | |
| }, |
Spacing controls show by default but their application here is a bit of a niche use case. To make them hidden by default, they need to be explicitly opted out of.
Comment on lines
50
to
56
| "style": true, | ||
| "__experimentalDefaultControls": { | ||
| "radius": true, | ||
| "color": true, | ||
| "width": true, | ||
| "style": true | ||
| } |
There was a problem hiding this comment.
Suggested change
| "style": true, | |
| "__experimentalDefaultControls": { | |
| "radius": true, | |
| "color": true, | |
| "width": true, | |
| "style": true | |
| } | |
| "style": true |
To make the border controls optional (hidden by default) we can just omit this default controls config.
|
@aaronrobertshaw Thank you for your feedback. I have addressed the suggested changes in the latest commit. |
There was a problem hiding this comment.
Thank you for iterating here @shail-mehta 👍
While testing the latest changes, I noticed the default margins on the block override those applied via Global Styles. This is the same issue noted in my review of the Comment Pagination Previous PR.
My hunch is we might need to lower the specificity of those default styles to 0-1-0 and make sure that they still work as expected, including with custom layouts applied to the parent Comments Pagination block.
This is what I was seeing while testing:
Screen.Recording.2024-08-09.at.2.54.20.PM.mp4
P.S. As the scope of this PR has expanded to spacing supports, can you update the PR title, description, and test instructions to match? This will help others who may come to the PR in the future seeking context.
shail-mehta
changed the title
|
I have updated PR Title, description and Instructions in this PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Add Border and Spacing support to the
Comments Pagination Nextblock.Part of #43247
Why?
Comments Pagination Nextblock is missing border and Spacing support.How?
Adds the Border and Spacing support in block.json.
Testing Instructions
Comments Pagination Nextblock's border and Spacing is Configurable via Global Styles.Comments Pagination Nextblock and Apply the border Styles and Spacing.Comments Pagination Nextblock styles take precedence over global Styles.Comments Pagination Nextblock borders and Spacing display correctly in both the Editor and Frontend.Screenshots or Screencast
add-border-support-in-comment-next.mp4