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]: Style Engine: Alternative approach to rendering common layout styles from presets #39374

Conversation

andrewserong
Copy link
Contributor

What?

🚧 🚧 🚧 🚧 🚧 WIP: This is a messy exploratory PR that is highly likely to change 🚧 🚧 🚧 🚧 🚧

This is a very early draft exploration alternative to #38974 to look at potentially trying to generate common layout styles via presets in theme.json.

The main objective is to hook into the theme.json style generation for invoking generating styles for layouts.

Why?

Based on discussion in #38974, we'd like to look into a presets approach for generating common layout styles, that individual blocks can then link to via classname.

How?

Currently it adds an additional preset metadata item for layouts, and then iterates over those layout types — there's an escape hatch from the usual global styles generation that then goes to call a function from the Layout block support to generate the styles. This then uses a similar mechanism as to #38974 to generate the styling rules, and then passes it back to the theme JSON class, so that those styling rules can be included in the global stylesheet.

This is a very naive approach at the moment, and it is not close to a working example.

Testing Instructions

Screenshots or screencast

Styles included in the global styles sheet:

image

@@ -25,6 +25,108 @@ function gutenberg_register_layout_support( $block_type ) {
}
}

function gutenberg_generate_common_flow_layout_styles() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: not all of the common styles have been added yet. In its current state, this PR is just a proof of concept that we can pass the rendered styles back up to the global styles stylesheet.

@ramonjd
Copy link
Member

ramonjd commented Mar 11, 2022

Thanks for looking into this. I'm still learning about presets and their usages so it's really helpful to see how you're using the available tools to output layout styles. 🙇

I'm thinking out loud unfiltered here so it might be all babble 😄

I'm assuming, based on a reading of the docs, that we're hooking into the presets functionality to generate the styles, and ultimately define the available layouts in the UI controls, but we're not really creating a new "preset" category per se given there are no 'values' (or 'presets') that we can configure in the theme.json (?)

I might be reading it all too literally or not understanding things. I was just curious to know if we would sell layouts as a preset, or rather more as a block supports-type setting that informs the application about which layouts it is allowed to expose in the editor.

Would all block-level customization styles – and there could be many depending on how many layouts we offer and their corresponding properties – be piped into the global-styles-inline-css style sheet? Or just the high-level "common styles" that are the base requirements for each layout? I'm mainly thinking about where variations, custom widths and so on, might live. Perhaps these could be the obfuscated classnames.

Also would theme authors want to be able to define global values for certain layout properties? For example:

	....
	"styles": {
		"blocks": {
	              ....
		},
		"elements": {
	              ....
		},
		"layout": {
	              "flex": {
                           "gap": "24px",
                           "align": "left"
                       },
	              "stack": {
                           "gap": "24px"
                       },
	              "grid": {
                           "gap": "24px"
                       },
		},
		"spacing": {
	              ....
		},
		"typography": {
	              ....
		}
	},

Sorry for all the questions! I know many might not yet have answers, but I just wanted to get them down.

🍺

@andrewserong
Copy link
Contributor Author

Sorry for all the questions!

Thanks for taking a look @ramonjd, much appreciated! No apologies necessary, these are just the sorts of questions that are good to ask at this stage!

but we're not really creating a new "preset" category per se given there are no 'values' (or 'presets') that we can configure in the theme.json

Not yet, but it does open up the possibility to do that if we wanted to explore it (which may or may not be a side-track from our goals of style consolidation). With this PR I was mostly curious to see how hard it'd be to hook into the global styles style generation and pass off the logic for constructing the styles to somewhere else (in this case the Layout block support). But the question of how much we use of the presets function is a great one — because we can pass along the preset metadata array (from the theme JSON class) and the preset data from the theme.json file itself, it means we've got a bit more data we could potentially play with for constructing styles.

So, the question of how much we want to expose of layout settings, not just at the block level, but within theme.json is a good one! It occurred to me that we could always customise the styles we return within gutenberg_generate_common_flow_layout_styles based on the data we receive, so there's an opportunity for exploration there.

Or just the high-level "common styles" that are the base requirements for each layout?

I was thinking that (at least at first), we'd just be focusing on outputting the "common styles" or those styles with a fixed set of options (like how alignments don't have an infinite number, maybe only a small handful of classes), and we'd then defer the highly specific values (like content width or gap value) to the rendering at the individual block level like in #38974 — and we can then make the choice between whether the classname reads in a semantic way, or if it's more obfuscated.

