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

Find a better approach for centering blocks by default #30

Closed
melchoyce opened this issue Jan 9, 2018 · 14 comments
Closed

Find a better approach for centering blocks by default #30

melchoyce opened this issue Jan 9, 2018 · 14 comments

Comments

@melchoyce
Copy link
Collaborator

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

@amdrew
Copy link
Contributor

amdrew commented Feb 6, 2018

> * 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
Copy link

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

@karmatosed
Copy link
Member

karmatosed commented Feb 6, 2018

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
Copy link
Collaborator Author

Anyone up for making a PR trying it out?

@BE-Webdesign
Copy link

BE-Webdesign commented Feb 6, 2018

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
Copy link
Member

@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
Copy link
Contributor

amdrew commented Feb 9, 2018

PR: #35


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

before2


With PR:

after2


Notes:

  • The image width in the first image changed but that's because the demo actually has a class of alignwide on the first image, not alignfull.

  • I don't think :not is necessary since the .alignwide and .alignfull selectors can easily override the previous styles. The browser support doesn't appear to be the best either (https://caniuse.com/#search=%3Anot):

screen shot 2018-02-10 at 2 43 52 am

@karmatosed
Copy link
Member

karmatosed commented Feb 10, 2018

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
Copy link
Contributor

amdrew commented Feb 11, 2018

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
Copy link
Contributor

amdrew commented Feb 11, 2018

Circling back to the initial comment on this issue (#30 (comment)):

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
Copy link

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
Copy link

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
Copy link
Member

Closing as merged.

@anybodesign
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants