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 to target specific selectors for Global Styles #20290

Draft
wants to merge 9 commits into
base: master
from

Conversation

@nosolosw
Copy link
Member

nosolosw commented Feb 18, 2020

Continues work from: #19883 #20047

This PR explores how we could target styles for selected parts of the DOM without an explosion of variables.

Context

In #19883 we had a design token tree with "global" and "blocks" variables. That was problematic for a few reasons: the number of variables would grow wildy + themes wouldn't be able to consolidate styles due to all kind of variables that could be registered by non-core blocks and they are unaware of.

In #20047 we simplified the design tokens tree to only hold global variables. That came with the trade-off of not being able to target selected parts of the DOM (ex: style all paragraph blocks at once).

This proposal allows for targeting specific parts of the DOM while only using general variables.

How would this work?

Via experimental-theme.json, themes would register a design tokens tree such as:

{
    "core": {
        "color": {
            "background": "#fff",
            "text": "#000"
        }
    },
    "core/paragraph": {
        "color": {
            "text": "aquamarine"
        },
        "typography": {
            "font-size": "var(--wp-theme-font-size-p)"
        }
    },
    "core/heading/h1": {
        "color": {
            "text": "hotpink"
        }
    },
    "core/heading/h2": {
        "color": {
            "text": "yellow"
        }
    }
}

which would be transformed into something akin to:

:root {
  --wp--color--background: #fff;
  --wp--color--text: #000;
}

p {
  --wp--color--text: aquamarine;
  --wp--typography--font-size: var(--wp-theme-font-size-p);
}

h1 {
  --wp--color--text: hotpink;
}

h2 {
  --wp--color--text: yellow;
}

The key here is using the CSS cascade to our advantage. Note that we only use one CSS Custom Property (--wp--color--text) but we define different values for different scopes (body, paragraphs, headings).

Note that, at the moment, the mapping from keys to selectors (core => :root, core/paragraph => p, etc) is taken from the block.json in a way that needs evolving. How blocks would opt-in into this is an open question.

How to test

  • Enable the FSE experiment.
  • Install and activate the demo theme available here (or create your own).
  • Load the site front-end. Verify that everything works as expected.
@nosolosw nosolosw requested review from ItsJonQ and jorgefilipecosta Feb 18, 2020
@nosolosw nosolosw self-assigned this Feb 18, 2020
@nosolosw

This comment has been minimized.

Copy link
Member Author

nosolosw commented Feb 18, 2020

When reviewing this, note that you should omit the 67c3079 commit. That's part of #20273 that I've only inlined here so it's easier to test.

lib/global-styles.php Outdated Show resolved Hide resolved
@ItsJonQ

This comment has been minimized.

Copy link
Contributor

ItsJonQ commented Feb 19, 2020

@nosolosw Haiii!!! 👋

Apologies for the late reply! My day was pretty packed yesterday and didn't get me many opportunities for code things. I did review this update early and let it marinate throughout the day.

These are my thoughts :)

I think we need a .wp-gs environment...

...or a mechanism that provides a scoping feature. The reason is that if the selectors are too generic, the styles will apply to unintended parts within Gutenberg:

Screen Shot 2020-02-18 at 9 03 06 PM

(Notice Site Editor at the top and Inspector on the side. These have been affected by the heading styles)

With a .wp-gs environment selector, we can better control the styles within Gutenberg. It's less applicable on the front-end of the user's site ✌️

Nested Block Styles

This was a tough one! By specifying custom selectors, I think (I may be wrong) it makes it more difficult to apply styles from a "container" block when we need to support nesting.

I think keeping the variables at the :root, and declaring custom values at DOM nodes, allows for predictable scoping to work (Bonus: It also follows the cascading model of CSS variables).

<ContainerBlock>
  <InnerBlock />
  <InnerBlock />
</ContainerBlock>
:root {
  --wp--core-paragraph: black;
}
.ContainerBlock {
  --wp--core-paragraph: red;
}
.InnerBlock {
  color: var(--wp--core-paragraph);
}

The above should allow for nesting to work nicely.

However, if the CSS variables were injected at a custom selector (as the original "root"), example:

.ContainerBlock {
  --wp--core-paragraph: red;
}
.InnerBlock {
  --wp--core-paragraph: black;
}

The customization at the Container level would not take effect.


Those are the only 2 thoughts I had with your recent exploration :).
I think it was a great effort 🙏 .

I can't put my finger on it... but there's something to the custom selectors feature that I really like. I think maybe useful and powerful later on as we continue to explore and expand Global Styles

@nosolosw

This comment has been minimized.

Copy link
Member Author

nosolosw commented Feb 20, 2020

We need a .wp-gs environment.

Certainly, we need to scope the styles within the editor. How about using the existing editor-styles-wrapper? At the moment the selector generator harcodes the values, but it can be easily taught how to add any wrapper we want.

Nested Block Styles

A few things here!

  • I think we should have all variables within :root, definitely. However, this proposal is about enqueuing some extra classes if and only if the theme adds the custom selectors. If they aren't declared, the classes won't be enqueued.

  • Your example looks fantastic to evaluate. My thinking is that it is a matter of education and what authors want to do. They'll be still required to know CSS best practices, the cascade, etc. There's no difference from CSS here. I think we can be mindful of the selectors we offer, but we're limited by how CSS works and the current state of blocks. A potential source of headaches is going to be the blocks that don't have a class we can use to target them (ex: there is no .wp-block-paragraph class, so the p selector is going to potentially interfere with the paragraphs in other blocks, no matter you use CSS or the style system). My point is that you have the same problem if you write CSS:

