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

Document experimental theme.json #22518

Merged
merged 8 commits into from
May 22, 2020
Merged

Document experimental theme.json #22518

merged 8 commits into from
May 22, 2020

Conversation

oandregal
Copy link
Member

@oandregal oandregal commented May 21, 2020

This PR intends to document the current direction and work in progress about how themes can hook into the various sub-systems that the Block Editor provides.

Read current status here.

@oandregal oandregal self-assigned this May 21, 2020
@oandregal oandregal added [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Type] Developer Documentation Documentation for developers labels May 21, 2020
@oandregal
Copy link
Member Author

I was thinking where to put this document and I felt the block editor handbook wasn't a proper place just yet. The reason is that this is in evolution so it'll change. Having a new page in the handbook requires a bit of setup (and more if we want to change later the name, etc), so I thought about starting in a more lightweight way.

@youknowriad
Copy link
Contributor

@nosolosw Thanks for starting this, it's great. I think it's fine to include in the handbook though, we already have experimental things there (like the block-based themes), we just need to be explicit about the state of things.

@github-actions
Copy link

github-actions bot commented May 21, 2020

Size Change: -19 B (0%)

Total Size: 1.12 MB

Filename Size Change
build/edit-post/index.js 302 kB -19 B (0%)
ℹ️ 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 3.39 kB 0 B
build/autop/index.js 2.83 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.93 kB 0 B
build/block-directory/style-rtl.css 790 B 0 B
build/block-directory/style.css 791 B 0 B
build/block-editor/index.js 105 kB 0 B
build/block-editor/style-rtl.css 10.9 kB 0 B
build/block-editor/style.css 10.9 kB 0 B
build/block-library/editor-rtl.css 7.17 kB 0 B
build/block-library/editor.css 7.17 kB 0 B
build/block-library/index.js 119 kB 0 B
build/block-library/style-rtl.css 7.48 kB 0 B
build/block-library/style.css 7.48 kB 0 B
build/block-library/theme-rtl.css 684 B 0 B
build/block-library/theme.css 686 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 190 kB 0 B
build/components/style-rtl.css 17.1 kB 0 B
build/components/style.css 17.1 kB 0 B
build/compose/index.js 9.24 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 771 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 3.11 kB 0 B
build/edit-navigation/index.js 6.63 kB 0 B
build/edit-navigation/style-rtl.css 857 B 0 B
build/edit-navigation/style.css 856 B 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.9 kB 0 B
build/edit-site/style-rtl.css 5.31 kB 0 B
build/edit-site/style.css 5.31 kB 0 B
build/edit-widgets/index.js 7.73 kB 0 B
build/edit-widgets/style-rtl.css 4.59 kB 0 B
build/edit-widgets/style.css 4.59 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.6 kB 0 B
build/editor/style-rtl.css 5.06 kB 0 B
build/editor/style.css 5.06 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.64 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 711 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

@oandregal
Copy link
Member Author

Ah, I just realized the block-based theme docs are also there, which are also "experimental".

Moving this under the handbook then.

@oandregal oandregal requested a review from mkaz as a code owner May 21, 2020 10:25
@oandregal oandregal changed the title Theme & Block Editor APIs Document experimental theme.json May 21, 2020
@oandregal oandregal added the Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json label May 21, 2020

This describes the current efforts to consolidate the various APIs into a single point: a `experimental-theme.json` file.

When this file is present and the Full Site Editing experiment is enabled, a few Block Editor mechanisms that are activated.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need to limit the theme.json to FSE?

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, so there are two things here:

  • The FSE experiment needs to be enabled for this to work. I liked that as it follows what the other experiments are doing. However, one alternative would be to enable this if there's an experimental-theme.json, independently of the FSE experiment status. Would that make more sense? I guess if a theme provides that file it knows what's doing, so it should be fine. I don't have a strong opinion on this.

  • The stylesheet is only enqueued for front end & site editor. I wondered if we could open this to the other editors as well (post & widgets).

Copy link
Contributor

Choose a reason for hiding this comment

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

However, one alternative would be to enable this if there's an experimental-theme.json, independently of the FSE experiment status. Would that make more sense?

Yes, that was my thinking, I see value for this for regular themes too.

The stylesheet is only enqueued for front end & site editor. I wondered if we could open this to the other editors as well (post & widgets).

Definitely, especially "post". What's the reasoning for loading this in the site editor page only?

Copy link
Member Author

Choose a reason for hiding this comment

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

Draft PR to open this up #22520

What's the reasoning for loading this in the site editor page only?

Just being extra-cautious about which contexts had this loaded. With the current direction (enable features, etc), it makes more sense to open it up.

gradient: [ ... ],
},
styles: { ... },
features: { ... }
Copy link
Contributor

Choose a reason for hiding this comment

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

I still have a small concern about the "presets" and "features".

For instance to disable colors entirely you'd have to do:

presets : {
   color: []
},
features: {
   customColors: false,
}

So two separate locations to "control" the editor.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I agree, that'd be not ideal. This is not implemented yet but the way I imagine this could work is by compressing it to:

presets : {
   color: false
}

Copy link
Contributor

@youknowriad youknowriad May 21, 2020

Choose a reason for hiding this comment

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

it's still two different top level keys to disable a feature though. it's not the value itself that bothers me.

@@ -0,0 +1,219 @@
# Themes & Block Editor: Experimental theme.json

Copy link
Contributor

@youknowriad youknowriad May 21, 2020

Choose a reason for hiding this comment

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

Actually, we should add a sentence here to clarify the experimental state of the APIs and that they're subject to change at any moment (like the block-based theme doc)

Copy link
Member Author

Choose a reason for hiding this comment

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

How about 7f82de8 ? Mostly copied verbatim from the block-themes page.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Is that the link you intended to share? It points to this same thread so I'm not sure I follow.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, it's the comment bout the FSE requirement :)

@youknowriad
Copy link
Contributor

Ship it

@ZebulanStanphill
Copy link
Member

No idea where these questions should be asked, so please let me know if this is the wrong spot.

Anyway, I checked out theme-json.md, and overall, it looks quite nice. But I have a few questions about the format:

{
  global: {
    features: {
      dropCap: false
    }
  },
  core/paragraph: {
    features: {
      dropCap: true,
    }
  }
}

Should all the block contexts really be at the top level like this? I would think that they would all be under a "blocks" key. It would certainly help to organize the file (and perhaps optimize it for parsing) if additional kinds of contexts were added in the future, for example:

{
  global: {
    features: {
      dropCap: false
    }
  },
  blocks: {
    core/paragraph: {
      features: {
        dropCap: true,
      }
    }
  },
  somethingElse: {
    ...
  }
}

Or are we only ever going to have the global and the block contexts?

/* core/heading/h1, core/heading/h2, etc */
  core/heading/h*: {
    styles: {
        line-height: <value>,
        font-size: <value>,
        color: <value>,
    }
  },

How does this sub-namespacing work? Should this even be a thing? At first glance, it looks like a bad idea to me. If themes target only h* elements inside Heading blocks, they'll be missing any 3rd-party blocks that use the h1-h6 elements.

@oandregal oandregal merged commit 351591f into master May 22, 2020
@oandregal oandregal deleted the add/themes-block-editor branch May 22, 2020 08:31
@github-actions github-actions bot added this to the Gutenberg 8.2 milestone May 22, 2020
@oandregal
Copy link
Member Author

Hey Zeb, this is a fine place to comment on this.

The shape of theme.json: this is just another iteration, it has already changed from the initial shape and it'll probably change. The next step that will inform how the shape will change is probably nested content: container blocks, templates, patterns, etc. One thread where we discussed some alternatives is this one.

As for the second question: one of the next steps is to enable 3rd party blocks to register their own selectors. Note that theme.json intends to be more semantic than CSS: it targets blocks, not HTML elements.

@youknowriad
Copy link
Contributor

if additional kinds of contexts were added in the future

This is a good point.

@oandregal
Copy link
Member Author

I agree it's one of the options we have. It's in line with what Jorge brought up here.

In that line of thinking, we could go hierarchical to encode different entities:

{
 blocks: { ... },
 templates: { ... }
}

However, we could also go this way:

{
  block/core/paragraph: { ... },
  template/core/footer: { ... }
}

My point is that when only encoding top-level entities any approach could really work. Neither of them descend too many nested levels, so most are human-friendly and maintainable. The rubicon we need to cross is nested content. Until that point, I'd say everything is open.

As an illustration to this: an exercise that I think is valuable is to try capturing the existing numbered pattern with the proposed spec for theme.json. It's a simple pattern that only uses paragraphs and the columns block. In my early attempts to capture it, I needed 3 nesting levels, which, with a hierarchical approach becomes unwieldy pretty quickly. That's why I want to keep an open mind about the next steps.

