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

Using "layout" in theme.json adds margin-left and -right: auto with !important to full width elements #33956

Closed
3 tasks done
philbuchanan opened this issue Aug 9, 2021 · 11 comments
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Bug An existing feature does not function as intended

Comments

@philbuchanan
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Have you tried deactivating all plugins except Gutenberg?

  • I have tested with all plugins deactivated.

Have you tried replicating the bug using a default theme e.g. Twenty Twenty?

  • I have tested with a default theme.

Description

When you add "layout": { "contentSize": "800px" } to the theme.json file, it adds the following CSS to the editor:

.editor-styles-wrapper .edit-post-visual-editor__post-title-wrapper > *, 
.editor-styles-wrapper .block-editor-block-list__layout.is-root-container > * {
    max-width: 773px;
    margin-left: auto !important;
    margin-right: auto !important;
}

If you add a block with alignfull it resets the width with max-width: none; but the margin-left and right which are set as such:

.editor-styles-wrapper .block-editor-block-list__layout.is-root-container > .wp-block[data-align=full] {
    margin-left: -8px;
    margin-right: -8px;
}

cannot be overridden due to the !important.

The result is an extra space of 8 pixels on each side of the editor.

Step-by-step reproduction instructions

See above.

Expected Behavior

Generally, I think the use of !important should be avoided since it makes it nearly impossible to override styles. Specifically, in this case, I'm not sure why the !important is necessary at all. I would expect the -8px margins to be applied to alignfull elements.

Current Behavior

See above.

Screenshots or screen recording (optional)

No response

Code snippet (optional)

No response

WordPress Information

WordPress 5.8

Gutenberg Information

Not using the Gutenberg plugin.

What browsers are you seeing the problem on?

Firefox, Chrome, Safari, Microsoft Edge

Device Information

No response

Operating System Information

No response

@annezazu annezazu added [Type] Bug An existing feature does not function as intended Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels Aug 9, 2021
@annezazu
Copy link
Contributor

annezazu commented Aug 9, 2021

cc @nosolosw in case you have thoughts here :)

@oandregal
Copy link
Member

@youknowriad would know best about layout things.

@youknowriad
Copy link
Contributor

!important is unfortunately necessary here because left/right margins applied to children blocks using the margin UI for instance (non aligned) shouldn't apply because the block should remain centered in the "content area". We tried other solutions to "center" blocks without relying on margin: auto but we failed so far.

That said, the negative margins above are added by the editor itself, I think adding important there might be an acceptable stop gap fix for now (for full aligned blocks).

@allancole
Copy link

allancole commented Oct 21, 2021

the block should remain centered in the "content area".

I’m not sure if this is on the roadmap but in the future how will we deal with site designs where the content area needs to be aligned to the left or right? It seems it should be possible to add a "contentAlignment" parameter to theme.json.

Maybe something like this could go in theme.json:

"layout": { 
    "contentSize": "680px",
    "contentAlignment": "left"
}

And then it could output something like this:

.editor-styles-wrapper .edit-post-visual-editor__post-title-wrapper > *, 
.editor-styles-wrapper .block-editor-block-list__layout.is-root-container > * {
    max-width: 680px;
    margin-left: 0 !important;
    margin-right: auto !important;
}

Visual example:

image

@colorful-tones
Copy link
Member

I've been digging a bit and have come up empty. I'm curious how we arrived to using the theme.json entry for layout:

{
	"settings": {
		"layout": {
			"contentSize": "860px",
			"wideSize": "1300px"
		},
	}
}

To output a bunch of these inline styles on the frontend: .wp-container-61810012475e7 > * {max-width: 860px;margin-left: auto !important;margin-right: auto !important;}

It seems like it would be a bit easier to work with the layout entries if they were just tied to a single class output, e.g. .wp-align-wide-max-width { max-width: var(--wp--preset--layout--wide-size); } or even .alignwide { max-width: var(--wp--preset--layout--wide-size); }

I would then allow theme authors to opt in to using margin-left/right: auto with something like:

{
	"settings": {
		"layout": {
			"contentSize": "860px",
			"wideSize": "1300px",
                        "horizontalMargin": true/false,
                        "horizontalMarginSize": "auto" 
		},
	}
}

Possibly related Issues:

@colorful-tones
Copy link
Member

Actually, @keithdevon already proposed a similar approach in #36135 👏

Would it be possible to add a class to the block instead, e.g. layout-inherit, and then CSS be added to the head like so?

@landwire
Copy link

I am with @allancole here #33956 (comment). I think for full flexibility we need a separation of width and alignment. We then obviously need two controls on the blocks.

width: ['content', 'wide', 'full']
alignment: ['left', 'center', 'right', 'float-left', 'float-right']

"layout": {
    "content": {
        "width": "680px",
        "alignment": "center",
    },
    "wide": {
        "width": "980px",
        "alignment": "center",
    },
}

@carolinan
Copy link
Contributor

carolinan commented Jul 25, 2022

Updating the CSS examples since they have changed since the issue was created.
WP 6.0.1 Gutenberg 13.7.2.

Current CSS for regular width group block without alignment, front:

.wp-block-group.wp-container-2 > :where(:not(.alignleft):not(.alignright):not(.alignfull)) {
    max-width: 840px;
    margin-left: auto !important;
    margin-right: auto !important;
}

Editor:

.editor-styles-wrapper .wp-block-group.wp-container-1 > :where(:not(.alignleft):not(.alignright):not(.alignfull)) {
    max-width: 840px;
    margin-left: auto !important;
    margin-right: auto !important;
}

Site editor, full width enabled on nested group:

.editor-styles-wrapper .wp-block-group.wp-container-1 > .alignfull {
    max-width: none;
}
.editor-styles-wrapper .is-layout-flow > * + * {
    margin-block-start: 24px;
    margin-block-end: 0;
}
.editor-styles-wrapper .is-layout-flow > * {
    margin-block-start: 0; // Note: This is overriden.
    margin-block-end: 0; // Note: This is overriden.
}

@carolinan
Copy link
Contributor

With the new useRootPaddingAwareAlignments setting enabled in theme.json, the css for the full width blocks is

.has-global-padding > .alignfull {
    margin-right: calc(var(--wp--style--root--padding-right) * -1);
    margin-left: calc(var(--wp--style--root--padding-left) * -1);
}

@tellthemachines
Copy link
Contributor

Is this issue still valid? The description states that margins with !important are a problem specifically on full width blocks, which no longer have those margin styles applied since #42085.

@tellthemachines
Copy link
Contributor

I'm going to go ahead and close this one as fixed; please feel free to reopen if there's any outstanding issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

9 participants