Parent issue is now merged!
To restore when Core 11.3 will be released.
And need to update Core requirement.
Updated MR to fix existing core tests and code quality.
Added an admin permission to the DesignToken entity for easier integration with existing tests like rest and JSON:API tests. And also at least now the entity has an admin permission for later APIs.
PS: remaining test failure seems unrelated.
Fixed provided on ui_examples 🐛 Using item_list in examples break rendering Active .
Please port the fix here.
grimreaper → made their first commit to this issue’s fork.
Just thinking.
With ✨ Add a style utility API Active , I think it will be possible to set:
attached:
styles:
my_style: my_option
And with the proposed event subscriber (removed from Core for now, but it could be moved in ui_styles until then) then styles could be attached to body from the render array of the example.
Hi @anmolgoyal74,
Thanks for the MR!
I put 2 review comments.
Also please ensure code quality jobs are ok.
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!
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',
],
];
Hello,
Rebased done.
I also updated tests to fix some.
I think the remaining tests failure are not related to the MR.
Proposed new title:
Different modules can't override the same entity type class
Proposed summary:
If an entity class is overridden by multiple modules, then static call of intermediary overriding class will raise error as not possible to find matching entity type.
Example with entity_view_display overridden by Layout Builder LayoutBuilderEntityViewDisplay then overridden by another contrib or custom module.
In layout_builder.install, the following code will raise an error:
function layout_builder_install(): void {
...
$displays = LayoutBuilderEntityViewDisplay::loadMultiple();
...
Proposed resolution
The entity type should be aware of all intermediary class overrides.
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.
Thanks for the feedbacks.
JSON schemas: removed
Discovery: I tried something but see review comment in MR, YamlDirectoryDiscovery expects an identifier key which is not present as token identifier is its path.
Pushed WIP JSON:Schema validation.
Put similar mechanism as Icon API and Style API MR, but it validates against invalid/non-existing type. I think it is because of remote schema references. Didn't have time to ensure it is due to that yet.
DONE:
- minimum config entity
- config schema
- rework design token value plugin data injection
- CSS generation/serialization from config entities
TODO:
- validate tokens.yml againsi JSON schema
DONE:
- design token value plugin types
- some design token values to POC, simple and a composite one
- put parsing logic entirely in design token plugin manager
TODO:
- need to take care of name with spaces.
- config entity
- tests
Dropping some notes after discussion with @pdureau. Plus personal thinking.
Plugin type PHP, token type
- applicable (either method or in plugin annotation)
- toCssVariable
- fromDtcg
- getFormElement (for Core 11.4)
In DesignToken plugin, value should be an instance of a design tokenValue plugin with value.
Serialization only for config entity.
design_token.group__group__border_radius.yml:
id: 'group__group__border_radius'
type: 'border'
scopes:
:root:
value: '5'
unit: 'px'
%foo:
value: '10'
unit: 'em'
Config schema:
design_token.type.border:
type: mapping
...
design_token.type.font_family:
type: string
...
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.
Hi,
Thanks for pushing the MR forward.
My personal preferences (regarding your last comment):
- {{ attributes.addClass(['foo', 'bar'])
: so the opposite of current state of Drupal files, and what TwigCS does by default.
- {{ include('template.twig', {}, with_context: false) }}
: agreed with you.
Not sure if TwigCS configuration had been discussed in this issue or in another one.
Hello,
It is in the scope (see comment 16) and already implemented in the MR.
- 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
✨
Add an interface to apply ui styles to the page body?
Active
EDIT: no need of other event subscriber supposedly.
That way more complicated stuff will be doable like #3259377: Add an ImportProcessor plugin to fetch translations → , ✨ Handle default language and its translations at once Needs work or ✨ Support Crop API Active where crop entities does not reference the image properly only with an ID.
And for the original problem of memory/timeout, the client website can then do the batches on those prepared lists.
So the batch could have more fine grained operations.
And during entity list import, no more need of the current entity reference/embedded entities, etc mechanism which pops other imports.
Also for JSON:API Extras field enhancer plugins, no more need to alter JSON:API output to provide data for the client to search for referenced blocks, etc. Server website will have already prepared the JSON:API urls to search directly.
TODO
UI Styles:
- Update Stylesheet generator:
- Check if drilling is still needed:
- Check if MachineNameTrait is still needed:
- tests updated:
- provide Drush code upgrade command:
- update documentation
UI Styles Block:
- current features:
- tests updated:
- config update:
UI Styles CKE5:
- current features:
- tests updated:
- config update:
- 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:
- current features: OK
- tests updated:
- config update:
UI Styles Layout Builder:
- current features:
- convert hook_entity_view_alter into hook_entity_view?
- tests updated:
- config update:
UI Styles Page:
- current features: OK
- Add utilities on html and body to have equivalent of ui_skins mode.
- tests updated:
- config update:
UI Styles Library:
- current features: OK
- Rework library:
- tests updated:
UI Styles UI Patterns:
- current features:
- tests updated:
- config update:
UI Styles Views:
- current features:
- tests updated:
- config update:
UI Styles 1:
- warn extra deprecated
- mark stuff as deprecated
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:
-
-
-
-
-
-
-
- support of prefers-color-scheme or light-dark()
To test:
-
-
-
- StyleDefinition applyOrBubbleOnAttribute
- StyleDefinition applyOnAttribute
- StyleDefinition applyOrBubbleOnArray
- Plugin manager getDefinitionsForTheme.
- Form element with theme.
- Attribute object
- Attribute helper
- style application logic
- event subscriber
- bubbleablemetadata change
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.
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
Documented Core patch.
Documented update workflow.
Refactored code duplication between hook_update_N and Drush command.
Added message at the end of hook_post_update_NAME.
I let @nicoloye prepare automated tests.
Also try to:
- update the ui_examples examples to use this new API.
- update components template and presenter templates to use this new API.
It would require to wait for ui_suite_bootstrap 5.3.x or to increase Core requirements.
Idea of the night.
Should we add lifecycle/deprecation properties on style and/or on style options?
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
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
✨
Add an interface to apply ui styles to the page body?
Active
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
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.
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.
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);
}
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?
TODO:
- Plugin manager getDefinitionsForTheme not tested yet.
- Renderer to introduce #styles
- Attribute: add addStyle method.
Hi,
Dropping an idea I got, unrelated to existing thread. Just to write it out of mind.
It involves a big change.
I thought about introducing a new endpoint in Entity Share Server. This endpoint will give the list of entities and their dependencies to pull given a channel and given import config infos.
So client website tells server website "I want to import those content entities (selected through pull from or whatever) with referenced entities and this depth of referenced tree, with embedded entities in RTE, etc.".
The server website does the computation to respond the list of entities per entity type.
The client website can then do the batches on those lists. And when handling relationships, if the referenced entity exists, just reference it and that's it, it will be updated later in the batch. If the referenced entity does not exists, create a placeholder/empty entity (I guess like what Migrate does for chicken/egg problem).
It would require:
- a new endpoint on entity share server
- a new method on import processor plugins to get the query parameters to add in case it is enabled
- a new plugin type on entity share server to make the endpoint response dynamic and which will listen to the query parameters.
It would allow:
- to fix timeout/memory limit problem of the current batch problematic architecture
- allow for features like
#3259377: Add an ImportProcessor plugin to fetch translations →
,
✨
Handle default language and its translations at once
Needs work
TODO:
-
-
-
-
- Plugin manager getDefinitionsForTheme not tested yet.
- Form element
- Renderer to introduce #styles
- Attribute: add addStyle method.
Rebased without merge commit.
Tested, and reviewed: ok
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.
Discussed in ✨ Add a style utility API Active .
Too side effect prone, let's stay simple.
field.html.twig wrapper ignored by UIP2 and Display Builder.
Need to take ✨ Make style definition options translatable Active into account.
For attribute object core/lib/Drupal/Core/Template/Attribute.php, add a "addStyle" method.
Discussed with @pdureau, like with Icon API and SDC.
Try to validate style definition against JSON schema.
I will use this issue to provide change for UI Styles 2 regarding Core issue ✨ Add a style utility API Active
grimreaper → made their first commit to this issue’s fork.
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.
Seems that a live editor exist that will ouput the markdown to copy/paste: https://mermaid.live/edit#pako:eNpVjbFugzAQhl_FuqmVkghwnDoeKjWkzRKpHTIVM...
Hi,
Thanks for the issue and the MR.
Is it related or a duplicate of ✨ Consider length for compression? Active ?
Put all changes related to coding standard on separated branch.
Maybe should go into 📌 [2.0.9] Add missing strict_types=1 diretcives Active
Hum, thinking out loud.
But if this new route has nothing to do with styles plugin, why put it in ui_styles?
Should this logic be extracted into a dedicated module?
Hi,
I have created a quick and dirty workaround to avoid problem on existing websites if they want. I copied the CSS from app/libraries/dsfr/dist/core/core.css impacting icons.
I agree that a dedicated dynamic to be generated is a good solution. But as ui_suite_dsfr is a theme it can't provide controller and route.
I think, we can create a new route in UI Styles like ui_styles.stylesheet but in the extraction logic, instead of filtering regarding the styles plugin, it could take as parameters:
- prefix (like existing route)
- library (list of libraries or string if targeting just one)
And so it would just generate from the CSS of this library.
grimreaper → made their first commit to this issue’s fork.
Hello,
I confirm that with changes in other issues, this MR is no more needed.
Hi @herved,
Thanks for opening this issue, I also witnessed that and for the moment not a problem but I understand that it is not the best possible behavior.
If I remember well, the looping on LB sections was for block content entities (put in a node LB override for example) which view modes (block content) were using LB. As block content entities were rendered inside a block.html.twig template and the attributes variables there was not the same as the one prepared in hook_entity_build...
I think we should use the occasion of UI Styles 2 to rethink this behavior.
Also block content (using DB in its view display) no more render the block title (at least in LB, not tested with Block Layout).