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 new syntax for dynamic selectors #24423

Closed
wants to merge 1 commit into from

Conversation

oandregal
Copy link
Member

@oandregal oandregal commented Aug 6, 2020

Related:

Blocks that support the Global Styles system need a way to be converted to a CSS selector. At the moment, the way we do it is by using this algorithm:

  1. If the block doesn't define a __experimentalSelector within its block.json, the selector is built based on its name: .wp-block-${ blockName }.Example: core/group maps to .wp-block-group.

  2. If the block does define a __experimentalSelector key within its block.json and it is a string, the selector is the value of __experimentalSelector. Example: core/paragraph maps to p because it declares support by "__experimentalSelector": "p".

  3. If the block does define a __experimentalSelector key within its block.json and it is an object, the block has as many selectors as object entries. Example: core/heading can be core/heading/h1, core/heading/h2, ..., core/heading/h6 and each one has its own selector that is defined this way:

"__experimentalSelector": {
    "core/heading/h1": "h1",
    "core/heading/h2": "h2",
    "core/heading/h3": "h3",
    "core/heading/h4": "h4",
    "core/heading/h5": "h5",
    "core/heading/h6": "h6"
}

This works fine for the static nature of server-side global styles (it doesn't use the block values, only its description). However, for the dynamic nature of the client-side (particularly feature detection) we need a way to map the specific instance of a block to a particular selector. Example: the core/heading block when its level is 3 should be detected as core/heading/h3, while it should be detected as core/heading/h2 when its level is 2.

This PR proposes a new syntax for dynamic selectors:

"__experimentalSelector": {
    "attribute": "level",
    "expression": "h${attribute}",
    "range": ["h1", "h2", "h3", "h4", "h5", "h6"]
}

The attribute and expression would be used client-side (or in mobile) to construct the selector based on dynamic data (the value of the level attribute). The range would be used server-side to process the block.

Note that I haven't implemented the change yet because I want to gather thoughts first.

@oandregal oandregal self-assigned this Aug 6, 2020
@oandregal oandregal added [Feature] Blocks Overall functionality of blocks Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels Aug 6, 2020
@oandregal oandregal changed the title Try new syntax Try new syntax for dynamic selectors Aug 6, 2020
@github-actions
Copy link

github-actions bot commented Aug 6, 2020

Size Change: -3 B (0%)

Total Size: 1.15 MB

Filename Size Change
build/block-library/index.js 132 kB -3 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.67 kB 0 B
build/api-fetch/index.js 3.44 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 7.97 kB 0 B
build/block-directory/style-rtl.css 953 B 0 B
build/block-directory/style.css 952 B 0 B
build/block-editor/index.js 125 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.59 kB 0 B
build/block-library/editor.css 7.59 kB 0 B
build/block-library/style-rtl.css 7.76 kB 0 B
build/block-library/style.css 7.77 kB 0 B
build/block-library/theme-rtl.css 729 B 0 B
build/block-library/theme.css 730 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.3 kB 0 B
build/components/index.js 200 kB 0 B
build/components/style-rtl.css 15.7 kB 0 B
build/components/style.css 15.7 kB 0 B
build/compose/index.js 9.68 kB 0 B
build/core-data/index.js 11.8 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.45 kB 0 B
build/date/index.js 5.38 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.23 kB 0 B
build/edit-navigation/index.js 10.9 kB 0 B
build/edit-navigation/style-rtl.css 1.08 kB 0 B
build/edit-navigation/style.css 1.08 kB 0 B
build/edit-post/index.js 304 kB 0 B
build/edit-post/style-rtl.css 5.61 kB 0 B
build/edit-post/style.css 5.61 kB 0 B
build/edit-site/index.js 17 kB 0 B
build/edit-site/style-rtl.css 3.06 kB 0 B
build/edit-site/style.css 3.06 kB 0 B
build/edit-widgets/index.js 9.38 kB 0 B
build/edit-widgets/style-rtl.css 2.45 kB 0 B
build/edit-widgets/style.css 2.45 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/editor/index.js 45.3 kB 0 B
build/editor/style-rtl.css 3.8 kB 0 B
build/editor/style.css 3.79 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.72 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 621 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/keyboard-shortcuts/index.js 2.52 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.11 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.33 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 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.41 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 13.9 kB 0 B
build/server-side-render/index.js 2.71 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@jorgefilipecosta
Copy link
Member

Hi @nosolosw,
What if relate these CSS selectors to the block variations concept:

  1. We would only have simple selectors but block variations could have a CSS selector.
  2. For each heading level h1,h2,h3,etc... we would have a block variation, I guess we could add a mechanism to hide variations from the inserter to not pollute it.
  3. To identify if a block instance matches a selector if we would identify if the block instance matches a block variation.
  4. To know if a block instance matches a variation we could check if all the attributes defined in the variation have the same value in the current block instance.

@oandregal
Copy link
Member Author

Hey Jorge, for context, I'm sharing a little exploration I did about using block variations for this #22805

If I understood correctly this proposal, this is what would happen:

  1. The user adds the core/heading block as an h2 block variation. At this point, the attribute level is 2, and the selector is h2.
  2. The user changes the block level to be 3. At this point, the attribute level is 3, but the selector is still h2.

Is that correct? Let me share my raw thinking on this:

  • Do we serialize the selector value as h2?
  • How do we know that we should compute a selector (ex: in the case of heading) or use its value directly (ex: in the case of paragraph) if both are string selectors? I guess we check if the block has variations? If so, can a block define a variation but still use a selector directly without computing it?
  • What if a block defines variations with attributes other than the essential to the selector computation? Ex: let's say the core heading defines 6 variations (h1, ..., h6) and each one of them comes with a different value for level but also with a different value for style.fontSize. How do we know that we only need to take the level attribute into account to compute the selector?

I guess I'm struggling to see the value of using block variations in addition to the runtime value of attributes to compute the selector value. 🤔 Computing the selector exclusively from the runtime value of attributes seems simpler (conceptually and from an implementation point of view).

@jorgefilipecosta
Copy link
Member

Hi @nosolosw,

Regarding the possible alternative using the block variations API,

The user changes the block level to be 3. At this point, the attribute level is 3, but the selector is still h2.

No, in that case, the selector would be h3. The way to match if a variation is being used or not is not by what variation was inserted. We could use a rule for example a given variation is active if all the selectors the variation defines contain the same value in the current block instance.
We would not serialize any selector or have any static information stored.

I guess we check if the block has variations? If so, can a block define a variation but still use a selector directly without computing it?

We would only use a variation selector if the variation provides a selector, otherwise, we would normally use the block selector.

What if a block defines variations with attributes other than the essential to the selector computation? Ex: let's say the core heading defines 6 variations (h1, ..., h6) and each one of them comes with a different value for level but also with a different value for style.fontSize. How do we know that we only need to take the level attribute into account to compute the selector?

This case would be the most problematic with the variations approach. If that's the case I guess, one would need to specify some variations that are hidden, that just contain attributes relevant to compute the selector. But I guess this case would not be very common.

I guess I'm struggling to see the value of using block variations in addition to the runtime value of attributes to compute the selector value. 🤔 Computing the selector exclusively from the runtime value of attributes seems simpler (conceptually and from an implementation point of view).

The value of using variations is that block authors would not need to learn a new API with a custom syntax they would simply use the variations API that already exists and use the normal __experimentalSelector option that already exists for blocks.

@@ -23,13 +23,9 @@
"__experimentalFontSize": true,
"__experimentalLineHeight": true,
"__experimentalSelector": {
"core/post-title/h1": "h1",
Copy link
Member

Choose a reason for hiding this comment

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

It seems we have an issue both post-title and heading block map to the same CSS selectors so CSS output from one block will overwrite the CSS output of the other blocks. One post title the selector should probably also include a class.

@jorgefilipecosta
Copy link
Member

Regarding the API being proposed on this PR,

"__experimentalSelector": {
    "attribute": "level",
    "expression": "h${attribute}",
    "range": ["h1", "h2", "h3", "h4", "h5", "h6"]
}

What would happen if the way to compute the selector needed more than one attribute?
I guess our syntax could use the attribute name inline e.g: "h${level}", so we can reference multiple attributes.

What if we have a block that uses an isInline boolean attribute when true needs a "span.wp-block-x" selector and when false needs a "div.wp-block-x" selector. Or an isVideo that is used when we need a video element but otherwise we use an image selector?

Ideally, we would have some function that given the attributes returns the selector, but that would not allow us to know the selector on the server which may be needed in the future. So I think we should follow a path that allows us to know the selector on the server and the client.

@oandregal
Copy link
Member Author

Going to close this in favor of #26945 which is simpler!

@oandregal oandregal closed this Nov 13, 2020
@oandregal oandregal deleted the try/dynamic-experimental-selectors branch November 13, 2020 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks 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