Skip to content
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

Suggestion: Add a default alignment class to all blocks #10152

Closed
brettsmason opened this issue Sep 25, 2018 · 17 comments
Closed

Suggestion: Add a default alignment class to all blocks #10152

brettsmason opened this issue Sep 25, 2018 · 17 comments
Labels
[Type] Enhancement A suggestion for improvement.

Comments

@brettsmason
Copy link

The .alignwide/.alignfull functionality is great and provides some much needed flexibility with layouts. However one problem I see and have been experiencing with the system is there is no default alignment class.

There are a few reasons/use cases where this would really help:

  1. In themes, I at least have been styling content like this:
.entry-content > * {
    max-width: 1024px;
}
  1. In creation of custom blocks in a plugin, for example a container block, there is no way to know what width to contain the inner content to, eg:
<div class="my-container-block"><!-- This is full width -->
    <div class="inner-content"><!-- this should be the same width as point 1 -->
    </div>
</div>

Both of the above cases could be made a lot easier with a simple class that every block receives and is a required style in a theme. I'm not sure what that class should be, but I think it would be really helpful for more complex layouts. Happy to expand on the above if its not clear enough.

@designsimply designsimply added [Type] Help Request Help with setup, implementation, or "How do I?" questions. Needs Technical Feedback Needs testing from a developer perspective. labels Sep 26, 2018
@designsimply
Copy link
Member

Would it help to look at the cover image block code as an example? I don't know the specifics, but in testing I know that the text in that block has center alignment by default.

@brettsmason
Copy link
Author

@designsimply So looking over the cover image block, you are correct that that the .wp-block-cover-image-text class has a max-width assigned - https://github.com/WordPress/gutenberg/blob/master/packages/block-library/src/cover-image/style.scss#L39

However that is referencing a Sass variable $global-width which is available to internal Gutenberg blocks only. Some way of having a global width available for 3rd party plugins/themes would be great so its possible to match the width between core and 3rd party blocks by using a standardised way of managing this width.

Also to give a 3rd party example: https://github.com/brettsmason/BreezeBlocks/blob/develop/src/blocks/container/block.js#L115 - that's the div that I would want the same width as any other block that uses a max-width in this way.

Hopefully this is clear, but if not happy to expand in more detail.

@timhibberd
Copy link

@brettsmason - could you take a look at the following issue I raised and tell me if it relates to your issue. As a theme designer I have an issue with GB styling the base class and am recommending all styling be qualified. So, in the case of an image block it would be .wp-block-image .alignnone in the default state. Is my suggestion of making all block styling qualified and adding .alignnone the same issue you are having with no default alignment class (i.e. missing .aligndefault)?

@brettsmason
Copy link
Author

@timhibberd I think there is some overlaps for sure.
Here is a quick illustration to explain in more detail:

widths

Most themes seem to use code similar to Twenty Nineteen to control the max width: https://github.com/WordPress/twentynineteen/blob/master/sass/blocks/_blocks.scss#L7

They set a max width on every element that's a direct child of entry-content, and then overwrite this with the alignwide and alignfull classes.

This works in the illustration above on the 2nd block, but what about the nested block within the first example? There is no way for a block author to add a simple class and let a theme author control the max width of this easily.

Adding a default class to every block would solve this. I don't know what this class would be, but I think its really needed and would be a pretty easy inclusion.

@timhibberd
Copy link

Agreed.

My issue is that if GB block styles the base class it makes it hard to target.

A theme can easily target a qualified base class but anything targeted to the unqualified base class will affect all qualified base-class instances as a side-effect. GB and the themes need to work together and if GB leaves the block base class unstyled, the theme can choose to style the block base class or not and the end-user's designer can still use specific targeting or ! to override the theme. Everyone is happy.

If nothing else someone needs to explain why the Image Block & Cover Block inconsistently set max-width, width, and left / right margins relative to Media & Text Block and Embed Block.

And I'm not sold on the equations other theme designers chose to put in their themes. I'm not challenging their choice...it's just not a solution I want to take. If the block sets .wp-block-image .alignfull {width:100%}and .wp-block-image .alignwide {width:80%} then I just need to choose how I want that to appear within my theme container model. In my case it is as simple as removing / reducing the container margin for the blocks:

.alignfull{
	margin: 0 -40px;
}
.alignwide{
	margin: 0 -20px;
}

@designsimply designsimply added [Type] Enhancement A suggestion for improvement. Needs Design Feedback Needs general design feedback. Needs Decision Needs a decision to be actionable or relevant and removed Needs Technical Feedback Needs testing from a developer perspective. [Type] Help Request Help with setup, implementation, or "How do I?" questions. labels Jan 30, 2019
@mapk
Copy link
Contributor

mapk commented Mar 8, 2019

I'm open to this idea, but I'm unfamiliar with any historical context to this decision. @jasmussen any thoughts that might help shift this decision one way or the other?

@jasmussen
Copy link
Contributor

Thank you for the ping Mark, and thank you bretts for great ticket and notably the illustration in #10152 (comment).

This specific issue has come up a number of times and in a number of different variations, suggesting all blocks should have a "wp-block" class or something in that vein. For most situations, we've been able to avoid additional classes and markup, by some clever CSS'ing on a per-use case bases.

However this is an interesting proposal, and given that we have just shipped a Cover block with nested children, and hope to soon ship a "Section" block, it's definitely sensible to resurface the conversation around a centered main column existing in conjunction with wide and fullwide alignments. In that vein, although this suggestion is slightly different in the suggested implementation, it could in spirit, be a duplicate of #14234. Can you all take a look at that ticket also, plus comments, and verify if it's similar?

Because if yes, then I really liked @youknowriad's suggestion there.

On a slightly transversal note, it would be good to narrow this conversation to be specific to the post content, and not any blocks that might or might not in the future be outside the post content area. To clarify, one of the concepts currently being explored is to use the block editor for the full page, including everything. As we try to figure out exactly how that works and what that means, we should be careful not to optimize too highly towards one situation or the other. For example full and wide alignments are concepts that make sense inside the post canvas itself — as illustrated, with wide and full wide images breaking up the layout of a centered column of text. But when it comes to layout, say for example you have a giant Header block — this block just as in regular HTML, should receive its layout from templating CSS, and not have to be applied "full-wide" in order to stretch from edge to edge.

@brettsmason
Copy link
Author

@jasmussen apologies for the delay!
Yes I'd agree that that issue in spirit is trying to solve a similar issue.

I guess my issue was focused more on the CSS implications of not having a default alignment class on every block. It adds to the specificity and then requires you to have to do some less than ideal selectors to get the desired effect.

In the context of the section block, if every block receives this default alignment class, then you could manipulate the inner content width purely by adding the alignwide/alignfull classes.

I know its probably more complicated than that, but making this easier in a way that allows for clean CSS is my hope 😄

@mapk
Copy link
Contributor

mapk commented Apr 5, 2019

@brettsmason where are we with this one? Is it resolved with the Section block, or are we still missing classes that you feel should be added?

@brettsmason
Copy link
Author

@mapk I personally think we are still missing a way for custom block authors especially to style inner blocks to match a themes styles.

@joyously
Copy link

The trick is to remember that alignwide and alignfull are not alignments. They are widths. So to treat them as alignments messes up existing standards for legacy content and legacy themes.
However, if you want to add a default width class, that might be a way to help make the misnamed alignwide and alignfull classes work better.

@kjellr
Copy link
Contributor

kjellr commented Apr 16, 2019

👋 We discussed this in this week's Design triage on slack:
https://wordpress.slack.com/archives/C02S78ZAL/p1555431995099600
(A WordPress.org slack login may be required to view the discussion there)

In general, I'm definitely torn on this one. I know from experience that it's difficult to target non-aligned blocks. When working on theme styles for InnerBlocks inside the Section/Group block recently, I definitely had to deal with some frustrating :not(.alignfull):not(.alignwide) classes in order to restore expected nonaligned block behavior. Gutenberg's full of :not() rules, and I think this would help cut out a bunch of them.

But philosophically, I do think that classnames should be additive. If there's no extra state/feature/setting/functionality, no class should be added. Adopting this suggestion would immediately add a huge amount of alignnone classes to the front-end of most pages. That adds up to a lot of code bloat, so we should think about whether or not it's really necessary.

@brettsmason
Copy link
Author

@kjellr Thanks for the update.
Even a simple wp-block (or something less specific to WP) that gets applied to every block would solve the issue.

🤞 something gets decided on and moved forward...

@gziolo
Copy link
Member

gziolo commented May 23, 2019

@mtias could you help to decide as you should know best why we don't provide default alignment classes anymore. I think we had some in the past.

@designsimply
Copy link
Member

@kjellr would it be okay to remove the Needs Design Feedback label now that you've mentioned the discussion from Design triage and posted some notes or do you think further design feedback is needed here?

@kjellr
Copy link
Contributor

kjellr commented Jun 6, 2019

I think further design feedback is needed — I think the design team leans towards no on this for the reason set forth above, but it could use some more discussion.

@mapk
Copy link
Contributor

mapk commented Jun 11, 2019

But philosophically, I do think that classnames should be additive. If there's no extra state/feature/setting/functionality, no class should be added.

For this reason stated by Kjell, I'd rather not add a class here. I'm going to close this one for now. If further conversation is required, please feel free to open it back up.

@mapk mapk closed this as completed Jun 11, 2019
@mapk mapk removed Needs Design Feedback Needs general design feedback. Needs Decision Needs a decision to be actionable or relevant labels Jun 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

8 participants