Add a style utility API

How does this integrate (or not) with asset loading?

I've been trying to get rid of as much of the system/base library as possible, and this has exposed various more or less undocumented 'utility classes' in core that have been around for years - clearfix, container-inline etc. that are loaded on every page despite in some cases only being used on a single or handful of admin pages in core.

Would the styles plugins also come with a .css file, or something else?

Hi,

@catch, this is something I have been thinking about too to avoid loading unused CSS on all pages.

- #3478113: [5.2.x] Add Bootswatch themes
- #3372792: Starterkit: split Bootstrap CSS per component

Like with UI Icons or UI Skins, adding a "library" key to a style declaration could be possible. Then is a style of this declaration is used, the library could be automatically attached.

Problem would be in some "free" areas not connected to UI Styles like WYSIWYG, or link class attributes, (same problem regarding components BTW), to detect automatically that a class is used then search for matching style and so library... This can quickly become tricky. Especially if sometimes the same CSS class is declared in multiple style declarations.

Funny that you mention "clearfix, container-inline", those classes are also in Bootstrap, and in UI Suite Bootstrap I have removed them:

libraries-override:
  ...
  system/base:
    css:
      component:
        css/components/clearfix.module.css: false
        css/components/container-inline.module.css: false
        ...

And the case you mentioned, would this mean that Core should have declared libraries with one small CSS file only and properly attach only when needed?

@grimreaper I've been slowly doing that in #2880237: [meta] Refactor system/base library although mostly for larger files used in less places. However pretty much all the use cases even for these tiny utility declarations are on admin pages so overall I think we should be doing the same.

We're approaching the point where the standard profile + stark loads about 1kb of CSS on a page instead of 7kb a year ago, this could be zero if we keep going. It opens up possibilities like inlining CSS for anonymous users without massive customisation etc. But ofc the last 1kb is the hardest - although stable9 means it's not a BC break for themes using that as a base.

@catch, Thanks! This looks very nice to go down to this few CSS by default!

Current Style definition in UI Styles:

  protected array $definition = [
    'id' => '',
    'enabled' => TRUE,
    'label' => '',
    'description' => '',
    'links' => [],
    'category' => '',
    'options' => [],
    'empty_option' => '- None -',
    'previewed_with' => [],
    'previewed_as' => 'inside',
    'icon' => '',
    'weight' => 0,
    'additional' => [],
    'provider' => '',
  ];

Style definition proposal:

  protected array $definition = [
    'id' => '',
    'enabled' => TRUE,
    'label' => '',
    'description' => '',
    'links' => [],
    'category' => '',
    'options' => [],
    'weight' => 0, // to keep?
    'library' => '',
    'class' => '', // for #3469785: Add HTML class property for styles?
    'additional' => [],
    'provider' => '',
  ];

#3469785: Add HTML class property for styles?
Need to address: #3517009: POC: Core styles API:

'target' => 'body'/'attributes'/'foo',
'key' => 'class',

like in https://git.drupalcode.org/project/ui_skins/-/blob/1.1.x/src/Definition/... ?

Do we prefix plugin IDs with provider Id like SDC ?

In this case it would require to put in place a "replaces" mechanism like SDC too?

Currently override (in sub themes) of plugin ID are used to disable style plugin as a whole or to add/remove options.

Should it be handled like SDC only allowing adding new options to props, new props/slots but not removing ones?

This impact the subject of value validation.

I would say let's not namespace to allow override. but when processing if a style plugin is disabled (we have not put this key in the proposition by the way..) or option is missing, ignore and log.

Discussed with @pdureau, like with Icon API and SDC.

Try to validate style definition against JSON schema.

For attribute object core/lib/Drupal/Core/Template/Attribute.php, add a "addStyle" method.

Need to take #3493495: Make style definition options translatable into account.

Discussed with @pdureau, splitting into modes.yml and utilities.yml, would require to create some additional YamlDiscovery classes to inject dynamilcally the "kind" also there is a risk of plugin ID collision and we want to avoid namespacing so let's have only one file.