The key part to getting this PR working in a useful way, is to still add in generating the classnames to inject at the block level when rendering the layout block support, so that the individual blocks hook up to the classnames we've generated in the global styles sheet. I thought I'd probably play with that next.

But that's just what I had in mind, though, very happy to go in any other direction if anyone has strong feelings on it. And feel free to jump in on this PR if you'd like to hack around of course! I'll continue having a play with it from sometime mid next week 😄

Copy link
Contributor

@tellthemachines tellthemachines left a comment

Choose a reason for hiding this comment

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

Thanks for getting this started! I mainly have high-level questions at this point 😄

but we're not really creating a new "preset" category per se given there are no 'values' (or 'presets') that we can configure in the theme.json (?)

This is an interesting one. We could let themes add preset values for flow width and flex gap. But are presets only valid when they are theme-configurable, or can we also have purely internal ones, in which case, potentially all the flex justification and orientation options could be presets?

Another thought, though maybe not immediately relevant: will there ever be a scenario where layout type can be picked by user? And if so I'm wondering if that might have an impact on this architecture.

);
}

function gutenberg_generate_common_flex_layout_styles() {
Copy link
Contributor

Choose a reason for hiding this comment

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

By common styles, do we mean only the styles that are shared across all flex variations (e.g. vertical orientation +justified center, or horizontal+space between), or will all the variations themselves be added in here too?

Copy link
Member

Choose a reason for hiding this comment

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

I could be mistaken, but I understood "common" to be the "fixed" styles required for a layout to work, so common to every container that uses a particular layout, e.g., if a bunch of Group Blocks use a flex layout, then the "common style" is display: flex for flex.

Does that sound legit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understood "common" to be the "fixed" styles required for a layout to work, so common to every container that uses a particular layout, e.g., if a bunch of Group Blocks use a flex layout, then the "common style" is display: flex for flex.

Yes, that's what I was thinking of, too — common in the sense that they're styles that will be output to the global stylesheet irrespective of the blocks contained in the current post that's being rendered. (If we settle on an approach, then there'll be some a fair bit of documentation / doc comments to add!)

In that category, I was imagining we'd have things like display: flex and possibly each of the different alignment values, since they're a fairly small fixed set of options, but probably not content width since that could be any arbitrary value. Although, perhaps we could include the parent / global level content width if it's set, since that will likely be shared by a bunch of different blocks?

But the goal I was interested in exploring was to see how far we can limit the amount of unique styles we generate.

lib/compat/wordpress-6.0/class-wp-theme-json-gutenberg.php Outdated Show resolved Hide resolved
// TODO: Before this PR merges, move this to be a part of the style engine package.
// Part of the build process should be to copy the PHP file to the correct location,
// similar to the loading behaviour in `blocks.php`.
require __DIR__ . '/class-wp-style-engine-gutenberg.php';
Copy link
Contributor

Choose a reason for hiding this comment

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

If we move this to the style engine package, won't we need to create the same compat/wordpress-x.y folder structure in that package as we have in lib, to avoid clashes once the initial version of this is in Core?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a very good question! I wasn't quite sure what the best way to handle that would be (so just left it as a TODO for the moment). I've found the compat folder structure and overrides for the theme JSON class a bit confusing to work with, so I was hoping we could figure out a simpler mechanism for overriding core classes somehow for the style engine.

Let me know if you think there's a particular way we should handle it! I was mostly deferring the decision until we're closer to something we'd like to land since it seemed like a bit of a rabbit hole 😅

@ramonjd
Copy link
Member

ramonjd commented Mar 14, 2022

I was thinking that (at least at first), we'd just be focusing on outputting the "common styles" or those styles with a fixed set of options (like how alignments don't have an infinite number, maybe only a small handful of classes), and we'd then defer the highly specific values (like content width or gap value) to the rendering at the individual block level like in #38974 — and we can then make the choice between whether the classname reads in a semantic way, or if it's more obfuscated.

This makes sense. Nice.

I know it's probably premature to think about output, but there is some chatter over on the Layout tracking issue about whether we could look at CSS vars as well, which presets are capable of rendering.

I'm mainly thinking about whichever layout values we make configurable.