Comment on lines +199 to +200
/* core/heading/h1, core/heading/h2, etc */
core/heading/h*: {
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to be exceptional behaviour in that it breaks the global|<blockName> model as defined earlier in the document. Can we clarify how this will work, how and whether block types can define these exceptions?

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, this is something that needs untangling. I see a few things here. They are related, although can be separate discussions as well. Here's the current direction and some thoughts about it:

  1. There are blocks that represent multiple HTML elements.

The heading block represents 6 HTML elements (h1-h6). That variation is captured as a block attribute, levels. I don't know any other block that does the same, although I can see potential for more cases. For example, a block that wanted to use <a> or <button> depending on some attribute.

There's another aspect here that I'm unsure how/whether plays with this: block variations. As far as I've seen, the block variations in use don't represent structural changes, but, they could if the block author uses them for that.

  1. How blocks (that represent multiple HTML elements) should register their selectors.

How any block register its selector is not documented. An old iteration of the managed CSS PR proposed using a new key selector within the block.json. At the moment, this data is hardcoded until we're able to register blocks in the server and consolidate the mechanism.

This is how the proposal looked like for the paragraph block:

// block.json
{
  "name": "core/paragraph",
  "selector": "p"
}
// theme.json
{
  'core/paragraph': {
      styles: ...
  }
}

The global styles mechanism knew how to match both pieces of data, the theme.json block name to the block.json selector.

This is fine for most blocks, but then we face blocks that represent multiple HTML elements, such as heading. This is what I proposed at the time:

// block.json
{
  "name": "core/heading",
  "selector": {
    "h1": "h1",
    "h2": "h2",
    "h3": "h3",
    "h4": "h4",
    "h5": "h5",
    "h6": "h6"
  }
}
// theme.json
{
  'core/heading/h1': {
      styles: ...
  },
  'core/heading/h2': {
      styles: ...
  },
  ...
}

Essentially, each block.json selector key creates a new block name in theme.json: core/heading/h1, core/heading/h2, etc. From the block.json perspective, I don't see many alternatives that fit with the "selector metaphor" nicely. There's something to say about the theme.json structure, though.

  1. How theme authors target blocks that represent multiple HTML elements in theme.json

The current proposal is:

// theme.json
{
  'core/heading/h1': {
      styles: ...
  },
  'core/heading/h2': {
      styles: ...
  },
  ...
}

However, we could do differently. Some alternatives that I've considered are:

  • using a new key
// theme.json
{
  'core/heading': {
    tag: 'h1',
    styles: ...
  },
  'core/heading': {
    tag: 'h2',
    styles: ...
  }
}
  • using nesting levels
{
  'core/heading': {
    h1: {
        styles: ...
    }
    h2: {
        styles: ...
    },
    ...
  }
}

The current proposal seemed clearer to me, both from the human and computer perspective.

Does this create any new thoughts/heps to advance the conversation?

Copy link
Member

Choose a reason for hiding this comment

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

As far as I've seen, the block variations in use don't represent structural changes, but, they could if the block author uses them for that

It's not exposed through any visual option yet, but the Group block has an attribute for changing which tag it uses. (So you could use it for <section>s or <aside>s.)

I think it's worth noting that, at least at some point (I don't know if it's still the case), it was thought that the block editor could be used in contexts where the output isn't HTML. I wonder if maybe some of the proposed ideas may be too HTML-specific. I suppose heavy reliance on HTML also causes problems for the mobile app.

As for the Heading block, I really don't think it makes sense to target h1-h6 elements through it. h1-h6 elements can and often will occur outside of Heading blocks. Theme authors ought to target h1-h6 elements as they normally would.

It's worth noting that there are variations in how blocks should be styled that depend, not on style variations, but an attribute that toggles whether a certain CSS class is added or not. The Latest Posts block has a grid and a list mode. The two should be styled very differently. But you can't use target either using style variations, because they aren't style variations. But the two modes also don't use different markup, if I recall correctly. The only difference is what CSS classes are applied.

But for a CSS class that is unique to a particular block, I don't think we should be targeting that class name in theme.json. I think instead, it would make more sense to target the block attribute. (This would have the benefit of not being CSS-specific, and it would allow us to change the specific class name used, since the theme.json wouldn't be referencing the class directly.)

I'm not sure exactly how/if this would work/look in practice. Just an idea I thought I'd share.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the group nudge, Zebulan. It led me to find another block that represents multiple HTML elements: the list block can use ul or ol.

For most blocks we have more specific selectors through the named class wp-block-{blockname}. However, there are a few for which we don't (paragraph, heading, list, and perhaps more) so we have to target the element. This is a source of headaches, but we also don't have another way that's back-compatible (unless we re-write block markup upon rendering the post).

As for not optimizing for HTML: I'd say that expressing the block attributes as a pair of property/value that is decoupled from the output (CSS) runs with that idea and may be helpful for other environments as well. Granted, right now, there's still a coupling between the block style attributes and CSS properties --- it doesn't need to be this way, though.

I'd put the latest post example in the same category of style variations. It seems the direction we're going towards is that style variations should be different sets of values for block style attributes (and not classes). Implementation-wise, we're far from that idea, but I have it in my background as something to explore.

Copy link
Member Author

Choose a reason for hiding this comment

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

After some PRs that have landed, I was able to put together #22698 which takes the block data (block selectors + block implicit attributes) from the block registry instead of being hardcoded.

Copy link
Member Author

Choose a reason for hiding this comment

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

As I mentioned above, I've looked at the intersection of block variations and block selectors at #22805 (comment) TLDR: those two things seem to be independent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Developer Documentation Documentation for developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants