Border controls: add defaults in the ToolPanel #34061
Merged
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
ramonjd
added
the
[Feature] Design Tools
ramonjd
requested review from
ajitbohra,
cameronvoell,
chrisvanpatten,
ellatrix,
guarani,
nerrad,
oandregal,
ntwb,
spacedmonkey and
TimothyBJacobs
as code owners
ramonjd
changed the base branch from
update/border-support-to-use-new-panel
to
trunk
ramonjd
force-pushed
the
update/border-block-support-toolbar-defaults
branch
2 times, most recently
from
759fb32
to
d6e8a5d
Compare
ramonjd
added
[Status] Blocked
|
Size Change: +93 B (0%) Total Size: 1.11 MB
ℹ️ View Unchanged
|
ramonjd
removed request for
spacedmonkey,
oandregal,
ntwb,
ajitbohra,
cameronvoell,
chrisvanpatten,
nerrad,
guarani,
TimothyBJacobs and
ellatrix
|
Not for this PR, but I also noticed that the controls are not aligned when configured with width and style:
|
|
Didn't mean to close |
andrewserong
approved these changes
Dec 9, 2021
There was a problem hiding this comment.
Thanks for following up @ramonjd! This is testing nicely for me now (tested in FF, Safari, Chrome, Edge on macOS):
✅ Button block
✅ Group block
✅ Image block
✅ Pullquote block, and box-sizing works correctly on the front end
✅ Search block and each of its different settings (button inside, outside, no button, etc)
✅ Table block, and it preserves the internal border style
Tested in Twenty Twenty Two and TT1-Blocks with all the above available. Also tested in a few classic themes (TwentyTwenty, Maywood) to make sure that the blocks that already have border available there work as expected.
Aside from the alignment of the Width input with the Style buttons (which as you noted we can fix up separately), I think this looks good to me. Might be worth getting another ✅ to confirm folks are happy with all the defaults selected, but they all looked good to me. It's a pity that our color controls are so high vertically — for the Group block, I'd personally prefer Dimensions to come first before Border controls, but again I think that's probably for another PR / discussion.
Nice work!
7 tasks
I've opened up a separate tiny PR to look at fixing up that alignment: #37244 |
stacimc
approved these changes
Dec 9, 2021
There was a problem hiding this comment.
This looks good to me as well! The defaults chosen seem reasonable to me, and I'm fine with including Style in as a default in blocks that support it to alleviate concerns raised in #36942. Especially considering the layout of the controls means the panel does not increase in size with Style added.
The only weird thing I noticed, which appears unrelated and should be investigated elsewhere, is that the Pullquote block always seems to display all of the border controls even if they're not supported in theme.json. Ie if you set customWidth: false in the border settings in theme.json, the width control will be omitted for other blocks that support it (Group, Search, etc), but it still displays and is editable for Pullquote 🤔 I can open a separate issue for this.
stacimc
requested changes
Dec 9, 2021
|
Merci for testing, folks.
Hmm, could it be because of this entry? gutenberg/lib/compat/wordpress-5.9/theme.json Line 236 in 61de106 If so, it should be only applicable in the testing env.
Excellent find @stacimc Thanks for spotting that. I'll add those defaults. |
Thank you, that's it! I missed it because it's not visible when looking at your
Got it, thanks for confirming :) |
…rontend for the Search Block. The Search Block skips serialization, which means we have to add the styles to the corresponding elements depending on the search style (button inside or not).
… a width so that the borders are consistent between setting/unsetting block style.
…s, padding etc do not add to the element's width and therefore flow outside the post content column.
ramonjd
force-pushed
the
update/border-block-support-toolbar-defaults
branch
from
456b523
to
52ca915
Compare
Code Block updated!
|
stacimc
approved these changes
Dec 9, 2021
There was a problem hiding this comment.
Thanks, this all tests great for me! ✨
Width and color seem like sensible defaults for the Code block. It looks like you have other blocks that support all controls opting into all of them by default (Group, Pullquote); IMO color and width are the most important so I don't have a strong opinion on either configuration. Looks good!
|
Thanks again @stacimc ✨
Pullquote opts into all of the controls for historical reasons that may or may not be still valid. When we enabled them back in #30951, Pullquote was displaying all border controls by default (before the age of ToolsPanel and block supports). My reasoning was that, since they were displaying by default to start with, I should probably keep it that way for consistency/predictability/whatever. I'm not so sure that reasoning is solid anymore given how we're redefining supports and defaults for a bunch of blocks anyway. As for the Group Block, it was a conscious decision to leave style and radius in defaults for similar reasons. They were all showing already. Again, now I'm wondering if that's a defendable ground for concern. I can merge this PR regardless. I'm very happy to scale back the defaults for these blocks in another PR now that the bulk of testing has been done. |
7 tasks
5 tasks
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.
Requires
Description
Lots of blocks are already taking advantage of border supports, but have they heard about the new ToolsPanel?
This PR is based on #33743, which switches border supports to the ToolsPanel, and sets which border properties are displayed in the ToolsPanel by default.
Testing
Ensure that border supports are switched on in theme.json
Create a new post and insert the follow blocks:
For each block check that:
__experimentalDefaultControlsin the block'sblock.jsonare displayed in the panel by default.For the Search Block, make sure:
For the Table Block, observe that:
For the Pullquote Block, ensure that:
Check in several themes.
Screenshots
Types of changes
Turning on existing default panel features for border supports.
Checklist:
*.native.jsfiles for terms that need renaming or removal).