/* flow */
--wp--preset--layout--flow-width: 88px;

/* flex */
--wp--preset--layout--flex-wrap: nowrap;
--wp--preset--layout--flex-justification: left;
--wp--preset--layout--flex-gap: 2em;

/* grid */
--wp--preset--layout--grid-gap: 2em;
--wp--preset--layout--column: 2 / 4;

But are presets only valid when they are theme-configurable, or can we also have purely internal ones, in which case, potentially all the flex justification and orientation options could be presets?

Yeah for sure! That's a neat way to look at it, given that we can control which presets we actually expose via the theme.json API, i.e., external ones like gradients vs internal ones

@andrewserong
Copy link
Contributor Author

Thanks for taking an early look, folks! Much appreciated.

in which case, potentially all the flex justification and orientation options could be presets?

Yes, that's a good point, I was thinking all of these could be presets (the current state of the PR is what I could hastily put together last week, but my hope was that we could have most of the configurable options rendered as presets)

Another thought, though maybe not immediately relevant: will there ever be a scenario where layout type can be picked by user?

Like an arbitrary layout type that we haven't already defined? If we wanted to allow custom configured layouts, I suppose we could (try to) defer configuration to the theme.json settings, which could be both powerful and / or complex to implement 🤔. One potential benefit if we can represent the layout better in JSON could be to reduce the work of replicating layouts in both JS and PHP. Although, once we've got most settings rendered server-side via presets, perhaps we'll already have reduced some of the duplication there.

about whether we could look at CSS vars as well, which presets are capable of rendering.

Good question! I like the idea of CSS variables, but I was wondering if we'd wind up running into the same issues we had with gap support when it was primarily CSS variable based — that is, child blocks inheriting the layout settings of their parent due to CSS variables, even when we don't want that inheritance to happen 🤔

@andrewserong andrewserong self-assigned this Mar 15, 2022
@andrewserong andrewserong added [Type] Experimental Experimental feature or API. [Status] In Progress Tracking issues with work in progress labels Mar 15, 2022
@andrewserong
Copy link
Contributor Author

I've had a play in dec7bd8 and ac165bb with moving the common style rules out of layout.php and into theme.json to see how it looks, and started to swap out the common styles that are rendered when the block is rendered, with injecting a class name instead. I'm not quite sure of this, but it certainly slims down the code of gutenberg_get_layout_preset_style and shifts the common style rules out of PHP (though might be a bit awkward in theme.json?). Moving the rules to theme.json does make it feel a bit more "preset"-ey, but I wasn't sure if using a rules object makes it too specifically just a list of CSS rules that we're defining 🤔

Here's a couple of screenshots of the current state of things:

View source Front-end (note the mix of meaningful and container class names)
image image

Tomorrow I might have a play with grabbing a little more from #38974 and take another look at the more unique styles that are rendered for each block, and / or those options that are a controlled set (like justification).

@ramonjd
Copy link
Member

ramonjd commented Mar 15, 2022

Moving the rules to theme.json does make it feel a bit more "preset"-ey, but I wasn't sure if using a rules object makes it too specifically just a list of CSS rules that we're defining

Very interesting!

It's pretty nice having all the rules in one, configurable spot.

it certainly slims down the code of gutenberg_get_layout_preset_style and shifts the common style rules out of PHP

Could the common style rules live in a dictionary or internal PHP/JSON/CSS/? file somewhere?

Forgive me if I'm scratching up the wrong pole here, but I would have thought that the base/common styles for each layout would be part of the underlying implementation (so a theme.json extension/or a bunch of rules stored somewhere in Gutenberg) and we'd only expose configurable properties in the theme.json like gap and flexwrap.

I suppose similar to the way we expose typography/spacing/border styles in blocks, and the design tools to modify them.

Mainly to avoid breaking layouts altogether and to ensure, for example, that type: flex will always have display: flex and so on.

@andrewserong
Copy link
Contributor Author

andrewserong commented Mar 15, 2022

Forgive me if I'm scratching up the wrong pole here, but I would have thought that the base/common styles for each layout would be part of the underlying implementation (so a theme.json extension/or a bunch of rules stored somewhere in Gutenberg) and we'd only expose configurable properties in the theme.json like gap and flexwrap.