TODO:
- Merge YAML files into styles.yml
- kind default to utility, can be manually set to kind.
- JSON schema: only allow utility and kind value for this property.
- Plugin manager provides method to get only plugins by a certain kind.
- Plugin manager getDefinitionsForTheme not tested yet.

- Form element
- Renderer to introduce #styles
- Attribute: add addStyle method.

TODO:
- Merge YAML files into styles.yml
- kind default to utility, can be manually set to mode.
- JSON schema: only allow utility and kind value for this property.
- Plugin manager provides method to get only plugins by a certain kind.
- Plugin manager getDefinitionsForTheme not tested yet.
- Form element
- Renderer to introduce #styles
- Attribute: add addStyle method.

TODO:
- Plugin manager getDefinitionsForTheme not tested yet.
- Renderer to introduce #styles
- Attribute: add addStyle method.

In UI Styles, sometimes there is "#item_attributes" instead of "#attributes" for image, responsive image.

  public const array THEME_WITH_ITEM_ATTRIBUTES = [
    'image_formatter',
    'responsive_image_formatter',
  ];

Should we handle that too?

I don't think it will be possible to add a method to attribute object because it know nothing about libraries.

So I think we will need to add a Twig function:

{% set attributes = addStyle(attributes, 'my_style', 'my_value') %}

So that it can alter the attributes object, but I am not sure that mechanism like attach_library could be triggered:

  public function attachLibrary($library) {
    assert(is_string($library), 'Argument must be a string.');

    // Use Renderer::render() on a temporary render array to get additional
    // bubbleable metadata on the render stack.
    $template_attached = ['#attached' => ['library' => [$library]]];
    $this->renderer->render($template_attached);
  }

TODO:
- Plugin manager getDefinitionsForTheme not tested yet.
- Update test target enum
- Test Attribute
- Test Attribute helper
- special bubbleable/attached for html/body?
- check target for mode only.
- put logic into Style definition to avoid duplication between renderer and Attribute + tests
- item_attributes?

I don't think we're going to get around the HTML preprocess, in fact, we won't be able to.
The core has a preprocess_html in app/core/lib/Drupal/Core/Theme/ThemePreprocess.php

Except we go through it before going through app/core/lib/Drupal/Core/Render/HtmlResponseAttachmentsProcessor.php, which handles attachments.

So even introducing a new type of #attached wouldn't work.

Or, we could create an event subscriber like core/lib/Drupal/Core/EventSubscriber/ActiveLinkResponseFilter.php
which will manipulate the response's HTML and access the response's attachments.

Before forgetting, discussion with @pdureau, for modes, light and dark is special because tight with browser and OS features of light/dark.

We should try to handle that too. So that it can prepare to provide a live theme switcher block like on design system documentation pages.

utility: use attribute only
If mode: look at target.

Twig attributes: new method addStyle
- add attribute:
-- empty target: OK
-- meta:
-- html/body:
- attached library: OK

render array:
- add attribute:
-- empty target: OK
-- meta: OK
-- html/body: Ok via attached.
- attached library: OK

Preprocess callback: #styles not usable.
- add attribute:
-- empty target: ?
-- meta: ?
-- html/body: ?
- attached library: ?

new attached key: styles:
- target:
-- attribute: value

$element['#attached']['styles'] = [
  'html' => [
    'class' => [
      'foo',
    ],
  ],
  'body' => [
    'data-bs-theme' => 'light',
  ],
];

OR

- target:
-- plugin_id: plugin_option

$element['#attached']['styles'] = [
  'html' => [
    'colors_background_color' => 'bg_primary',
  ],
  'body' => [
    'color_mode' => 'light',
  ],
];

With second option, possible during resolution to make usage of Attributes object so we can benefit from AttributeHelper::matchCollectionsTypes

UI Skins:
- preprocess HTML to set global theme: to replace with event subscriber executed before the one in Core handling html_attributes and body attributes, so able to set attachments.
- theme form settings alter: to put in UI Styles as now modes will be styles managed.
-- 2 styles form elements: one for html, one for body. Need to filter out styles by target (not just kind)
-- that way possible to handle #3485599: Handle styles on body tag

UI Styles Block:
- preprocess block: need to make it work preprocess callback

UI Styles CKE5:
- need to pass to a system of custom attributes to able to create a filter plugin that will be able to add attributes and attach library.
- on-the-fly style generation wrapped in .ck-content may be simplified to only load the library of the enabled style plugins. No more need to load all the libraries with parent theme recursively.

UI Styles Entity Status:
- hook entity view: should be unaffected or worst case, fall into same as preprocess.

UI Styles Layout Builder:
- preprocess block: need to make it work preprocess callback
- entity view alter: (to convert into hook entity view?)
- event subscriber

UI Styles Page:
- preprocess region: need to make it work preprocess callback

UI Styles UI Patterns:
- handling Attribute object: so should be ok

UI Styles Views:
- hook_preprocess_views_view: need to make it work preprocess callback

Discussed with @pdureau.

So, everything is OK:
- attributes.addStyle
- renderer / render array #styles
- preprocess callback (will alter directly $variables['#attached'] and $variables['#attributes'])

In each case:
- #attributes for kind: utility and kind: mode with empty target
- #attached html_head for kind: mode with target: meta
- #attached styles for kind: mode with target: html/body, then an event subscriber will put in html attributes or body attributes.
- #attached library for the library of the style plugin.

To add style utilities to html attributes or body attributes, use preprocess_html callback, (or force putting a style utility in #attached styles and default to body attributes).

$element['#attached']['styles'] = [
  'color_mode' => 'light',
];

Problem to solve is compatibility with big_pipe for #attached styles, and not sur if needed to be solved as for html and body tags big pipe should not intervene for those render element.

Also discussed about options:
- drop short syntax
- allow description per option

Idea of the night.

Should we add lifecycle/deprecation properties on style and/or on style options?

TODO:
- finish event subscriber for html/body
- add helper/refactor how to apply styles. Put logic into Style definition to avoid duplication between renderer and Attribute?
- drop short syntax for options.
- description of option.
- item_attributes? When adding a style, add optional parameter to specify attribute key?
- add lifecycle/deprecation properties on style and/or on style options?
- ensure big_pipe compatibility? not sure if relevant that a style alters body/html attributes asynchronously after page had been rendered.
- support of prefers-color-scheme or light-dark()

To test:
- Plugin manager getDefinitionsForTheme.
- Form element with theme.
- target property is now an enum
- Attribute object
- Attribute helper
- no more option short syntax
- description on option
- style application logic
- event subscriber

I will start/continue to update contrib modules to test with the new API.

Most of the stuff remaining here are to write tests.

If some review could be done to indicate if there is a blocker or an architecture problem before writing tests on stuff that will require rework it would be nice.

Thanks!

TODO:
- finish event subscriber for html/body
- add helper/refactor how to apply styles. Put logic into Style definition to avoid duplication between renderer and Attribute?
- item_attributes? When adding a style, add optional parameter to specify attribute key?
- ensure big_pipe compatibility? not sure if relevant that a style alters body/html attributes asynchronously after page had been rendered.
- drop short syntax for options.
- description of option.
- add lifecycle/deprecation properties on style and/or on style options
- support of prefers-color-scheme or light-dark()

To test:
- no more option short syntax
- description on option
- target property is now an enum
- StyleDefinition applyOrBubbleOnAttribute
- StyleDefinition applyOnAttribute
- StyleDefinition applyOrBubbleOnArray
- Plugin manager getDefinitionsForTheme.
- Form element with theme.
- Attribute object
- Attribute helper
- style application logic
- event subscriber
- bubbleablemetadata change

Coming in a bit late here sorry - I have been using a custom style_options plugin to allow editors to apply a css property value to an element via an inline style, ie:

<div style="--my-prop-name: 5">

I find this has advantages over simple css utility classes or data attributes, especially for use in unitless calculations.

Will a technique like this be supported?

Hello,

It is in the scope (see comment 16) and already implemented in the MR.

Thanks! I see it now.

The syntax for a simple range of values will be quite verbose, requiring repetition of the property name and manually specifying each individual value and unit.

As a future extension, I would like to allow for the possibility of a yaml definition something like:

type: style
element: range #specify the form widget
unit: rem #omit or '' for unitless
min: 0
max: 8
step: 1
property: '--my-prop-name'

A similar syntax could also be used with data attributes.

Given I had written a custom style_options plugin to support this, I'd be happy enough doing the same for this API as long as the underlying support and swappability is there.

Hello,

Thanks for your suggestion.

In short, I would say yes it will still be possible in extension as JSON schema allow additional unknown properties. But that's not the direction we want to promote.

I understand the feeling of code duplication. Know that with UI Suite Bootstrap, I went that way :D

The problem with such syntax is that it ties the style declaration to a form element and with our experience in UI Suite (UI Patterns, UI Styles, UI Skins, etc.) is that we want to decouple the declaration of the design system artifact (component, style, design token) to the UI forms (data can come from something else than a form).

1: because it is the front dev who declares the styles and how it will be applied, not how it will be configured. The style definition should know nothing (or as less as possible, maybe only suggest stuff) about Drupal Form API, where it will be configured. This would introduce drupalism into the declaration
2: having such link between style declaration and form element, would prevent (or make it harder) other contrib or custom alteration to choose something else.
3: concrete example of styles with Bootstrap:

flex_order:
  category: "Flex"
  label: "Flex order"
  description: "Change the visual order of specific flex items with a handful of order utilities. We only provide options for making an item first or last, as well as a reset to use the DOM order. As order takes any integer value from 0 to 5, add custom CSS for any additional values needed. Additionally there are also responsive order-first and order-last classes that change the order of an element by applying order: -1 and order: 6, respectively."
  links:
    - 'https://getbootstrap.com/docs/5.3/utilities/flex/#order'
  options:
    order_first:
      label: "First"
      value: "order-first"
    order_last:
      label: "Last"
      value: "order-last"
    order_0:
      label: "Order 0"
      value: "order-0"
    order_1:
      label: "Order 1"
      value: "order-1"
    order_2:
      label: "Order 2"
      value: "order-2"
    order_3:
      label: "Order 3"
      value: "order-3"
    order_4:
      label: "Order 4"
      value: "order-4"
    order_5:
      label: "Order 5"
      value: "order-5"

spacing_margin:
  category: "Spacing"
  label: "Margin"
  links:
    - 'https://getbootstrap.com/docs/5.3/utilities/spacing/#notation'
  options:
    m_0:
      label: "0"
      value: "m-0"
    m_1:
      label: "1"
      value: "m-1"
    m_2:
      label: "2"
      value: "m-2"
    m_3:
      label: "3"
      value: "m-3"
    m_4:
      label: "4"
      value: "m-4"
    m_5:
      label: "5"
      value: "m-5"
    m_auto:
      label: "Auto"
      value: "m-auto"

You will regularly have some styles with options mixing number and string so no possible to get a range.
4: Form alteration is possible (I am doing it in UI Styles with a different form element extending the one provided in this MR). But style application is currently done by the style definition (or at least providing helper in it), maybe that's not the correct approach, which will check options and not other properties.

MR looks good, a bit worried about adding more drupal specific logic to the attributes object, and more drupalism to SDC but that's a tradeoff that looks worth it

Can we get an issue summary update here - what's the use case for this - why do we need it etc?
@grimreaper asked me to review it but I'm missing the context.

It looks like you can put #styles on any element and they bubble up with attachments and then get replaced.

I'm not super keen on the HTML rewriting - can we do that with placeholders like we do for scripts/css in \template_preprocess_html?

re #41, sounds good. +1 to adding to the API of the attribute object

So, with the visibility shared in #45, does that means Mechanism for page wide application would need some more discussion and/or work? The API is already very valuable without this mechanism, because local applcitaion is the most common use case. So we are OK to remove core/lib/Drupal/Core/EventSubscriber/HtmlStylesResponseFilter.php from the MR.

In slack during discussion with @grimreaper I posted

If the bubbling to the body stuff isn't the 80% use case my advice would be to remove it. We should focus on MVP for core

It sounds like this bubbling isn't the main use case so I think we're in agreement there - thanks!

Before removing, I will do a test with @nod_ suggested usage of DOM. Then will remove.

And about remaining todo:

- update form element to be able to filter by target, provider, not just theme and kind.

I got an idea, I will isolate in a protected method the gathering of definitions, so that it will be easier to only override this part for contrib needs.

Hello, started at DrupalCon Vienna last friday, I took the time to finish my little simple test of the new API, I got 2 difficulties, due to the documentation composed from the comments above, so here are my take aways :
- the final filename is NOT my_theme.utilities.yml BUT my_theme.styles.yml (thank you Florent for the clarification)
- to add your styles to the render array in a preprocess, DON'T USE $variables['#styles']['background_color'] BUT $variables['#attached']['styles']['background_color']

Here is my complete test implementation, the idea is to provide a setting (background color) in the theme (named 'dcwien25'), the options of this setting are loaded from the yaml file, and finally the selected options is adding its corresponding css class to the html body.

dcwien25.styles.yml

background_color:
  label: "Background color"
  attribute: class # Overridable at the option level.
  options:
    light:
      label: Light
      value: 'bg-light'
    dark:
      label: Dark
      value: 'bg-dark'
    blue:
      label: Blue
      value: 'bg-info'

theme-settings.php

use Drupal\Core\Form\FormState;

/**
 * Implements hook_form_system_theme_settings_alter().
 */
function dcwien25_form_system_theme_settings_alter(array &$form, FormState $form_state): void {
  $style_manager = \Drupal::service('plugin.manager.style');
  /** @var \Drupal\Core\Theme\Style\StyleDefinitionInterface|null $definition */
  $definition = $style_manager->getDefinition('background_color', FALSE);

  $options = [];
  if ($definition) {
    $options = $definition->getOptionsAsOptions();
  }

  $form['dcwien25'] = [
    '#type' => 'details',
    '#title' => t('Settings available thank\'s to styles API'),
    '#open' => TRUE,
  ];

  $form['dcwien25']['bgcolor'] = [
    '#type' => 'select',
    '#title' => t('Background color'),
    '#default_value' => theme_get_setting('bgcolor'),
    '#options' => $options,
  ];
}

dcwien25.theme

/**
 * Implements hook_preprocess_HOOK() for html.html.twig.
 */
function dcwien25_preprocess_html(array &$variables): void {
  $bgcolor = theme_get_setting('bgcolor');
  if ($bgcolor) {
    $variables['#attached']['styles']['background_color'] = $bgcolor;
  }
}

My thoughts : this is pretty straightforward and easy to use, my next use case would be to add styling options in blocks or paragraphs (until they are replaced with Canvas !!)

Thanks @cedric_a for the tests.

About styles.yml it will be specified in the change record.

About #styles not working, it is because you have tested in a preprocess and potentially put #styles NOT on a render element. If you put #styles on a render element it will work:

$build['test'] = [
  '#type' => 'container',
  '#styles' => [
    'background_color' => 'dark',
  ],
];

Hi,

Form element updated for easier override of definitions obtention logic.

I will complete the tests during the coming days.

In the meantime if people can give reviews and feedbacks to ensure architecture.

I will appreciate to not write tests on stuff which would potentially be removed or reworked ;)

Thanks!

Reworked how Attribute object handle attachments to avoid side effects during rendering process and fix existing tests.

Only remaining existing test not passing is core/modules/syslog/tests/src/Kernel/SyslogTest.php

Because syslog config is null in the service during execution. I think it is due to the logger factory service added to the renderer service. So during test execution, the logger service is not recreated with the module's config imported.

But I had not the time to ensure that and figure out a fix yet.

Pipeline is green now!

Adding new tests.

Removing tag "Needs issue summary update" per comment 45.

Updating remaining tests to write.

To test:
- no more option short syntax
- description on option
- target property is now an enum
- enums
- StyleDefinition applyOrBubbleOnAttribute
- StyleDefinition applyOnAttribute
- StyleDefinition applyOrBubbleOnArray
- Attribute object addStyle and changes
- Attribute object addStyle through Twig
- Attribute helper changes
- style application logic
- bubbleablemetadata change
- unit or kernel or functional test on renderer change, aka apply on render array

I have completed the tests:

- Attribute object
- Attribute helper
- renderer
- bubbleablemetadata

I have not found an existing test for the one line addition of HtmlResponseAttachmentsProcessor

I started to write kernel test for the renderer to see resulting render array after processing, but this was equivalent to unit tests already done.

Same for calling Attribute in Twig, with the unit tests on it PHP side I think it is ok.

