Comment Pagination Previous: Add Border and Spacing Support #64308
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
|
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
|
shail-mehta
requested review from
ryanwelcher,
juanmaguitar,
fabiankaegy and
ndiego
as code owners
shail-mehta
changed the title
There was a problem hiding this comment.
Thanks for the PR @shail-mehta 🙇
This is looking pretty good so far.
During the recent wave of border support adoption across more blocks, a trend has developed that the border controls should be hidden by default where their application is more of a niche use case. This seems to be the case on most of the blocks that you might describe as "typographic", for example:
- Site Title
- Site Tagline
- Post Navigation Link
- Time to Read etc.
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 | |
| } | |
| }, |
Following other similar blocks, it might be best to hide these controls by default.
Spacing controls are normally displayed by default so need to be explicitly hidden here.
Comment on lines
51
to
56
| "__experimentalDefaultControls": { | ||
| "radius": true, | ||
| "color": true, | ||
| "width": true, | ||
| "style": true | ||
| } |
There was a problem hiding this comment.
Suggested change
| "__experimentalDefaultControls": { | |
| "radius": true, | |
| "color": true, | |
| "width": true, | |
| "style": true | |
| } |
Applying borders to this block is more of a niche use case. It might be best to hide these controls by default.
Unlike for the spacing support, here we can simply omit the __experimentalDefaultControls values.
This was referenced Aug 8, 2024
|
@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 further on this one.
I've taken it for another spin and noticed that the default margin styles for the comments pagination override the global styles margins.
It might be worth getting some design feedback on whether it does make sense to allow margin support on these inner comment pagination blocks or if the margin here should be control via the parent Comments Pagination block.
I've added a "Needs Design Feedback" label to this PR.
|
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 Previousblock.Part of #43247
Why?
Comments Pagination Previousblock is missing border and Spacing support.How?
Adds the border and Spacing support in block.json.
Testing Instructions
Comments Pagination Previousblock's border and Spacing is Configurable via Global Styles.Comments Pagination Previousblock and Apply the border Styles and Spacing.Comments Pagination Previousblock styles take precedence over global Styles.Comments Pagination Previousblock borders and Spacing display correctly in both the Editor and Frontend.Screenshots or Screencast
add-border-support-in-comment-previous-text.mp4