body { /* container */
  color: black;
}
p { /* inner block */
  color: red;
}

The example you shared also highlights the importance of focused selectors. My current thinking is that core should come with pre-defined selectors for each block and that selectors for third-party blocks should also be created when a block is registered. Theme authors would have them ready to use. Beyond those, it's on the authors to register the selectors they need at convenience -- I'm thinking of highly specific selectors like "target cite within a blockquote".

Finally, and perhaps the biggest issue I see with this proposal is how would we surface this to users? They are a few open questions here: how do we detect and expose in the UI the custom areas that are modifiable? Should users only be able to tweak the custom areas themes have declared or all the ones available? etc. I've seen the expectation that the system is able to do this (here, here, and here), so I guess is something that needs figuring out at the design level?

@nosolosw nosolosw force-pushed the try/global-styles-target-selectors branch from e3b98b9 to bc34183 Feb 27, 2020
@nosolosw

This comment has been minimized.

Copy link
Member Author

nosolosw commented Feb 27, 2020

Updated this to work with the global-style theme changes at WordPress/theme-experiments#22 and to use the scoped class .wp-site-blocks that is present with the FSE experiment enabled.

@github-actions

This comment has been minimized.

Copy link

github-actions bot commented Feb 27, 2020

Size Change: +41 B (0%)

Total Size: 889 kB

Filename Size Change
build/block-library/index.js 111 kB +41 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/annotations/index.js 3.4 kB 0 B
build/api-fetch/index.js 3.79 kB 0 B
build/autop/index.js 2.58 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.03 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 760 B 0 B
build/block-editor/index.js 102 kB 0 B
build/block-editor/style-rtl.css 10.2 kB 0 B
build/block-editor/style.css 10.2 kB 0 B
build/block-library/editor-rtl.css 7.22 kB 0 B
build/block-library/editor.css 7.22 kB 0 B
build/block-library/style-rtl.css 7.54 kB 0 B
build/block-library/style.css 7.55 kB 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/blocks/index.js 57.5 kB 0 B
build/components/index.js 195 kB 0 B
build/components/style-rtl.css 16.6 kB 0 B
build/components/style.css 16.5 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.04 kB 0 B
build/data/index.js 8.23 kB 0 B
build/date/index.js 5.36 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/index.js 2.75 kB 0 B
build/edit-navigation/style-rtl.css 279 B 0 B
build/edit-navigation/style.css 280 B 0 B
build/edit-post/index.js 92.9 kB 0 B
build/edit-post/style-rtl.css 12.3 kB 0 B
build/edit-post/style.css 12.3 kB 0 B
build/edit-site/index.js 10.1 kB 0 B
build/edit-site/style-rtl.css 5.02 kB 0 B
build/edit-site/style.css 5.02 kB 0 B
build/edit-widgets/index.js 7.17 kB 0 B
build/edit-widgets/style-rtl.css 3.74 kB 0 B
build/edit-widgets/style.css 3.73 kB 0 B
build/editor/editor-styles-rtl.css 399 B 0 B
build/editor/editor-styles.css 400 B 0 B
build/editor/index.js 42.8 kB 0 B
build/editor/style-rtl.css 3.49 kB 0 B
build/editor/style.css 3.49 kB 0 B
build/element/index.js 4.44 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 6.95 kB 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/i18n/index.js 3.57 kB 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/notices/index.js 1.57 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/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.84 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/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

// (3rd party blocks, custom selectors, etc).
$selectors = array(
'core' => ':root',
'core/paragraph' => '.wp-site-blocks p',

This comment has been minimized.

Copy link
@jorgefilipecosta

jorgefilipecosta Mar 31, 2020

Member

Should we try to move this to a more stable version e.g: make this selectors part of block.json so which block can configure its selector?

lib/global-styles.php Outdated Show resolved Hide resolved
@nosolosw nosolosw force-pushed the try/global-styles-target-selectors branch from bc34183 to 86e956b Apr 7, 2020

$selectors = array( 'core' => ':root' );

// We need to provide a mechanism for blocks to opt-in into this

This comment has been minimized.

Copy link
@nosolosw

nosolosw Apr 7, 2020

Author Member

@jorgefilipecosta I've tried here what you suggested (pulling the selectors from block.json), so that's working. However, we need a way for blocks to opt-in into this so we know which block.json to read and from where. I'm thinking we'd need some kind of server-side registration for blocks that want to opt-in into this, similar to what we do for dynamic blocks. cc @gziolo as I believe he was doing some work along the same lines.

nosolosw added 3 commits Apr 7, 2020
so remove unnecesary global.scss
@@ -171,22 +187,50 @@ function gutenberg_experimental_global_styles_get_theme() {
* @return string CSS rule.
*/
function gutenberg_experimental_global_styles_resolver( $global_styles ) {

This comment has been minimized.

Copy link
@youknowriad

youknowriad Apr 7, 2020

Contributor

I'm thinking instead of the automatic CSS variables generation we should rely on something very similar to this https://github.com/WordPress/gutenberg/pull/21428/files#diff-d30688729dfd9f0bba44ec6f9bfe6c57R43

This comment has been minimized.

Copy link
@nosolosw

nosolosw Apr 8, 2020

Author Member

I'm eager to experiment with that approach! I'm thinking this PR addresses a different problem (theme.json shape, theme.json to CSS conversion) and the exploration about CSS variables or not merits its own PR.

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

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.