Now waiting for reviews and feedbacks!

Thanks!

Discussed with @pdureau,

Changes related to removing special case of #item_attributes to use #attributes instead moved to a dedicated issue #3554447: Use #attributes instead of #item_attributes to simplify the MR of style API issue.

Did a really high level review, didn't review all of the test coverage.

I'm not super sure but this feels analogous to libraries and I wonder if info alter hooks should be executed somewhere: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...

Hi,

Thanks for the reivew!

Did an update regarding small changes. Waiting for discussion approval for the remaining points.

For comment 64, I think you are right, I have not tested the hook_info_alter for styles. Looking at DefaultPluginManager:

protected function alterDefinitions(&$definitions) {
    if ($this->alterHook) {
      $this->moduleHandler->alter($this->alterHook, $definitions);
    }
  }

So the alterDefinition of the stylePluginManager should be changed as???:

  protected function alterDefinitions(&$definitions) {
    parent::alterDefinitions($definitions);
    if ($this->alterHook) {
      $this->themeManager->alter($this->alterHook, $definitions);
    }

    /** @var \Drupal\Core\Theme\Style\StyleDefinitionInterface[] $definitions */
    foreach ($definitions as $definition_key => $definition) {
      if (!$definition->isEnabled()) {
        unset($definitions[$definition_key]);
      }
    }
  }

I'm honestly not sure what the established pattern is, I just know we want to alter themes after modules.

Maybe asking one of the plugin subsystem maintainers would be helpful.

Hi,

All code review threads had been fixed.

Issue still in needs review if other feedback are needed.

I think all of my feedback has been addressed, I don't think I can RTBC as I haven't reviewed it deep enough, but I think it's ready for that deep review.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

MR was not mentioning git conflict and tests are still green after rebase.

Back to needs review.

Hum, I am not sure how to fix pipeline after rebase.

https://git.drupalcode.org/issue/drupal-3517033/-/jobs/8345516

The "Drupal\Core\Theme\Style\StylePluginManagerInterface::getGroupedDefinitions()" method will require a new "string $label_key" argument in the next major version of its interface "Drupal\Component\Plugin\CategorizingPluginManagerInterface", not defining it is deprecated.

In the latest commit I have added this argument, but CI is still not happy.

I don't get the deprecation when executing the tests locally.

I forgot a method, misreading the error message.

Thanks godotislate on Slack!

https://drupal.slack.com/archives/C079NQPQUEN/p1770144037257769

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

The Functional JS error is unrelated.

Back to needs review after rebased.

I pushed a separated branch with the commits history before I squashed it to ease current and future rebases.

The issue summary could use an update here, mostly an example of what the proposed API looks like.

Good suggestion!

I am preparing that as a change record and also updating IS.

CR created: https://www.drupal.org/node/3586264

Should documentation pages be created before merge? In case someone reads a documentation page for an API not merged yet...

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Accessibility tag for dark mode support.

Edit: And contrast, forced-colors, or anything else related to accessibility.

There are some interesting ideas here. I'd love classes to be more than just random names.

I was thinking about this a bit last week. Not this specifically, but how do we add structure to how we define style guides. As sites get more complicated we have to start understanding why.

https://mgifford.github.io/ACCESSIBILITY.md/examples/MODERN_CSS_THEME_AR...
https://mgifford.github.io/ACCESSIBILITY.md/examples/COLOR_CONTRAST_ACCE...

Having more of an organizing logic around these styles is important.