Thanks for taking a look! That's a great point — this PR probably exposes too much internal logic that someone could mess with and then break the layout implementation. I'll have a play with where else to put it!

@ramonjd
Copy link
Member

ramonjd commented Mar 15, 2022

this PR probably exposes too much internal logic that someone could mess with and then break the layout implementation. I'll have a play with where else to put it!

This made me think though... how extensible would we want things to be? What if theme.json authors could define their own layouts? Seems like your idea might open something like that up assuming it's palatable. 😄

@andrewserong
Copy link
Contributor Author

This made me think though... how extensible would we want things to be? What if theme.json authors could define their own layouts? Seems like your idea might open something like that up assuming it's palatable. 😄

Also a good point! A suppose it could also give theme authors a neat way to switch off auto-generated styles if they wanted a more vanilla experience, too.

I might leave that bit as-is for the moment while I look into the styles that have a controlled set (justification, etc). It shouldn't be too hard to move the JSON around when it becomes clearer where it should go 😁

@tellthemachines
Copy link
Contributor

Like an arbitrary layout type that we haven't already defined? If we wanted to allow custom configured layouts, I suppose we could (try to) defer configuration to the theme.json settings

I was thinking that, once we have more layout types, we could provide the option to switch between two or types in a block. Say for a row block, there could be the option to make it a carousel. But that can also work as block variations; it's mostly down to what would make the best UX.

I hadn't thought of theme-configured layouts! That would require a whole new API. But if the layout types we provide are flexible enough, themes shouldn't have to hack up their own.

@andrewserong
Copy link
Contributor Author

Say for a row block, there could be the option to make it a carousel. But that can also work as block variations; it's mostly down to what would make the best UX.

Yes, I think the ability to switch between layouts will be super powerful, particularly if we add carousel or masonry-like layouts, that'd be very cool to have for the Query Loop or Gallery blocks as a super-quick way of switching between quite different designs and layouts.

@andrewserong andrewserong force-pushed the try/style-engine-server-rendering-with-layout-presets branch from ac165bb to c2780bf Compare March 16, 2022 05:20
@andrewserong
Copy link
Contributor Author

andrewserong commented Mar 16, 2022

Update: in 8313c31, I've had a go at adding in controlled sets of options for style generation to the JSON structure, so we can now render out some classes for justification, e.g.

