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

Global Styles: use CSS variables to mark block style hooks #22317

Closed
wants to merge 1 commit into from

Conversation

oandregal
Copy link
Member

@oandregal oandregal commented May 13, 2020

Continues work from: #19883 #20047
Alternative to #20290
See larger picture at #22296

This PR takes a different stance than #20290 so should be considered an alternative. Note that this is a Proof of Concept, to share direction and gather feedback. The idea is that we take the implicit attributes declared from blocks and make them CSS variables bound to that block.

Input. Let's say the paragraph block declares through its settings that it has support for some implicit attributes and also provides a CSS selector:

__experimentalSelector: 'p',
supports: {
    __experimentalColor: true,
    __experimentalLineHeight: true,
    __experimentalFontSize: true,
},

Output. We enqueue a new embedded stylesheet for that block that looks like this:

p {
    color: var(--wp-color);
    line-height: var(--wp-line-height);
    font-size: var(--wp-font-size);
}

Unlike #20290 this doesn't try to provide a way to "manage CSS" so it's the theme responsibility to set the values of the variables for the contexts in which a block can be used (top-level document, within a pattern, etc).

A theme should do something along this lines:

p {
  --wp-color: <value>,
  --wp-line-height: <value>
  --wp-font-size: <value>
}

// Different values for a paragraph within the numbered features pattern
.wp-block-column:first-child p {
  --wp-color: <value>,
  --wp-line-height: <value>
  --wp-font-size: <value>
}

@oandregal oandregal self-assigned this May 13, 2020
@oandregal oandregal added the Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json label May 13, 2020
@youknowriad
Copy link
Contributor

p {
    color: var(--wp-color);
    line-height: var(--wp-line-height);
    font-size: var(--wp-font-size);
}

Why not just?

p {
  color: "something"; // The value defined in theme.json
  line-height: "something";
  font-size: "something";
}

I'm asking cause when we originally tried the CSS variables approach for these styles (color, background, gradient), we had cascade issues, applying a gradient to a container applies it to all its children.

Not saying we shouldn't use CSS variables at all but I'm thinking more that they are more useful for what I call the "branding configs" (link-color, gridd-size-unit...)

@github-actions
Copy link

Size Change: 0 B

Total Size: 828 kB

ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 4.08 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.59 kB 0 B
build/block-directory/style-rtl.css 764 B 0 B
build/block-directory/style.css 764 B 0 B
build/block-editor/index.js 104 kB 0 B
build/block-editor/style-rtl.css 10.6 kB 0 B
build/block-editor/style.css 10.6 kB 0 B
build/block-library/editor-rtl.css 7.12 kB 0 B
build/block-library/editor.css 7.12 kB 0 B
build/block-library/index.js 115 kB 0 B
build/block-library/style-rtl.css 7.38 kB 0 B
build/block-library/style.css 7.38 kB 0 B
build/block-library/theme-rtl.css 683 B 0 B
build/block-library/theme.css 685 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.1 kB 0 B
build/components/index.js 181 kB 0 B
build/components/style-rtl.css 17 kB 0 B
build/components/style.css 16.9 kB 0 B
build/compose/index.js 6.66 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.42 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 3.1 kB 0 B
build/edit-navigation/index.js 5.59 kB 0 B
build/edit-navigation/style-rtl.css 618 B 0 B
build/edit-navigation/style.css 617 B 0 B
build/edit-post/index.js 28 kB 0 B
build/edit-post/style-rtl.css 12.2 kB 0 B
build/edit-post/style.css 12.2 kB 0 B
build/edit-site/index.js 12.1 kB 0 B
build/edit-site/style-rtl.css 5.22 kB 0 B
build/edit-site/style.css 5.22 kB 0 B
build/edit-widgets/index.js 8.37 kB 0 B
build/edit-widgets/style-rtl.css 4.69 kB 0 B
build/edit-widgets/style.css 4.69 kB 0 B
build/editor/editor-styles-rtl.css 425 B 0 B
build/editor/editor-styles.css 428 B 0 B
build/editor/index.js 44.3 kB 0 B
build/editor/style-rtl.css 5.07 kB 0 B
build/editor/style.css 5.08 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 734 B 0 B
build/format-library/index.js 7.63 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 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.13 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 5.29 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 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.56 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.85 kB 0 B
build/rich-text/index.js 14.8 kB 0 B
build/server-side-render/index.js 2.68 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.02 kB 0 B
build/viewport/index.js 1.84 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@youknowriad
Copy link
Contributor

For your particular use-case (tweaking the number of a features pattern) I'd solve it like that:

Register a "number" style variation for the paragraph block with the following style:

p.is-style-number {
   // do  somethihg
  color: "something";
}

and on the pattern markup apply that style variation (I'm assuming the pattern comes from the theme itself). If the pattern doesn't come from the theme itself, I don't see any way of solving the issue, since it's not conceivable that a theme would style all patterns available (imagine a repository of patterns).

@oandregal
Copy link
Member Author

Why not just? (example without binding implicit atts as CSS properties).

The reason is that this doesn't solve the specificity issues, so themes will still have to try to "win" over core declarations if any. It'll be like #20290 but without addressing nested content. I'm happy to try that approach, but my feeling is that it's not enough. Which takes me to the question I posed in the other thread: from a theme author perspective, what'd be the advantage of using theme.json in this case over using regular CSS?

I'm asking cause when we originally tried the CSS variables approach for these styles (color, background, gradient), we had cascade issues, applying a gradient to a container applies it to all its children.

Yeah, I understand that and I'm glad that you brought those issues up. That's mainly why I explored #20290 without CSS variables!

@youknowriad
Copy link
Contributor

Yeah, I understand that and I'm glad that you brought those issues up. That's mainly why I explored #20290 without CSS variables!

I'm not sure how #20290 is any different in that regard though, you still need to know what properties are made available by blocks and patterns that come from the repositories (am I missing something)

@oandregal
Copy link
Member Author

oandregal commented May 13, 2020

For your particular use-case (tweaking the number of a features pattern) I'd solve it (using style variations).

I like that and actually thought about it myself. The problem I see with this approach is that we need to create multiple styles for the block paragraph to cover every pattern, cluttering the UI. Also: when you change themes, everything is messed up if the variation comes from the theme itself.

One potential way out would be if we were able to "filter" style variations by block and context so they're only shown in particular contexts. Not sure I like this idea, but talking out loud in case it moves the conversation forward.

@youknowriad
Copy link
Contributor

youknowriad commented May 13, 2020

Which takes me to the question I posed in the other thread: from a theme author perspective, what'd be the advantage of using theme.json in this case over using regular CSS?

Same answer: The main advantage is for the "global" config. you'd be able to define properties that work across blocks without knowing how these blocks will use them (especially useful for the branding configs)

),
'core/columns' => array(
'supports' => array( 'color' ),
'selector' => '.wp-block-columns',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be ok to use class selectors as a default and not provide the selectors explicitly?

@jorgefilipecosta
Copy link
Member

Is this PR still relevant? It seems #20290 also does what is being done here.

@oandregal
Copy link
Member Author

Yeah, this is different in that it "marks" the blocks with CSS variables, but it looks like #20290 has got enough momentum and support, so we can close this for now.

@oandregal oandregal closed this May 15, 2020
@oandregal oandregal deleted the try/block-style-hooks branch May 15, 2020 09:49
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.

3 participants