Added support for custom CSS properties in the StyleWrapper

[sneridagh]

This is a pattern I use lately, and it makes a lot of sense to me.

A couple of questions:

• Would it be useful/needed to also inject a className in order to hook to? I was about to do it, but I don't know if that would be required... maybe to know if a value is set or not... but you can always have a default fallback value.
• Naming... what not! I chose CSSProperty as the sufix... any other suggestions?

I'm well aware that this opens the door to fine grained style customizations, if we extend the pattern not only to use custom CSS properties but also use random CSS properties (another converter should be created, then do not append the -- and disable nested support, then) I leave it up to the community. However, the initial feeling when we came up with the style wrapper stands: I wouldn't like the wrapper to be missunderstood, and that people are able to easy create blocks that resemble WordPress Elementor...

Let's iterate over this, in the meanwhile, I will work on integrate the PoC into VLT.

Update 01/01/2024: Answer to questions:

• It's not really needed, since one could build a selector like this:

 .block.teaser[style*='--background-color'] {
    padding: 20px 0;
}

if required.

• This PR is not using a suffix discriminator to differentiate from the others. Now it detects if the property starts by -- see the documentation.

[netlify]

✅ Deploy Preview for volto canceled.

Name	Link
🔨 Latest commit	7f449db
🔍 Latest deploy log	https://app.netlify.com/sites/volto/deploys/6592e6f87d986600083203d7

[netlify]

✅ Deploy Preview for plone-components ready!

Name	Link
🔨 Latest commit	7f449db
🔍 Latest deploy log	https://app.netlify.com/sites/plone-components/deploys/6592e6f8246b720008557d02
😎 Deploy Preview	https://deploy-preview-5581--plone-components.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[stevepiercy]

Although not part of this PR, I found classname and fieldname and their plural forms to be weird. Are they supposed to be inline literals? If yes, then they need to be wrapped with single backticks and use the correct casing, else they should be "class name" and "field name".

[stevepiercy]

Fixed a heading and follow up comments.

[stevepiercy]

Thank you!

[tiberiuichim]

The slope is steep and full of grease. Pretty soon we'll end up here: https://raw.githubusercontent.com/eea/volto-block-style/master/docs/screenshot.png

When I started volto-block-style I had the same intention: let me create a way to centralize block "themes" and make it possible to assign those. That's why the "Style presets" widget is placed first in that volto-block-style sidebar. Pretty soon it devolved into whatever this is: https://github.com/eea/volto-block-style/blob/1fd970881ca1d816f8db764cdb2498348f10f191/src/StyleWrapper/schema.js#L3

[sneridagh]

@tiberiuichim I know... it's not easy. It's very hard to predict which techniques and patterns are the best for the use case.

When I first pushed for the wrapper I had one thing clear: The intention was not to provide an easy way to plague Volto blocks settings with Elementor-like controls... because it's not what we are aiming for in Volto. volto-style-block was a bit of that. It's absolutely legit if you have a client that explicitly asks for this, and it's editors are real power users who knows how the web works and are capable of handling this complexity. So not enabling doing it in an easy way was important for me.

This is something in between, still with a clear purpose, and documented so people can know how to take advantage of it. A width value, a color, so you can then take it in the CSS and use it right away, one shoot only CSS.

As I said, this opens the pandora's box by perverting it could be used to really inject arbitrary CSS properties. If you really wanted to do it, you can, but on your own. Although, given the power of using custom CSS properties instead, who wants this nowadays?

[davisagli]

Thanks for changing it so the developer can just write the property name the same way it is rendered; I think that makes it easier to use.

I do have some of the same doubts about whether people will use the feature judiciously, but the potential problem of adding too many options is there whether or not this feature exists. Would it help to give an example in the docs of a situation where using this would be appropriate and one where it would not?

[sneridagh]

@tiberiuichim @davisagli I am testing it in a current project, and I have to say that the result surprised me quite a bit. I didn't expect it to be so convenient.

Let me explain, Volto Light Theme as starting point. We have in there in place a global CSS custom property --image-aspect-ratio so all the images in blocks (teasers, listings too) use it. The idea was that someone in his customized theme could set it and change them all in runtime, because, custom CSS properties.

When the feature in this PR is used combined with the above you can enhance a block's schema (eg. teaser) as follows:

  schema.properties.styles.schema.fieldsets[0].fields = [
    ...schema.properties.styles.schema.fieldsets[0].fields,
    '--image-aspect-ratio',
  ];

  schema.properties.styles.schema.properties['--image-aspect-ratio'] = {
    widget: 'select',
    title: 'Aspect Ratio',
    choices: [
      ['1', '1:1'],
      ['16 / 9', '16/9'],
    ],
    default: '1',
  };

Then, the markup of the block will contain the custom property:

<div class="block teaser" style="--image-aspect-ratio: 1">
...
</div>

So, locally, that block and only that block, will apply that CSS definition without having to change a single line of code of CSS.

I will add this to the docs.

[sneridagh]

@davisagli I improved the example and use case in the documentation. I thought about a "bad example" given the current implementation, and I can't come up with one. I guess that one would still to come up with a mapping 1:1 of custom properties and basic arbitrary CSS properties in CSS or completely override and allow arbitrary CSS properties injection. What about documenting the reasoning behind we do not allow them?

@stevepiercy please take a look again!

[stevepiercy]

I am unsure of a couple of passages, and noted them as such.

I also suggested reordering the code blocks and their narrative text to improve flow.

[stevepiercy]

I think we're getting closer. I rearranged the CSS to be first, so it flows better, and filled in a couple of pieces I thought were missing. Finally, I took another stab at those two sentences that confuse me more than help me understand.

[stevepiercy]

These two sentences got missed again in the review. I'll create a separate PR to fix it.