Skip to content

/ gutenberg

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

Jump to bottom

Make global styles available to all themes #34334

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from
Open

Make global styles available to all themes #34334

wants to merge 2 commits into from

Conversation

@oandregal
Copy link
Member

@oandregal oandregal commented Aug 26, 2021 •
edited

Fixes #34328

This PR makes global styles available to all themes.

  1. Themes without theme.json and no experimental-link-color support either: the stylesheet contains the core presets.
  2. Themes without theme.json but experimental-link-color support: the stylesheet contains the core and theme presets.
  3. Themes with theme.json: the stylesheet contains the core and theme presets, plus the result of merging core & theme styles.

How to test

Activate TT1-blocks: a theme with theme.json support. Visit the front end. Check the content of global-styles-inline-css is:

  • Both core & theme presets (classes and variables).
  • The block gap styles (see).
  • The theme styles.

Activate the TwentyTwentyOne theme: a theme without theme.json but with experimental-link-color support. Visit the front end. Check the content of global-styles-inline-css is:

  • Both core & theme presets (classes and variables).

Activate the TwentyTwenty theme: a theme without theme.json and no experimental-link-color support. Visit the front end. Check the content of global-styles-inline-css is:

  • Core presets (classes & variables).

Follow-ups

Remove the styles included here from the block-library package.
@oandregal
Make global styles available to all themes
f7a1387 
They just differ on what data they have:

1. Without theme.json and no experimental-link-color support either:
   the stylesheet contains the core presets & styles.

2. Without theme.json but experimental-link-color support:
   the stylesheet contains the core and theme presets,
   plus the core styles if any.

3. With theme.json:
   the stylesheet contains the core and theme presets,
   plus the result of merging core & theme styles.
@oandregal oandregal self-assigned this Aug 26, 2021
@oandregal
Copy link
Member Author

@oandregal oandregal commented Aug 26, 2021 •
edited

@youknowriad I'm seeing this piece of CSS in addition to the presets output for TwentyTwentyOne and TwentyTwenty. I presume this doesn't have to be enqueued unless there's a theme.json file?

body {
  --wp--style--block-gap: 24px;
}
.wp-site-blocks > * + * {
  margin-top: var(--wp--style--block-gap);
  margin-bottom: 0;
}

@oandregal oandregal requested a review from youknowriad Aug 26, 2021
@oandregal
Do not enqueue block styles if theme does not have theme.json
a59775a
@oandregal
Copy link
Member Author

@oandregal oandregal commented Aug 26, 2021

ok, removed the block gap styles for TwentyTwentyOne and TwentyTwenty.

@oandregal oandregal marked this pull request as ready for review Aug 26, 2021
@oandregal oandregal requested a review from spacedmonkey as a code owner Aug 26, 2021
@jffng
jffng reviewed Aug 26, 2021
View changes
Copy link
Contributor

@jffng jffng left a comment

The test cases work for me as described.

ok, removed the block gap styles for TwentyTwentyOne and TwentyTwenty.

There may be a harmless edge case here:

  1. Activate a theme with a theme.json
  2. Make a post/page and add a block that inherits layout
  3. Switch to a theme without a theme.json
  4. Gutenberg will still generate alignment styles for those containers, but --wp--style--block-gap will be undefined.
@oandregal
Copy link
Member Author

@oandregal oandregal commented Aug 31, 2021

@jffng That seems to happen in the transition from a theme with theme.json to a theme without theme.json, so it happens with or without this PR. I'd think it's unrelated to this.

This was referenced Aug 31, 2021
Open
Draft
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jffng jffng

@youknowriad youknowriad

@spacedmonkey spacedmonkey

@aristath aristath

@carolinan carolinan

@kjellr kjellr

@scruffian scruffian

@MaggieCabrera MaggieCabrera

At least 1 approving review is required to merge this pull request.

Assignees

@oandregal oandregal

Labels
Global Styles
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

Make the global stylesheet available for all themes
2 participants
@oandregal @jffng