WordPress / gutenberg-starter-theme

A simple theme for testing Gutenberg.
GNU General Public License v2.0
689 stars 168 forks source link

Find a better approach for centering blocks by default #30

Closed melchoyce closed 6 years ago

melchoyce commented 6 years ago

So we don't have to manually center every block by hand. Is there a shared class we can call on?

amdrew commented 6 years ago

> * could work, just not sure about performance (granted it's only selecting all direct children of .entry-content):

.entry-content > * {
    margin: 1.5em auto;
    max-width: 740px;
    padding-left: 20px;
    padding-right: 20px;
}
BE-Webdesign commented 6 years ago

> * is my personal go to, then any widths that extend beyond that can be changed on a class by class basis.

karmatosed commented 6 years ago

Does that work across all device though? Also what about where there are classes on entry-content? Would be great maybe to get a PR.

melchoyce commented 6 years ago

Anyone up for making a PR trying it out?

BE-Webdesign commented 6 years ago

Does that work across all device though? Also what about where there are classes on entry-content? Would be great maybe to get a PR.

Here is the support.

karmatosed commented 6 years ago

@jasmussen may have solved this in a similar way so throwing in for a suggestion:

article > *:not( .alignwide ):not( .alignfull ) { max-width: 50%; margin-left: auto; margin-right: auto; }

https://codepen.io/joen/pen/oEzYxb

amdrew commented 6 years ago

PR: https://github.com/WordPress/gutenberg-starter-theme/pull/35


Here's a before (master branch) of the Gutenberg demo content page:

before2


With PR:

after2


Notes:

screen shot 2018-02-10 at 2 43 52 am
karmatosed commented 6 years ago

I think to work across all instances article is better maybe @amdrew - is there a reason you didn't use the version suggested? I think the :not does have a value where overriding isn't needed, but I'd want to be sure.

amdrew commented 6 years ago

is there a reason you didn't use the version suggested?

Yes, the CodePen (https://codepen.io/joen/full/oEzYxb/) assumes that all blocks are a direct child of article:

screen shot 2018-02-12 at 12 03 03 am

That works great, but in this theme, the article's direct children are actually header, div, and footer:

screen shot 2018-02-12 at 12 05 14 am

We need to target the direct children of whichever element houses all the blocks. In the theme, this is the div with the class of .entry-content.

This shows all the blocks are direct children of .entry-content:

screen shot 2018-02-12 at 12 18 01 am

We could still use the suggested code with :not, but we'd need to change the initial selector so it's .entry-content (not article), meaning the direct children (the blocks) are properly targeted:

.entry-content > *:not( .alignwide ):not( .alignfull ) {
// code here
}

We'd then need to fix the image in the demo being full-width, since it has the alignwide class applied to it. .wp-block-cover-image is overriding it with its 100% width:

screen shot 2018-02-12 at 12 40 02 am
amdrew commented 6 years ago

Circling back to the initial comment on this issue (https://github.com/WordPress/gutenberg-starter-theme/issues/30#issue-287136689):

So we don't have to manually center every block by hand. Is there a shared class we can call on?

It be great if there actually was a shared class that existed. Thinking outside the bounds of this theme, we can never guarantee that a theme even has an .entry-content div. Some themes might also have .entry-content applied to multiple elements.

Something like .wp-block added to every block would be ideal (and perhaps filterable?). This would allow us to ditch the > * selector and just target .wp-block which we know will always be a block.

Anyone agree/disagree? An issue for the Gutenberg repo perhaps?

youknowriad commented 6 years ago

Something like .wp-block added to every block would be ideal (and perhaps filterable?). This would allow us to ditch the > * selector and just target .wp-block which we know will always be a block.

Right now, this won't be possible because some blocks like the "classic block" don't have wrappers.

youknowriad commented 6 years ago

Just a small clarification. The :not selector is correctly supported by all browsers, it's the multiple arguments for this selector that is not widely supported

https://caniuse.com/#search=%3Anot

screen shot 2018-02-13 at 17 06 57
karmatosed commented 6 years ago

Closing as merged.

anybodesign commented 6 years ago

I agree that the class .wp-block should be added to every block, seems logical to me, and easier to target all blocks with CSS.