The naming is still up for grabs of course (I couldn't quite think of anything better than controlledSets today 🤷). Tomorrow, I'll have a go at hooking up the class names at the individual block level for these — I think we should be able to look up the layout settings and add a bit of logic to generate them at the block level instead of hard-coding them, but I'll have a play.

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

Really cool to see some movement happening in the style engine front. Left some comments on this PR but I guess the same applies to the other one as well.

* WP_Style_Engine class
*
* @package Gutenberg
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll focus my review on this file, as If I'm not wrong, this is the "style engine" package equivalent. Well I guess my first question is, should we try to move this file to the package (like we do for "parser", and "block-library")?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should we try to move this file to the package (like we do for "parser", and "block-library")

Good question! I suppose it depends on what will be easiest for when the feature needs to be merged into core. I see Ramon asked a similar question over on: #39446 (comment) — given that PR will be in better shape to land before this one, it might be better for us to explore where the file goes in that PR. For the moment, I mostly just put it here for convenience while hacking around!

* @since 5.5.0
* @var array
*/
private $registered_styles = array();
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the style engine need to be stateful? The JS version is stateless, so I was wondering if we should have the same signature/API?

I guess the question here is what is $resitered_styles for?

Copy link
Contributor

Choose a reason for hiding this comment

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

(Expanded on my questioning here on my next comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does the style engine need to be stateful?

It's a great question, and I think the answer is most likely a "yes".

I'll give a more thorough answer to the other comment, but there are two main reasons to have state:

  • To consolidate and de-dupe styles.
  • To enable outputting a single set of styles to a single style tag or stylesheet. (Instead of appending an additional style tag when each block is rendered).

In my experimenting so far, there doesn't appear to be a way around collecting the styles we would like to later output without having some kind of state.

*
* @return string The class name for the added style.
*/
public function add_style( $key, $options ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, so If I'm understanding properly, the style engine here has a different meaning than the style engine package in JS.

The style engine package in JS is responsible for "style object -> css rules" transformation (potentially also css rules -> css string) while the current php style engine is more for "css rules -> css string". Please correct me if I'm wrong.

I kind of feel like the first step is actually the most important for the style engine work (style object -> css rule) and the second step is an optimization step (array of css rules -> css string).

That said, that's fine, if we're focusing for the second part of the style engine, I'm wondering why are we building the API like that:

php

$engine->add_style( $rule );
$engine->add_style( $rule );
$engine->add_style( $rule );

$css = $engine-> output_styles();

instead of just

$css = engine_output_styles( $rules );

the latter being stateless and also more aligned with what I think will be the output of the first step of the style engine (an array of rules).

I guess the proposed API is motivated by the fact that the current block supports are "discrete" php files but personally I feel like they shouldn't be discrete, it's just the way they were implemented originally. I guess we can try to discuss this.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for checking this out!

I'll let @andrewserong elaborate with specific reference to this PR, but I have some general thoughts:

I kind of feel like the first step is actually the most important for the style engine work (style object -> css rule) and the second step is an optimization step (array of css rules -> css string).

Agree. The first step is happening over at:

#39446 👍

I see this following the stateless approach you mention.

This PR (and #38974) is part of our explorations into what's possible and what we (think we) want to see down the track as well (CSS consolidation, optimisation and a better developer experience).

I think we also wanted to noncommittally test some of the hypotheses/feature requests that have been floating around, e.g.,

  • obfuscation
  • consisten css classname hooks
  • deduping

Layout was a good starting point for this given the amount of CSS we generate on the frontend and rule duplication.

I guess the question here is what is $resitered_styles for?

I believe it stems from the long-term idea that we could render all CSS in one style block on the frontend (instead of many), and, in doing so, leave open the possibility for pre-render processing/filtering of CSS rules.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the questions @youknowriad! I'll answer slightly out of order to hopefully explain some of my thinking 🙂

At the outset, I do want to flag that in the current state, this PR is weird and experimental! 😁 My approach at the moment is to stuff this PR full of ideas / hack things in there to get them working, and then gradually pull things out into something neater.

Just echoing what @ramonjd wrote above, but the goal of this PR was to take a bit of a further out look to see what kind of end point we'll need to reach if we want to achieve some of the things we're hoping to get out of the style engine work. In Ramon's PR (#39446), the stateless / closer to the JS package version is being explored, and that'll likely be in good shape to merge much sooner than this PR.

I kind of feel like the first step is actually the most important for the style engine work (style object -> css rule) and the second step is an optimization step (array of css rules -> css string).

Absolutely. But I think that optimisation step could be harder to implement if we don't design for it — I'm keen for us to improve the style output (primarily de-duping and consolidating to a single stylesheet), and we get quite a bit for free if we add in the internal state and separate out the two steps (adding styles and output).

I think being able to separate generating and outputting styles into separate steps via some internal state will probably allow us some good flexibility for doing things like:

  • De-duping styles
  • Sharing classnames or styles across functions / at different execution times
  • Being able to access generated styles separately — for example, if we want to explore at some point making generated styles available via an API endpoint, having easy access to that state will make those explorations much easier

While those ideas sound very "optimise-ey" — i.e. things we'd usually think to do in later steps, and not do prematurely — here, we've got some concrete optimisations that we know we need to make and solve for, so I think it's worth it to dig in a bit to see what might look viable.

I guess the proposed API is motivated by the fact that the current block supports are "discrete" php files but personally I feel like they shouldn't be discrete, it's just the way they were implemented originally. I guess we can try to discuss this.

It's less motivated by the separate PHP files, and more motivated by styles being output one block at a time as each block is rendered, resulting in one style tag per rendered block (that uses the container id approach). We currently abstract that away in a small way in gutenberg_enqueue_block_support_styles — but my hunch at the moment is that we'll benefit from having some more smarts around this rather than relying on the wp_footer and wp_head hooks directly.

However, this is all still very much experimental! I'm more than happy for us to see how far we can get with the stateless approach in the shorter-term — my main interest right now is to ensure that if we do want to add in the stateful approach it's easy enough for us to migrate it in there to address optimisation, rather than having to rewrite everything! So, at least from my perspective, the current approach has pointed toward using a class for the style engine so that we can encapsulate (and namespace) our logic, and make it easy to add in the internal state if / when we need it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks that make sense.

It's less motivated by the separate PHP files, and more motivated by styles being output one block at a time as each block is rendered, resulting in one style tag per rendered block (that uses the container id approach).

That makes sense but personally what I don't like is the singleton because it's very hard to reason about depending on where you are (regular php template, api...). I'm just nitpicking at this point but I'd have a small preference for something more explicit.

Basically, what I don't like about the singleton approach (and I guess all regular WP style php) is that it's all dependent on order of execution of things, actions... It's not really clear when are we allowed to add styles to the instance, from where and who can do it. So we end up with hidden bugs, for instance what happens if I add a style to the singleton after the generate function has been called?

My "weird" mind prefers something more dumb :P

function render_blocks( $blocks ) {
   $markup = '';
   $rules = array();
   foreach( $blocks as $block ) {
      list( $css_rules, $block_markup ) = render_block( $block );
      $markup .= $block_markup;
      $rules = array_concat( $rules, $css_rules );
   }

   return [ $rules, $markup ];
}

list( $rules, $markup ) = render_blocks( $my_block_list );
$css = generate_optimized_stylesheet( $rules );

I guess it might not be easy to implement because it kinds of touches the fundamental of the existing do_blocks but anyway it's worth sharing just in case 🤷

we can also imagine that later we could also return JS in addition to "css_rules" and "markup" if we want to do JS optimizations.

list( $css_rules, $block_markup, $scripts ) = render_block( $block );

Wanted to clarify that there's no blockers from me here, I'm just basically sharing my mental model in case it helps in any way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm just basically sharing my mental model in case it helps in any way.

Thank you, it's always very much appreciated! 😊 That code has a more JS-ey flavour to it that immediately feels more at home to me, too.

You make great points about the challenges with singletons, actions, and the order of execution. I hadn't even thought of seeing if we could mess with the render_block API 😄 — while changing the return type there could open up a can of worms, this is some very helpful nudging for us to try to keep the API we design explicit and hopefully simple to work with. I'll continue having a play!

Copy link
Member

Choose a reason for hiding this comment

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

Basically, what I don't like about the singleton approach (and I guess all regular WP style php) is that it's all dependent on order of execution of things, actions... It's not really clear when are we allowed to add styles to the instance, from where and who can do it. So we end up with hidden bugs, for instance what happens if I add a style to the singleton after the generate function has been called?

Hmm, valid point, and the example is very helpful!

I suppose we haven't really decided where we'd dump the contents of every registered style 😄 It's a good excuse to dig into render_block too to test the idea.

@mrwweb
Copy link

mrwweb commented Mar 22, 2022

Very excited about the potential direction of this PR! I want to flag that it is extremely aligned with a lot of the suggestions in #38998 which discusses a number of the benefits of this approach for theme portability, CSS customization, and plugin-theme interoperability. Specifically, that proposal hopes to:

  • Keep the specificity of CSS standardized and low for all core styles
  • Use a CSS utility class like framework for both:
    1. block styling needs (just like @andrewserong says: "I was thinking that (at least at first), we'd just be focusing on outputting the "common styles" or those styles with a fixed set of options (like how alignments don't have an infinite number)"), and
    2. Theme-configurable values (font size, spacing, colors, etc.) that correspond to block settings

@andronocean had a fabulous summary of the benefits and details of how a system like this should work that I quoted at length.

@andrewserong
Copy link
Contributor Author

Thanks for the feedback everyone! I'm pleased to see folks are interested in the direction this is taking and that it looks like it could address some of the issues raised in the CSS standardisation discussions. There's a couple of other alternatives being explored, too (e.g. #39708), but it sounds like going in the direction of presets for common styles is looking promising 👍

In the immediate term, with the WordPress 6.0 timeline approaching, I'm going to pause working on this particular exploration for a little while, to focus on shorter-term layout issues and bug fixes / enhancements like those in the prerequisites section of the layout options tracking issue, that we can hopefully land in time for 6.0.

@andrewserong
Copy link
Contributor Author

Closing out this experiment now that #40875 has landed which progresses a consolidated class-based approach to common layout styles (and the style engine is progressing nicely toward style optimisation, too).

@scruffian scruffian deleted the try/style-engine-server-rendering-with-layout-presets branch July 13, 2022 07:59
@priethor priethor removed the [Status] In Progress Tracking issues with work in progress label May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Experimental Experimental feature or API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants