Refactor block alignments and margins #20689
Conversation
|
Size Change: -307 B (0%) Total Size: 864 kB
ℹ️ View Unchanged
|
| @@ -56,3 +56,65 @@ | |||
| @import "./components/editor-skeleton/style.scss"; | |||
| @import "./components/inserter/style.scss"; | |||
|
|
|||
|
|
|||
| // Default styles for alignments in case the theme doesn't provide them. | |||
| .wp-block { | |||
There was a problem hiding this comment.
Ideally, this should be something that a theme author can copy/paste to its stylesheet and tweak the numbers.
There are some tweaks necessary to achieve it though.
- We should replace the
data-alignwith the classNames. - We should see whether we can remove the
wp-blockclassName in favor of something closer to what themes do. Something more like.entry-content > *
When we'll be able to remove IE11 support, we could use CSS variables for the widths and margins.
There was a problem hiding this comment.
- We should see whether we can remove the
wp-blockclassName in favor of something closer to what themes do. Something more like.entry-content > *
If we're refactoring now anyway, I really think this is something that we should do now. I see the alignment wrapper as context to a block or blocks, not as a block itself.
There was a problem hiding this comment.
Also, if this is the main reason for magically adding the alignment wrapper, and we're going to remove the wp-block class anyway, does it still make sense to magically add it? Tbh, I'd much rather have an explicit component for it instead of magically adding wrappers
There was a problem hiding this comment.
My original ideal in #20650 (comment) was to have something like:
<AlignmentWrapper align={align}>
<Block.figure />
</AlignmentWrapper>Both in the edit and save function.
| return ( | ||
| <div | ||
| className={ classnames( 'wp-block wp-align-wrapper', { | ||
| [ `wp-align-${ attributes.align }` ]: !! attributes.align, |
There was a problem hiding this comment.
Restating my concerns in #20650 (comment), I think now is the prime time to revise the naming scheme for the alignment/position/width options. wp-align-wide shouldn't be a thing because "wide" is not really an alignment, and wp-align-left is misleading if it actually refers to a float.
There was a problem hiding this comment.
What do you propose? for me we should stay consistent in terms of classNames, in terms of labels in the UI it can be different
There was a problem hiding this comment.
If we were to put them all under a single naming scheme, I'd go with something like this:
wp-position-xxx with the xxx replaced with one of the following for each of the possible values:
align-left(as in actual left alignment)align-rightalign-centerfloat-left(or maybepull-left)float-right(or maybepull-right)stretch-wide-width(or maybe justwide)stretch-full-width(or maybe justfull-width)
I'm not sure whether wp-position- is the right naming scheme, though. CSS already has a thing called position that isn't really the same concept. Perhaps it may also be worth distinguishing from potential vertical options by using wp-horizontal-position- or wp-x-position- or something like that.
But I'm not convinced that different options should use the same naming scheme if they're technically different things. If we were to use more diverse names depending on what exactly the position/alignment/whatever actually is, I would go with something like the following:
wp-align-leftwp-align-rightwp-align-center(or possiblywp-align-center-horizontallyorwp-align-center-xto distinguish from potential vertical centering or something like that in the future)wp-float-left(or maybewp-pull-left)wp-float-right(or maybewp-pull-right)wp-width-widewp-width-full
I prefer this naming scheme since it's more concise, and doesn't try to fit a redundant, possibly incorrect word like "position" into the class names.
There was a problem hiding this comment.
I don't think we can change the name of the alignments today left/right/wide/full/center are here to stay. I don't think it's worth the hassle to rename these for BC. We should try to figure out a prefix that work well for these. Otherwise, we'll just add complexity to map the alignment name with the classname without high benefits.
Also cc @aduth as my go to naming person :P
There was a problem hiding this comment.
@youknowriad wp-align-xxx is already different from the existing alignxxx class names, though, so we're already updating the class names, aren't we?
There was a problem hiding this comment.
Yes, but I'm talking about the attribute values, not the classNames.We also need to keep. BC for existing themes with data-align[value] stylesheets. Again renaming this is possible but not without needless complexity.
There was a problem hiding this comment.
Can we omit the wp- prefix for alignments?
There was a problem hiding this comment.
@jasmussen I'm fine with removing them.
There was a problem hiding this comment.
@youknowriad Personally, I think that changing the attribute values, even if it means some somewhat complex back-compat, is well worth it to get class names that actually make sense. Changing how block alignments work is likely to break styles anyway, so we might as well take advantage of the opportunity so people don't have to update their styles twice.
Also, do we even need data-align anymore? See #20689 (comment).
| } = useContext( BlockContext ); | ||
| const supportAlignments = hasBlockSupport( name, 'align', false ); |
There was a problem hiding this comment.
Should this accept an array of possible alignments?
| @@ -199,7 +202,9 @@ const BlockComponent = forwardRef( | |||
| { ...props } | |||
| id={ blockElementId } | |||
| ref={ wrapper } | |||
| className={ classnames( className, props.className ) } | |||
| className={ classnames( className, props.className, { | |||
| 'wp-block': ! supportAlignments, | |||
There was a problem hiding this comment.
Why does this class have to be removed when there's alignment? Does it hurt to have .wp-block > .wp-block?
| // between blocks in the editor. | ||
| .editor-styles-wrapper .block-editor-block-list__block { | ||
| margin-top: $default-block-margin; | ||
| margin-bottom: $default-block-margin; |
There was a problem hiding this comment.
I've been wanting to remove this for ages, but there's always been some reason for keeping it. Can it safely be removed now? Cc @jasmussen
There was a problem hiding this comment.
It seeems like we can! From quick testing in a theme that doesn't style the editor, things look right. And it'd definitely a rule we want to remove. So it does seem now's the time!
Note that these two rules seem to mostly take its place:
and
Good to be mindful of these removals, when we get to the DOM "lightening", can check on a per-block basis, because now it will be up to each individual block to ensure a margin.
|
Theres' now two alignment buttons on thee image block. |
|
I cannot click the image block if it's not aligned. For some reason the height is 0. |
| className={ classnames( 'wp-block wp-align-wrapper', { | ||
| [ `wp-align-${ attributes.align }` ]: !! attributes.align, | ||
| } ) } | ||
| data-align={ attributes.align } |
There was a problem hiding this comment.
Why are we adding the data attributes here if we're going with classes? I think this will only make things more confusing as it's the old way of styling alignment in the editor.
|
I'm seeing the list block not centered. This is in a theme with no editor style. |
|
I do think I have earlier read it above here.... not able to locate it right now. If Global option alignment center is selected the user will see that the default alignment is center. |
|
@paaljoachim The problem with that is that the default alignment can change based on theme. You need to be able to distinguish between having an alignment set versus just using the default value. One notable issue with the current "block alignments" UI is that there is no way to explicitly select left alignment (as in actual left alignment, not float left). There's also no way to select actual right alignment for a block, explicit or implicit. The problem is made worse by the fact that the float options are called alignments, which tend to cause confusion when the user selects them and gets unexpected float behavior. |
I'll push back on this one a little bit, because this is only true in the western world via the left to right reading direction. Additionally, no alignment is also not the same as left alignment. Left aligned is explicitly set: text-align: left; as an inline style, whereas no alignment outputs nothing. You might have a WordPress theme that justifies text, and in that case it's useful to be about to text align left on a per block basis. |
|
Snap! Zeb beat me 😅 (And added additional valuable context!) |
|
To clarify a bit of my previous post, you can explicitly set left/right text alignment in the text alignment toolbar, but no equivalent exists in the "block alignment" toolbar (which contains not just alignments, but also floats and width options, and which probably needs a better name). |
|
I got a feeling that with your replies Zeb and Joen that there is needed some clarity here.
Taking a step back.... I need to take the basic steps. Using the theme Twenty Twenty as an example. Text alignment. Default text alignment is to the left. Even though it does not by default show. Default image alignment. Default image alignment is left but not float. Clicking Align left gives it a float. Default image alignment is not having any active alignment option selected. As one has to deactivate the image alignment options to get the image to not float and not to center. My conclusion (see a professor standing infront of the class lecturing) is that we also need alignment options for float of all media. Does this mean ---> An added icon for float alignment? Or have a multi choice in the same alignment drop down. Select media alignment. Then further below select if you want it to float left,center, right or no float. |
|
It's not really accurate to highlight an option as if it were selected when in fact it is not. If you explicitly set a block to use an option, that setting will persist across themes and wherever you copy-paste the block to. When an option isn't set (or in some cases set to an "auto"/"default" value), the block's style will depend on what theme and what template and sometimes even what its parent block is. This does make me wonder if perhaps the alignment toolbars should be refactored into radio option groups, with a clearly visible "auto"/"default" option listed. In fact, that may also increase accessibility. However, that's out-of-scope for this PR. To be completely clear, in case anyone is confused, the block alignment toolbar is not really just a block alignment toolbar. I would not consider floats to be alignments (or at least, they're a sort of alignment combined with an altered flow in the page), and the wide/full options are definitely not alignments in the traditional sense of the term. The only option that is an actual alignment is the "Align center" option, which actually does simply center the block. The toolbar should probably be called something else, though I'm not sure what name would make the most sense. Block position toolbar, perhaps? Currently, the "Align left" and "Align right" icons depict float behavior, so those icons should remain the icons for the float icons. New icons should be used for actual left/right alignment options. (Effectively the only difference being the lack of the two short horizontal lines representing text flowing on the side of the block; in other words, the "Align center" icon, but with the middle rectangle pushed to the left or right.) To summarize, here's my mapping of old to new:
Additionally, new options for actual left alignment and actual right alignment would be added: "Align left" ( All of this is referring to what is currently called the "block alignment" toolbar. The text alignment toolbar is unaffected by these changes. |
|
One thing I forgot to mention is that the current icons being used for |
|
Btw here are even more alignment options: |
It's worth noting that in one of the iterations of that PR, the "full screen" option was replaced with a "stretch to viewport height" option that can be toggled separately from the horizontal position/alignment options. You could theoretically introduce other options such as a "stretch to tall" option that makes something about 80% of the viewport height, thus creating a vertical position toolbar, complementing the existing horizontal position/alignment toolbar. |
| @@ -46,8 +46,8 @@ $z-layers: ( | |||
| ".components-drop-zone": 40, | |||
| ".components-drop-zone__content": 50, | |||
|
|
|||
| // The block mover for floats should overlap the controls of adjacent blocks. | |||
| ".block-editor-block-list__block {core/image aligned left or right}": 21, | |||
| // The flotaed blocks should be higher than regular blocks | |||
There was a problem hiding this comment.
| // The flotaed blocks should be higher than regular blocks | |
| // The floated blocks should be higher than regular blocks |
|
While I agree that "alignwide" and such doesn't make much sense to us as it's not an actual alignment per se, the simplified approach to left/center/right/wide/full makes sense (to me) from a user experience perspective. As long as these align/position states all continue to exist within one dropdown/group and are mutually exclusive, I think works well. After all, if alignments are split from width settings while both being simultaneously available (I'm not sure anyone here is arguing for this anyways), is there a visual distinction between full-width vs align-left + full-width? Do some of these controls become disabled if incompatible align/width states have been applied? However this more sophisticate approach is implemented, I'd love to see better global and block-specific settings (including built-in blocks) for theme developers for enabling/disabling certain alignment/position states. As discussed above, a theme can take any number of approaches for handling content with and wide/full alignments, which may be incompatible with a traditional "float" or other such specific alignment options. @ZebulanStanphill proposed distinguishing between align left/right and float left/right, but honestly I wonder if that distinction is relevant or necessary to the user. More complex layouts can be achieved already with the columns block if a certain block is not aligning the way the user wants. What I guess I'm advocating for is that the left/right alignments be handled by the block/theme context, whether that's an actual CSS float or an auto margin or flex-item justify-self/align-self, etc. If a distinction should be made for the icon itself then perhaps that's a point in favor of having both float & align classes with block and global configuration that decides which are available. But ultimately, I think it's important to step back and consider if additional options are relevant for the user and if they need to know how the alignment is working under the hood. |
|
Yeah, I'm only suggesting that the options be individually named better; it still makes sense to keep them all in the same toolbar as they are all mutually exclusive. There have definitely been occasions where I wanted to right-align (as in actual align) an image, and the default alignment (left unless the site is RTL) is pretty useful anyway, so I think it would be a good idea to add both as proper options. Making the actual left alignment option explicitly available (rather than just implicitly) would also be useful for cases where the default alignment is center. |
|
I replied to a question on the Gutenberg plugin support forum. In relation to it I made this issue: #20941 Bottom line: User had a problem finding out how to use a kind of normal alignment for the Media & Text block. I told him that he had to turn off the default wide/full alignment. |
|
Yeah, it's not entirely clear at first that you have to deselect the selected option to get the default alignment. Even without adding explicit left/right (actual) alignment options, an "Auto"/"Default" option explicitly listed in the dropdown could be nice (or necessary if the dropdown became a radio control). |
|
Closing this PR. It is clear to me that the new direction for alignments should be declarative (InnerBlocks config) and offer different possibilities (regular, flex, grid using the layout prop). |
|
Just bringing this out there as we are talking about various kinds of alignments here. |
No description provided.