-
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
Block Support: Add width block support feature #32502
Conversation
Size Change: +765 B (0%) Total Size: 1.15 MB
ℹ️ View Unchanged
|
This is cool to see, thank you for work on this. Going back to #27331, the design there shows a different width toggle: I understand that there will be cases where we need precise width control. But arguably, and the reason for that initial design, most of the time you should be using one of the theme-defined widths, such as default, wide, or full-wide. By being default/wide and full-wide, the controls in the sidebar are also aware of what you might have set in the block toolbar, rather than potentially conflict with them (such as choosing full-wide in the block toolbar, and 20px in the inspector). We almost certainly want the fine-grained width-control for some blocks, such as Button. But probably not group? Or perhaps we need an improved UI that enables both? But in any case, that makes it a bit harder to land this one. |
I'll update this. I got a bit of tunnel vision after adding the initial height block support PR and worrying about how these controls would play within the new block support panel (Dimensions in this case). I've added the "In Progress" label to this for the time being.
The group block was only used to test as it is a simple block. While updating this I'll see what options we might have via the block support flags to perhaps opt in to varying levels of width support. Simply opting in might give the segmented control. Setting the support flag to "custom" or something might then allow for the block to be given a UnitControl for that fine-grained control. |
@jasmussen I've had a chance to revisit this one today however I am struggling to understand how it is supposed to work based on the animated GIF and your previous comment. I didn't find any real answers in #27331 or #28356 either so I must be missing something. My thoughts around the mock up for the width control include:
In short, my current thoughts boil down to:
Thanks in advance for the help getting up to speed on this one 🙏 |
It's a bit forward looking and needs a few other pieces in place, so definitely consider it a mockup more than a fully formed thought. In fact there are a few mockups that go in different directions for exactly the reasons you outline. But to respond specifically around the alignments/widths mockup, that was intentional enough in that it acknowledges explicit horizontal pixel widths as something you should rarely if ever need for most blocks, as they very easily break responsiveness. The alternative are "width toggles" like wide and fullwide, as shown as a width "alignment". There's another mockup that embraces it as an "alignment", but makes it a dropdown instead, like so: But even that still needs a bit of thought, with some conversations suggesting we separate the actual alignments from the width based ones, and call those "Positions" instead: More high level, those designs assume a dimensions panel that is contextual to some of the larger container blocks, like Cover or Group being selected. For other blocks, such as Button or Search, those "positions"/alignments would not make sense, and we'd likely want to end up with something closer to what we have today, with a segmented control featuring shortcuts to 25%/50%/75%/100% sizes. The balance in the following mockup isn't right — the pixels need massaging, maybe we want to show primarily the segmented control but make explicit values available in a toggle similar to individual paddings? But to convey the point, it's this general thought: So my previous comment was more to convey that the Dimensions panel needs to be a bit more contextually aware, offering some affordances only to blocks like Search or Button, and alignment affordances to others. Since this PR targets the group, that's why I brought it up. But given we'll almost certainly still need the width control (even if not the height control) for the Button (singular) block, perhaps that's the best place to start? If it is specifically destined for the group, we'll need to advance the designs a bit, and today is a bit tricky for me on that front, but I can probably find some time next week. |
Thank you for the explanations, it's becoming a little clearer 🙇
It sounds like we need to iterate on the designs first before we can go much further down the path of a segmented control.
Would explicit percentage widths not be perhaps desirable in some cases?
If these toggles overlap with the align
Ok, I interpreted this mockup as only including the separate alignment support, not width. I didn't realise they were supposed to be one and the same in this mockup.
I'd absolutely vote for separating them.
This is something concrete that I could update the width control with in this PR. Do you think it is worth the time doing that now or should I hold off for any further design iterations/decisions?
This PR doesn't actually target the Group block. I simply used it as a block to test with as it's generally a very simple block. This width control will only be included within the Dimensions panel for blocks that opt into the width block support. If it doesn't make sense for the Group block, we don't opt in for it. If it does make sense for a Button, great, we opt in and so on. Same goes for the height support etc. If it helps I can rewrite the testing instructions and update the demo GIF to revolve around the individual button block? |
Cool, I'll find some time next week.
These toggles in the inspector would be the same as those in the toolbar. But in the mockups they are included because they are conceptually related to dimensions. Omitting them from dimensions because they are in the block toolbar already might be confusing. In this case, I think of the block toolbar as a shortcut. You can see how that works with the Cover block "full height" option, which when toggled simply sets a 100vh height: You can see the connection between the two. That's also the key bit about potentially including such alignments in the inspector: wide and fullwide likely need to unset any explicit widths you might have set, because they take precedence. When you see the width becoming unset as you toggle one of those alignments (perhaps the entire widht panel graying out) it might make for a more obvious user experience. This is part of what needs to be made clearer in the mockups, bug hopefully this comment can set the stage.
Yes, Button does that and it's working well, so we'd at least want it as a default for those. We may also want it for the bigger blocks, but we need to find a balance of prominence in the sidebar that naturally leads towards good choices by the user. It's very easy for someone to set a specific width of their block, looking at a desktop interface, and not realizing that width just broke the responsiveness of their site. Perhaps that balance is showing alignments by default (whatever we call them), and hiding explicit width in the progressive menu. That would at least enable it for patterns and power users.
Let me see if I can take a quick stab at an overall doodle.
👍 👍 |
Here's a thought: Inspired by the padding control, there could a default and an advanced view for the width control. By default, it's just shortcuts to useful values such as 25+100%. Click the edit button and the value becomes directly editable. I'd appreciate a quick sanity check, perhaps @critterverse @javierarce @kjellr? It would be primarily an upgrade to this interface for the Button block: — but perhaps it could also be a more general Width control you could toggle on for the larger blocks, as discussed in this thread. Edit: To clarify, this would not be a replacement for all width controls, but be in addition to, something like a "Segmented width control". For something like an Image, we'd still almost certainly need the basic control: |
I like this. I'm not sure it's a totally clear parallel to the padding control though. For the padding control, you're splitting a value into four sub-values, whereas in the font-size and width controls, you're overriding the preset options. Your mockup feels more akin to the font size control: ... but more nicely done. 🙂 (As an aside, in the padding mockup from #27331 I'm not sure folks will know to click on the padding icon, but your width mockup doesn't have that issue.) |
The proposed segmented width control looks great! ✨ I'll work on a prototype for it in this PR along with a means to choose which type of width control make sense when opting into the width block support.
My current thinking is that the feature flag for the block support would accept a couple of different values rather than only a boolean. If it is set to
Ok, so this means that the block toolbar controls are alignment block support controls and so to are the controls in the inspector. This is where I think I've failed to communicate the confusion I've had. Previous descriptions and the mockup labelled alignment controls as width controls leading me to believe we needed to be overlapping their functionality. My question about omitting options from the width block support controls was to remove some of that overlap thinking that the alignment block support controls would cover those elsewhere in the dimensions panel. Whether that was in a segmented control or the proposed dropdown.
If the alignment block support adds controls to the dimensions panel as mentioned above, the inspector controls there would inherently match or be "connected" to the alignment options in the block toolbar as they'd both use the same block attribute. My current understanding that I'll move forward with is:
If the above is correct, then all that is required on this PR is to:
|
My reason for thinking this approach of two width controls can work is that:
That's the primary problem we want to solve. The thing about the design I'd like to still noodle on Kjell brings up. I'd like a stronger pattern for this type of toggling between basic and advanced, I'm not sure the edit button is right. I'm not sure we want a slider for the width. I'd love to find a pattern that is more easily scalable to other controls that might swap between basic and advanced like this. So I think we probably want this, but I'd also love a small sanity check by @mtias if he has time. |
In case it aids further reasoning about the width control and width support I have a separate PR trialling adding the width block support and new control to the button block. #32878 |
Thanks for your patience. I'm getting some notices running this branch, probably a result of my local env ( And from what I can see, I think it's worth trying:
As for next steps, outside of a good code review:
My biggest gotcha with this one is that when you click the edit icon to "unlock", you have to "tab backwards" to set focus in the width panel. |
I've started fresh, checking out this branch, building it, then following through the test setup as per PR description (steps 1-3). I can't replicate the issues you were facing. Opting into the width support for the paragraph block specifically, since that was mentioned in the error, worked fine as well (not suggesting we want support on that though). My guess is if you were switching between branches with the build watch task running something went awry. Rebuilding this branch should sort it out.
I'm not sure I follow what you are looking for here. Do you mean we want the simple UnitControl display of the width control for the image block? Or, that we need a completely custom dimensions panel for the image block? One that allows for the size control etc.
By "shortcut" do you mean the version of the width control with the preset percentage widths? The panel for me is the dimensions panel. This PR is about the width block support and the control to support that.
I take it that you are not referring to the toggle button to switch from the preset button group display of the width control to the basic unit control display. At present, this width block support allows you to choose how the width control is presented. There are two options:
Once that PR is finished and lands, we can definitely update this width control. It can be done separately to this PR. I imagine we'll want to also update other uses of button groups that can be replaced by that segmented control.
I'll remove the slider from the new width control. It should also be possible to set focus to the unit control after "unlocking" the basic view via the edit icon. |
I was unclear here. I was simply confirming your thoughts: we will need the shortcuts from the segmented control with some blocks, and we'll need just a basic width input field for others. Your two options look good to me to try out 👍 👍 |
7c3ac3f
to
3c62628
Compare
ce0236d
to
1399bd8
Compare
What are the next steps in this PR? |
Thanks for checking in @paaljoachim 👍 The height and width block supports weren't in the must-haves for 5.9 so were put on the back burner until recently. Unfortunately, with the 5.9 release, we didn't have a means of overriding the theme.json classes to allow the addition of new block support features. This is currently being addressed with the required updates being backported to 5.9.1. As you would expect this has been a blocker to progressing this PR. That said, I've been working on bringing the new block support PRs up-to-date and should be in a position to push those changes sometime this week. In summary, the next steps are:
|
Thanks for the update @aaronrobertshaw |
3c62628
to
840b77d
Compare
There are ongoing issues with being able to extend the theme.json settings which are preventing updating this PR. It may not be completed this week. I'll also be AFK until mid-March so there might be further delays on progressing width support. |
It sounds like solving a few of the issues with the theme.json before landing this PR would be helpful. |
757dc48
to
0023551
Compare
1399bd8
to
4e0aedc
Compare
I've cherry-picked some commits from #38883 to allow the proper extension of theme.json and global styles. At this time, this PR is working however there has been a recent change of plans around how we might manage the general layout of blocks. This might actually include new layout blocks specifically. It is yet to be seen how appropriate control over height and width will be within this context. As a result, I will close this PR and the other related dimensions support PRs.(#32499, #32878) |
Regarding...
Is it possible to link to issues/PR's/more information about these things? |
Thanks for the question @paaljoachim. At this stage, I don't have any specific issues, PRs, or additional information about the layout blocks. Unfortunately, it is a bit too early for that and we'll need to do some exploration and experimentation first. About the best I can do is offer this link highlighting the various layouts we'll be looking to offer whether by block supports/theme.json or dedicated layout blocks. |
Interesting! Thank you @aaronrobertshaw |
Related:
Depends on:
Description
How has this been tested?
Style.js test:
npm run test-unit:watch packages/block-editor/src/hooks/test/style.js
Manual Test Instructions
true
instead of"segmented"
. Repeat the test process this time ensuring the width control is a standardUnitControl
Screenshots
Possible Next Steps
Types of changes
New feature.
Checklist:
*.native.js
files for terms that need renaming or removal).