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

Block Gap css variable is loaded in the editor for some classic themes #35197

Closed
stacimc opened this issue Sep 28, 2021 · 3 comments · Fixed by #35209
Closed

Block Gap css variable is loaded in the editor for some classic themes #35197

stacimc opened this issue Sep 28, 2021 · 3 comments · Fixed by #35209
Assignees
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@stacimc
Copy link
Contributor

stacimc commented Sep 28, 2021

Description

Related: #34452

The generated CSS variable for the block gap spacing config is being added to the stylesheet for some classic themes, only in the editor.

Screen Shot 2021-09-28 at 3 47 36 PM

The styling rules that pick it up and utilize it for layout spacing are not generated, but the existence of the global --wp--style--block-gap variable at all causes backwards compatibility issues for blocks that opt into the Gap block support or otherwise make use of the variable in their CSS. For example, this kind of styling rule anticipates that --wp--style--block-gap will be undefined in classic themes, allowing fallback to a default value:

margin-left: var(--wp--style--block-gap, 2em);

Instead, the global 24px gap is picked up in the editor, and the fallback value is picked up on the frontend.

Themes where I can reproduce the bug: Twenty Twenty One, Seedlet
Themes where I cannot reproduce: Twenty Twenty, Maywood

Step-by-step reproduction instructions

  1. Activate the Twenty Twenty One or Seedlet theme
  2. In a new post, insert a Columns block with 33/33/33 split.
  3. Inspect the center Column in the editor. You should see that --wp--style--block-gap is defined and is being picked up by this column's styles (so the left margin is 24px)
  4. View the post on the frontend and inspect the center Column again. You should see that the same style rule is applied, but --wp--style--block-gap is no longer defined, so the fallback 2em is used for the margin.

Screenshots, screen recording, code snippet

Styles in the editor:
Screen Shot 2021-09-27 at 4 06 58 PM

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@stacimc
Copy link
Contributor Author

stacimc commented Sep 28, 2021

This isn't terribly noticeable with the Columns block, because it only results in a slightly smaller gap between columns on the frontend versus the editor. I noticed this when trying to add the Gap block support to the Buttons block #34546, where it is much more apparent with rules like this:

// 0.5em is the current column-gap, kept for backwards-compatibility.
column-gap: var(--wp--style--block-gap, 0.5em)

On themes with the issue, in the editor the column-gap between buttons more than doubles in size, which can easily cause buttons to overflow to the next row and appear significantly different from the frontend.

@stacimc stacimc added the [Type] Bug An existing feature does not function as intended label Sep 28, 2021
@Mamaduka Mamaduka added the Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json label Sep 29, 2021
@andrewserong
Copy link
Contributor

Thanks for writing this up @stacimc! I think the culprit here is that the block_styles styles get rendered when a theme opts in to the experimental link color via add_theme_support( 'experimental-link-color' );. It looks like TwentyTwentyOne does that here and Seedlet does it here. Here's the conditional where global-styles.php causes the block styles to be loaded.

I wonder if a solution might be for us to see if we can guard generating the blockGap variable behind a check for the presence of the settings.spacing.blockGap setting? Possibly by extending get_block_classes to also accept the settings nodes, and then do the check in get_block_classes before calling compute_style_properties? Or, extend compute_style_properties to check settings, too?

@andrewserong
Copy link
Contributor

I've put together a PR looking at one particular way to try to handle it in #35209. Very happy for feedback on whether it looks viable!

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 [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants