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

Try/enable global styles #21346

Closed
wants to merge 4 commits into from
Closed

Try/enable global styles #21346

wants to merge 4 commits into from

Conversation

oandregal
Copy link
Member

@oandregal oandregal commented Apr 2, 2020

Extracted from #20530 (comment)

This PR enables the Global Styles mechanism (inline the CSS properties) without requiring theme support or the FSE experiment.

How to test

  1. Test that it works without FSE for themes that don't declare support:
  • Make sure you have the FSE experiment disabled in the Gutenberg plugin.
  • Load this PR with a theme that doesn't declare support (doesn't have a experimental-theme.json.
  • Verify that the following style rule is enqueued in both front-end and back-end:
:root {
  --wp--color--background: #fff;
  --wp--color--text: #000;
  --wp--typography--line-height: 1.5;
}
  1. Test that it works without FSE for themes that declare support:
  • Make sure you have the FSE experiment disabled in the Gutenberg plugin.
  • Load this PR with a theme that declares support.
  • Verify that a proper style rule is enqueued in both front-end and back-end with the values provided by the theme of your choice. You can use this demo theme, if that's useful.
  1. Test that there are no leaks to the sidebars.
  • Install and activate a theme that has support for global styles (you can use this one).
  • Modify the values in experimental-theme.json to be something really gross: red for colors, 5 for line-height, etc.
  • Verify that sidebars are fine (document settings & block inspectors). Note that leaks may happen to p, h2, and h3 elements. Some that you may check are:
    • block inspector: block title => h2
    • document settings: view post within permalink section => h3
    • block inspector: text below the drop cap in text settings section => p

@oandregal oandregal self-assigned this Apr 2, 2020
@oandregal oandregal added the Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json label Apr 2, 2020
@github-actions
Copy link

github-actions bot commented Apr 2, 2020

Size Change: +1.49 kB (0%)

Total Size: 885 kB

Filename Size Change
build/block-directory/index.js 6.03 kB -1 B
build/block-editor/index.js 102 kB +126 B (0%)
build/block-editor/style-rtl.css 10.6 kB -611 B (5%)
build/block-editor/style.css 10.6 kB -611 B (5%)
build/block-library/editor-rtl.css 7.22 kB +6 B (0%)
build/block-library/editor.css 7.22 kB +7 B (0%)
build/block-library/index.js 110 kB +96 B (0%)
build/block-library/style-rtl.css 7.5 kB +2 B (0%)
build/block-library/style.css 7.51 kB +4 B (0%)
build/blocks/index.js 57.5 kB -1 B
build/components/index.js 195 kB -4 B (0%)
build/components/style-rtl.css 16.6 kB +506 B (3%)
build/components/style.css 16.5 kB +499 B (3%)
build/edit-navigation/index.js 2.71 kB +228 B (8%) 🔍
build/edit-post/index.js 92.5 kB +229 B (0%)
build/edit-post/style-rtl.css 12.1 kB +74 B (0%)
build/edit-post/style.css 12.1 kB +75 B (0%)
build/edit-site/index.js 9.78 kB +683 B (6%) 🔍
build/edit-site/style-rtl.css 4.71 kB +88 B (1%)
build/edit-site/style.css 4.71 kB +89 B (1%)
build/editor/index.js 42.8 kB -3 B (0%)
build/element/index.js 4.44 kB +1 B
build/format-library/index.js 6.95 kB +1 B
build/i18n/index.js 3.57 kB +1 B
build/notices/index.js 1.57 kB +1 B
build/priority-queue/index.js 789 B +9 B (1%)
build/redux-routine/index.js 2.84 kB +1 B
build/warning/index.js 1.14 kB -1 B
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/annotations/index.js 3.45 kB 0 B
build/api-fetch/index.js 3.8 kB 0 B
build/autop/index.js 2.59 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 760 B 0 B
build/block-library/theme-rtl.css 669 B 0 B
build/block-library/theme.css 671 B 0 B
build/block-serialization-default-parser/index.js 1.65 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/compose/index.js 6.21 kB 0 B
build/core-data/index.js 10.7 kB 0 B
build/data-controls/index.js 1.03 kB 0 B
build/data/index.js 8.23 kB 0 B
build/date/index.js 5.37 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.05 kB 0 B
build/edit-navigation/style-rtl.css 239 B 0 B
build/edit-navigation/style.css 241 B 0 B
build/edit-widgets/index.js 4.43 kB 0 B
build/edit-widgets/style-rtl.css 3.74 kB 0 B
build/edit-widgets/style.css 3.74 kB 0 B
build/editor/editor-styles-rtl.css 423 B 0 B
build/editor/editor-styles.css 426 B 0 B
build/editor/style-rtl.css 3.49 kB 0 B
build/editor/style.css 3.49 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 1.93 kB 0 B
build/html-entities/index.js 622 B 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.3 kB 0 B
build/keycodes/index.js 1.7 kB 0 B
build/list-reusable-blocks/index.js 2.99 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 4.84 kB 0 B
build/nux/index.js 3.01 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.54 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/rich-text/index.js 14.5 kB 0 B
build/server-side-render/index.js 2.54 kB 0 B
build/shortcode/index.js 1.69 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.01 kB 0 B
build/viewport/index.js 1.6 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@johnstonphilip
Copy link
Contributor

@nosolosw This is working for me. To clarify, it works even if a theme does not declare any support.

You mentioned:
Load this PR with a theme that declares support.

Is a theme that declares support supposed to be required?

@oandregal
Copy link
Member Author

@johnstonphilip thanks for testing! Yeah, I suggest testing with both a theme that doesn't declare support (to highlight how core settings would be still available) and with one that has support (to highlight how themes can override them).

Added a numbered list, hoping it makes the testing instructions clearer.

@oandregal
Copy link
Member Author

We're reverting the addition of CSS variables at the block level at #21428 so this PR no longer makes sense as it is, so I'm closing it for the time being.

@oandregal oandregal closed this Apr 8, 2020
@oandregal oandregal deleted the try/enable-global-styles branch April 8, 2020 08:53
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants