Add border controls to Cover and Featured Image blocks #37783
Closed
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
jameskoster
added
[Type] Enhancement
5 tasks
|
Size Change: +304 B (0%) Total Size: 1.15 MB
ℹ️ View Unchanged
|
7 tasks
|
@jameskoster would it be possible to break this PR into three separate PRs, one for each block? The Featured Image and Cover block implementations seem pretty straightforward, leaving just the Site Logo for further discussion. |
|
I reverted the Site Logo so we can concentrate on that one separately. |
jameskoster
changed the title
aaronrobertshaw
requested changes
Mar 16, 2022
There was a problem hiding this comment.
Thanks for putting this together @jameskoster 👍
It might interest you that the Cover block portion of this PR appears to be a duplicate of #31370.
The linked PR was put on the back burner until new color controls were implemented and the border supports refined. It was delayed further when it was decided that the UX around separate controls for border color, width, and style, wasn't great and we should avoid rolling out the support until that was addressed.
There is ongoing work towards refining the border controls into a single component and these PRs should be merged in the near future ( #37769, #37770, #38876 )
My suggestion would be to continue with @ndiego's idea of splitting this PR into one per block, i.e. remove the Cover block changes here.
I did also give this PR a test and found the border radius isn't applied cleanly.
This is what I see for the cover block:
The same issue is present for the Featured Image block also.
Comment on lines
+46
to
+51
| "__experimentalDefaultControls": { | ||
| "color": false, | ||
| "radius": false, | ||
| "style": false, | ||
| "width": false | ||
| } |
There was a problem hiding this comment.
If all the border controls are in fact to be optional, I think you can omit this part of the configuration.
That said, when the border support panel was first switched to the ToolsPanel, the general idea was that any block opting into more than the border radius support should have all its border support features as default controls.
From memory, this was primarily to address UX issues around the split border color, width, and style controls. The fact this clutters the sidebar was also a driving force in refining the border controls and delaying opting into border support more widely.
Some background and context here can be found in #34061 and #36942.
|
Closing in favor of #42765 |
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.
This PR makes it possible to add border radii, color, width, and style to the following blocks:
To test Cover
To test Featured Image