Nice to see so much great people here (David, Jean, Mike, Wim).
It would be nice to add a Icon API Form Element into Drupal Core. We didn't do it last December, in order to iterate in contrib space first. But it may be relevant now.
Interesting. Also, I am proposing in this XB issue https://www.drupal.org/project/experience_builder/issues/3515074#comment... 🐛 References must not be required to guess props' JSON schema Active to move 2 UIP2 service to Core: they may be useful for XB too, they are inspired from a Python library, they don't depend on anything related to UI Patterns, and they implement pure JSON schema logic.
I think the concerns would be reduced if ✨ [PP-1] Allow schema references in Single Directory Component prop schemas Postponed is done and core provided a handful of common definitions — such as for images.
Indeed, i have added a extensive comment ✨ [PP-1] Allow schema references in Single Directory Component prop schemas Postponed with the hope we will move fast on this subject. However, it may be a complicated task if we want to do things right.
Slightly tricky: if the ordering of the properties is different, it should also match. For example: not src/alt/width/height, but width/height/alt/src. I assume you agree those should all be treated as equivalent?
Of course, order of properties in a JSON object doesn't matter as far as I know. You can have a look on UI Patterns 2's Canonicalizer service: we are ksorting the properties before working on them.
The compatible subset example … is an interesting observation 😅 I hadn't thought of that one for sure! I want to think that through some more, but purely logically speaking, it's hard to refute.
You can have a look on UI Patterns 2's CompatibilityChecker service. We have happily used this for one year for similar needs.
See also [#16043748], especially comment 40.
Just a little up-to-date summary of the 2 existing JSON Schema stream wrapper in contrib.
Experience Builder
Drupal\experience_builder\JsonSchemaDefinitionsStreamwrapper:
public: true
tags:
- { name: stream_wrapper, scheme: json-schema-definitions }
Example: json-schema-definitions://foo.module/bar
JsonSchemaDefinitionsStreamwrapper service is:
- Extracting the extension and the JSON schema $def ID from the URI. Example:
foo
module as extension andbar
$def - Load the
schema.json
file from the extension path. Example:/modules/contrib/foo/schema.json
- Return the extracted $def
Pro: is generic, any module can provide JSON schema definition without any dependency to Experience builder outside this stream wrapper.
Con: only static declaration
UI Patterns 2
ui_patterns.schema_stream_wrapper:
class: Drupal\ui_patterns\SchemaManager\StreamWrapper
arguments:
- "@plugin.manager.ui_patterns_prop_type"
tags:
- { name: stream_wrapper, scheme: ui-patterns }
Example: ui-patterns://bar
Streamwrapper service is:
- Extracting the . Example:
foo
module as extension andbar
$def - Load the
schema.json
file from the extension path - Return the extracted $def
Pro: a dependency to plugin.manager.ui_patterns_prop_type
service
Con: $defs are shipped alongside some niceties (type converter, data normalizer...) as a cohesive
Proposal
We can go the Experience Builder way because it is easier to ask UI Patterns 2 maintainers to add and maintain a schema.json
file in their module instead of introducing a new plugin type to Core.
Also, this may be the opportunity to:
- simplify a bit the URI scheme (previous proposal was
$ref: module://experience_builder/image
but we can propose another schema if "module" or "theme" are too generic) because this kind of URL will be typed a lot by component authors, with a lot of chance of mistakes - considerate Mateu's advice to use URI anchors to target the $defs ($ref: #) to leverage existing JSON schema mechanisms instead of extracting manually like JsonSchemaDefinitionsStreamwrapper is actually doing (however, I would advise to explore $anchor keyword instead of the propsoed JSON pointer
So, if we mix both, we can have something like that: $ref: module://experience_builder#image
This may not be enough
However, this proposal is not solving another issue reported by @finnsky : ✨ [2.0.0-beta5] Use HTTP URL isntead of a stream wrapper in JSON reference Closed: duplicate
We are becoming more and more generic, but it is still a Drupal only proposal (module
and theme
scheme, schema.json
convention...).
What can we do to make those references working outside of Drupal (for example: in storybook)?
Thanks you so much Kensae, your solution is very pragmatic, targeting exactly where the issue here in SDC prop conversion.
However, we are currently facing a similar issue in UI Example module: ✨ Support Single directory components Active and I would address to address both:
- in a more generic way, because the root cause is the wrong sue of render children by SDC
- in a similar way in both module
So, I let this in review state for now.
Hi @nicxvan, you have open a MR, can you assign the issue to you so people will know you are working on it?
From Enrique Lacoma:
Overriding components in a module, in the doc https://www.drupal.org/docs/develop/theming-drupal/using-single-director... → says you can't override a component in a module, but I was able to do it, is the documentation correct or doing the override inside a module can create any issue? thanks
So, we have agreed on using meta:enum
and leveraging this information and the related translations (from locale
module API) is up to the display building tools like UI Patterns 2 and Experience Builder.
Also, adding meta:enum
to the documentation will be done in this issue
🌱
Clarify SDC documentation by toning down Twig blocks promotion
Active
.
So, what can we do in Core?
- Add a mention of
meta:enum
in ComponentMetadata? Ans some light logic? - Add
meta:enum
to some of our test components - Add a specific test about
meta:enum
? which one?
wim leers → credited pdureau → .
DrupalCon chat with Wim. Let's check is something must be done in Core according to what we already do in UIP2:
- ✨ Enum vales do not have translatable labels Active
- 📌 [SPIKE] Comprehensive plan for integrating with SDC Active
- ✨ SDC does not respect JSON Schema validation Active
The Core subject where we can help:
- 📌 Twig disallows dashes in variable names, so SDC should disallow it in prop names Active
- 📌 ComponentPluginManager should detect if an extension provides two different components that resolve to the same plugin ID Active
- ✨ SDC DX does not inform about invalid JSON schemas Active
Also, let's check this if it is OK on UIP2 side:
Review OK
But PHPCS fix to do.
I will create the drupal_cms_olivero
issue, because the real problem is on their side.
Suggestion from @mlncn about replaces
property:
looking to improve the documentation slightly, the general advice should be to copy the entire component subdirectory into your themes components directory and add the replaces line. It's not like it can inherit anything it's a true replacement, yes?
Indeed, the replacement component is still a full component with its Twig template and its YAML definition.
The projects might not be using them directly, but there are dozens/hundreds of preprocess hooks running on those projects via core and contrib modules.
Of course.
but everything else in #99 I'm 100% agreed
@catch in #106 ? ;)
I don't fully understand #type => component or how to entirely get rid of preprocess via SDCs yet
In UI Suite community (not only the contributors in drupal.org, but also the agencies doing projects I am interacting with), we are not using preprocess hooks anymore, for many years, except for forms. I don't really know why it is a less an issue for us. Maybe because we fully switched to a design system + display builder paradigm. It is worth investigating and writing that down.
Adding three concrete active issues that could use reviews/help.
Thanks you. I will have a look after Atlanta.
I'm more likely to implement a twig extension instead of a preprocess function nowadays.
Twig extensions have a high risk of being harmful:
- Written in PHP, they may hide some business logic specific to a specific project, or calls to CMS API. A SDC component is just a "dumb" piece of UI logic receiving already resolved data.
- Written in PHP, they may introduce PHP in a Drupal theme (which we cant to avoid, a theme must be a front dev friendly place) or a dependency to module.
- Instead of fixing the root cause (why this data is not OK to be injected in my template? We are moving the problematic mechanism elsewhere
I wonder where this fits now that we have SDC, and Experience Builder is in early development?
We still need to work on this because:
- SDC is addressing only UI components, an important part of a modern Render API but not everything
- Experience Builder is a display builder, so an higher level tool. We need to address the lower levels
Nearly 5 years after my previous comment, here we are.
Current status
Let's not forget the Render API is great:
Declarative
: Easy to type. (de)Serializable if clean.Easy nesting
: The Virtual DOM of Drupal. We are building a tree.Data bubbling
: Declare locally, impact globallyAsset libraries
management: Our beloved libraries.ymlClever caching
: Context, tags, keys…
❤️ Building display by assembling configurable plugins returning renderables.
However, the renderables themselves are an issue.
They are too many of them and they are of 3 kinds:
- 35 render elements in Core:
$ grep -hoEr "#type' => '(\S+)'" core/ --exclude-dir tests | sort | uniq -c | sort -nr 138 #type' => 'details' 94 #type' => 'container' 93 #type' => 'link' 68 #type' => 'table' 47 #type' => 'actions' 36 #type' => 'inline_template' 27 #type' => 'fieldset' 23 #type' => 'html_tag' 21 #type' => 'status_messages' 14 #type' => 'pager'
- 99 theme hooks in Core:
$ grep -hoEr "#theme' => '(\S+)'" core/ --exclude-dir tests | grep -v "__" | sort | uniq -c | sort -nr 84 #theme' => 'item_list' 21 #theme' => 'username' 16 #theme' => 'image' 7 #theme' => 'status_messages' 7 #theme' => 'image_style' 6 #theme' => 'table' 6 #theme' => 'links' 5 #theme' => 'update_version' 5 #theme' => 'indentation' 5 #theme' => 'file_upload_help' …
- 2 special ones:
1016 '#markup' 85 '#plain_text'
😱 135 renderables to learn/use? This is the issue.
What can we remove?
We have SDC for UI components, so:
#type => component
can replace a lot ofhook_theme
, the one describing UI components:breadcrumb
,progress_bar
,links
...- We can also replace the 34 render elements which are wrapper around
hook_theme
(because what they do, adding asset libraries, variable typing... is already done by SDC) we removed :table
,status_messages
,pager
...
The hook_theme
not suitable for for conversion to SDC are often annoying wrappers we can get rid of anyway : block
, node
, view
, field
…
The few remaining render elements can't be expressed as UI components:
- some must be kept because they represent other design systems artefacts (icons...) or low level bricks (HTML elements, placeholders...)
- some may (not found in my today look) be shortcut to Drupal callables returning renderables, it is better to explicitly call the callable (service, function, method..) instead
Deprecation of ThemeManager::render()
If all template file based renderable are managed by SDC and because SDC is not passing through the ThemeManager, we dont need the template loading feature of the ThemeManager and we get remove it to get roi of the old confusing stuff:
- Theme wrappers: Confusing and useless. You can always use an upper level instead.
- Templates suggestions: Not discoverable. Messy. Blur the business / view separation.
- Preprocess hooks: Risky. Unfriendly. Blur the business / view separation. Unpredictable order of execution.
Conclusion
We can have an equally useful Render API with less than 10 renderables (not counting Form API):
- Design systems artefacts:
#type => component
: for UI components#type => icon
: for UI components
- Lower level bricks:
#type => inline_template
#type => html_tag
: for HTML elements#type => link
#markup
#plain_text
#lazy_builder
Keeping the beloved universal render properties:
#attached
,#cache
... and adding some related to design systems:#styles
,#tokens
,#mode
...And now we have an purely design/UI based Render API!
If we do that, step by step, deprecating slowly, not breaking anything, the display building tools from Core and Contrib will have a simpler implementation and will have an easier time introducing new features.
To play safe, we can also introduce a new Render API alongside the existing, using
@
instead of#
for example:#type => component, #component= "foo:bar"
⮕@component => "foo:bar"
#type => icon
⮕@icon => []
#type => inline_template, #template => "foo"
⮕@template => "foo"
#type => html_tag, #tag => "foo"
⮕
@element => "foo"</li> <li><code>#type =>link
⮕
@link
#markup
⮕@markup<
/li>#plain_text
⮕@plain_text
#lazy_builder
⮕@lazy_builder
Hi Christian,
Thanks for this issue. I hope I am understand what you are trying to do achieve here. I not, I will move this to its own ticket.
They are 3 levels in Field API and 3 kinds of prop types:
Indeed, we use typed_data
property only with scalar prop types nowadays:
$ grep -r typed_data src/Plugin/UiPatterns/PropType
src/Plugin/UiPatterns/PropType/UrlPropType.php: typed_data: ['uri']
src/Plugin/UiPatterns/PropType/StringPropType.php: typed_data: ['string']
src/Plugin/UiPatterns/PropType/NumberPropType.php: typed_data: ['decimal', 'float', 'integer']
src/Plugin/UiPatterns/PropType/EnumPropType.php: typed_data: ['float', 'integer', 'string'],
src/Plugin/UiPatterns/PropType/BooleanPropType.php: typed_data: ['boolean']
Let's focus on the 3 red crosses (❌) instead
So we have much to do to cover the full field API.
Every component prop type has a (JSON) schema, every prop type has a (Typed data) schema. They may be matchable.
Example: Icon field with an Icon prop
public static function propertyDefinitions(FieldStorageDefinitionInterface $field_definition): array {
$properties = [];
$properties['target_id'] = DataDefinition::create('string')
->setLabel(new TranslatableMarkup('Icon ID'))
->setRequired(TRUE);
return $properties;
}
is converted to JSON schema:
type: object
properties:
target_id:
type: string
type: object
properties:
pack_id:
"$ref": ui-patterns://identifier
icon_id:
type: string
settings:
type: object
required:
- pack_id
- icon_id
Not easy to map because it seems icon field has a single property (target_id
) for 2 prop types properties (pack_id
and icon_id
). Maybe with a separator. Let's check that with UI Icon team.
Example: LinkItem field to Links prop
LinkItem field type :
public static function propertyDefinitions(FieldStorageDefinitionInterface $field_definition) {
$properties['uri'] = DataDefinition::create('uri')->setLabel(new TranslatableMarkup('URI'));
$properties['title'] = DataDefinition::create('string')->setLabel(new TranslatableMarkup('Link text'));
$properties['options'] = MapDataDefinition::create()->setLabel(new TranslatableMarkup('Options'));
return $properties;
}
is converted to JSON schema:
type: object
properties:
title:
type: string
uri:
"$ref": ui-patterns://url
options:
type: object
Links prop type:
type: array
items:
type: object
properties:
title:
type: string
url:
"$ref": ui-patterns://url
attributes:
"$ref": ui-patterns://attributes
link_attributes:
"$ref": ui-patterns://attributes
below:
type: array
items:
type: object
Not bad :) But url
and uri
property names don't match, so we can have 2 strategies:
- We don't care about the property names ("title", "uri", "options"), we care only about the type and we let the builder do the mapping.
- Can we use a PropTypeAdapter plugin transforming
uri
andurl
for that? They were not made for this kind of flow, but it may work.
Example: Multivalued StringItem field to List prop
StringItem field:
public static function propertyDefinitions(FieldStorageDefinitionInterface $field_definition) {
$properties['value'] = DataDefinition::create('string')
->setLabel(new TranslatableMarkup('Text value'))
->setSetting('case_sensitive', $field_definition->getSetting('case_sensitive'))
->setRequired(TRUE);
return $properties;
}
Is converted to this JSON schema:
type: object
properties:
value:
type: string
So this when multivalued:
type: array
items:
type: object
properties:
value:
type: string
List prop type:
type: array
items:
type: ['string', 'number', 'integer']
It doesn't seem obvious at first, but it may be good match if we flat the single "value" field property.
Conclusion
None of my examples were a perfect match. But each of them is showing mechanism worth to study. There is something smart to do here.
One year later, I am not that sure this must be it's own component instead of just an addition to the footer component.
Not yet. Too busy
Great. Can you also change the plugin IDs accordingly?
The 2 field widgets have similar and confusing labels:
- "Component slot source" ('A field widget for a "component" source.'): the exact same label as the prop type?
- "UI Patterns slot source" ('A field widget for an UI Patterns source.'): what is the difference with previous?
we could also renovates our conversion mechanism which is now very simple and not easily alterable...
Wow! That sounds very interesting.
- a new plugin type to apply an operation from one prop type to the same prop type. those plugins may be used in components prop forms and component slot forms. One would select a source first, and one would optionally add a sequence of those plugins.
- a new plugin type to declare a conversion from one prop type to another. the introduction of those plugins would be done with a renovation of the prop type conversion system (declaration and conversion paths computation)
I agree only the first kind must be available in the admin UI, but I guess it can be a single plugin type because:
- they share the same interface
- we will also need the first kind in the revamped prop type conversion system
but i will give a try to map, every data type needs to have a chance 🤣
Thanks you so much. Can you do some performance tests? If they are more or less the same, let's avoid the introduction of a custom data type.
1
I am happily surprised to see you have added $element['#variant']
. I believe our 2 "special" component props (attributes
and variant
) needs their own (optional) render properties and I need to create the related ticket for #attributes
.
2
$component_attributes['data-component-variant'] = $context['variant'];
That's surprising too but why not.
3
variantDefinitions:
default:
label: Default
description: My default variant
Why a complicated lower camel case property: variantDefinitions
. I am already quite a few people struggling with libraryOverrides
instead of the more straightforward and expected library
. Can we have this instead:
variants:
default:
label: Default
description: My default variant
4
In my-cta.component.yml, variantDefinitions
is inside the prop property:
props:
type: object
required: []
properties: {}
variantDefinitions: {}
It is not compliant with JSON schema standard. Can we move it to the component definition root?
5
We use label
but:
- component is using
name
- slots are using
title
- props are using
title
Instead of introducing a third way of declaring the same information, can we copy the current slot structure?
6
Is it not too late?
if (empty($variant) || isset($context['variant'])) {
$template .= sprintf('{%% embed \'%s\' %%}', $id);
}
else {
$template .= sprintf('{%% embed \'%s\' with { variant: \'%s\'} %%}', $id, $variant);
}
Why tweaking the dynamically generate template instead of moving the variant to the props a bit earlier?
Non tested naive code snippet:
if (isset($element["#variant"] && other_condition_we_may_add)) {
$element["#props"]['variant'] = $element["#variant"];
}
Sorry to disturb that late, but do we really need a custom data type (Source) storing the data in JSON?
There are already data type in Drupal Core for such data structure which are (theoretically, we need to check) more performant for decoding/encoding nested PHP arrays, like https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21TypedData...
XB is also using a custom data type based on JSON, but I guess they send the exact data as HTTP Response Body, without the need of unserialize it to PHP arrays
What do you think about that?
There are multiple nice usecases.
for example, to invert a boolean value, to filter an output, to trim a string... etc etc
Indeed, and we need to scope this. I would like to introduce a restriction to avoid rewriting a full data processing framework inside UI Patterns 2 and maange unexpected side effects: we transform from the same prop type to the same prop type.
So:
- from attributes to attributes: any attribute source can be a filter, the attribuets are merged at each steps with something like AttributeHelper::AmergeCollections
- from string to string:
trim
,upper
,replace
... - from boolean to boolean:
invert
...
Is it a new plugin type? I was initially kind of investigate TypedDataFilter plugins from
https://www.drupal.org/project/typed_data →
What else can we study from Drupal contrib?
It really depends on how XB and equivalents implement the UI.
Just a side-note: Let's not implement this fetaure only to please some specific display builders. Let's do it the better way in a SDC point of view, and the display building tools will be able to add features upon it.
The key suggested seems somewhat weak for this action.
Maybe we need different keys:
suggested
: for the DaisyUI card example.enforced
: for the Carousel example
So, let's see how a display building tool can handle those:
It also might be worth exploring whether we can limit an SDC to only appear in a specific slot of a specific SDC. A use case of this is that I don't want my end user to be able to place an
accordion_item
in the main content area. It only works when placed inside ofaccordion_group
Interesting path to explore indeed:
- Is it always the reverse of
enforced
? So no need to add a new keyword? - If we need a new keyword, will it be also in the parent's slot definition ? Or in the child definition?
Let's be careful with any restriction we would add. For example, if the design system is expanded by a distinct team with new components, and one of the also want to use accordion_item
in of those slots, we need to allow that.
Thanks a lot Mike for this issue which will make a lot of people happy.
Instead of " are allowed" i would say "are suggested" because it would be a mistake for display building tools to strictly enforce this:
- sometimes business needs are surprising from a design system point of view and business always win. So, if a card component suggests
- sometimes a display builder doesn't know which component is a renderable. For example, placing a view inside a component slot. The view is build with another component, but the component is not know yet.
So, it is up to the display building too to decide how strict it will be considering this suggestion, but from a SDC point of view it is only a suggestion.
So, an early proposal with an example similar to yours, Bootstrap's accordion:
name: Accordion
description: "Render content in a box that expands and collapses vertically."
group: Accordion
slots:
content:
title: Content
description: "Accordion items."
suggested: [accordion_item]
Example with DaisyUI's Card:
name: Card
description: "Cards are used to group and display content in a way that is easily readable."
group: "Data display"
slots:
image:
title: Image
title:
title: Title
text:
title: Text
actions:
title: Actions
suggested: [badge, buttons]
What do you think?
The point of all this: thinking through what it'd mean to go much further in allowing SDC developers to specify the authoring experience for Content Creators.
Hi Wim, it has been a long time you haven't heard of my warnings so here I am again 😉
The mission of SDC developers (as component authors) is to provide the best implementation for UI components, they don't have to think about the business/applicative/CMS side, so they don't have to specify the content editor experience. Once a component author starts to think about the CMS, he is in risk of lowering the quality of its UI Component.
It is not only a theoretical stance, we are doing this with UI Patterns since 2017 and it works well. UI Patterns 2.x is the proof it still work (and work even better IMHO) with SDC: we are able to inject data to slots & props from many sources.
Content editors are not the only users of a component. An UI component has to be as usable in other contexts (non editorial, unexpected constraints, data retrieval from API...).
Thanks you.
2 possible fix:
- Add context to the translation https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Language%...
- Or, better in my opinion, change "View field" label by a less ambiguous one. (and don't forget to update the documentation)
- getChildPluginDefinitions >> getItems
- getSettingsSourceWithChildPlugin >> getMinimalSetting
- getChildPluginId >>getSelectedItem
interface SourceWithItemsInterface {
/**
* Get items.
*
* @return array
* Associative array of items definitions with ID as the key.
*/
public function getItems(): array;
/**
* Get the minimal settings for an item.
*
* @param string $item_id
* An item ID.
*
* @return array
* A source setting.
*/
public function getMinimalSetting(string $item_id): array;
/**
* Get the currently select item.
*
* @return string|null
* The item ID.
*/
public function getSelectedItem(): ?string;
Is it better?
Also:
- Do we also need a getItemLabel(string $item_id): string ?
- it may be good to move ::getMinimalSetting() to SourceInterface and to implemtn it in SourcePluginBase as a proxy to PluginSettingsInterface::defaultSettings(). What do you think?
getMinimalConfiguration does not need any parameter, unless it is static, which could be a nice idea after all.
Oops, a copy paste mistake, I fix it in my comment
Hi @freelock,
But now trying to make an SDC, we've created a menu_menu SDC and a menu_item SDC, and placed the menu_menu SDC using UI Patterns, as a block. So far so good, we have our menu.
Nice, an user of UI Patterns 👍 I hope you enjoy using it.
but it feels a bit nasty and introduces other dependencies (Twig Tweak).
Twig Tweak is nasty :) Its philosophy is the reverse of SDC one.
I would much rather simply call the child SDC (menu_item) from within the parent, but I don't have any access to the menu_item_content entity!
Are you using a slot in menu_menu
to inject menu_item
components and a slot in menu_item
to inject menu_menu components?
Maybe not because it seems you are manipulating a prop structure here:
The items do have a "below" array of child items...
Can you share with us the definition and the template of the 2 components?
I am not sure about the naming...
Proposal inspired from [ObjectWithPluginCollectionInterface](https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Plugin%21...) but not identical because we have a single collection and not many collections:
interface SourceWithCollectionInterface {
/**
* Get collection of items.
*
* @return array
* Array of items definitions.
*/
public function getCollection(): array;
/**
* Get the minimal source configuration for an item.
*
* @param string $item_id
* An item ID.
*
* @return array
* A source configuration.
*/
public function getSourceConfiguration(string $item_id): array;
/**
* Get the currently select item from the collection.
*
* @return string|null
* The item ID.
*/
public function getSelectedItem(): ?string;
Do we also need a getItemLabel(string $item_id): string
?
why is it called "Media: background image", there is nothing specific to background here, it depends of the usage in the component template, no?
grimreaper → credited pdureau → .
Hi Goz,
When we will move to entities, views & formatters for the component library, we will be able to do a formatter for that. See 📌 [2.0.0-alpha3] Use Drupal entities & plugins for the Library pages Active (expected late summer 2025)
Until then, is it possible to do it in the template? Something with json_encode()
with PRE and/or CODE HTML elements?
I am not pushing this solution, I am just asking.
Do I need to somehow define what the keys are in the associative array?
I guess you have to. You need to describe the data you expect, so someone (or something) using your component knows what to inject here.
So, you had this:
options:
type: array
items:
type: object
Processed like that in your template:
{% for option in options %}
{% if option.type == 'optgroup' %}
<optgroup label="{{ option.label }}">
{% for sub_option in option.options %}
<option value="{{ sub_option.value }}">{{ sub_option.label }}</option>
{% endfor %}
</optgroup>
{% elseif option.type == 'option' %}
<option value="{{ option.value }}">{{ option.label }}</option>
{% endif %}
{% endfor %}
So, your schema may be (non tested proposal):
options:
type: array
items:
type: object
properties:
label: {type: string}
value: {type: string}
options: {type: object}
I know there is still an undefined options
object but it is already usable like that.
Thanks so much. I will have a look.
Data primitives mapping between the 3 formats we are manipulating:
Maybe adding such a table will be useful in the SDC documentation.
Hi Liam,
I am a little worried about this sentence:
I think that I should replace
type: object
withtype: array
(because) the PHP type is an associative array.
AFAIK, here is the data primitives mapping between the 3 formats we are manipulating:
So, an associative array seems to be type: object
.
We may do a documentation session with @mherchel during DrupalCon Atlanta
I am very excited about that. There are many sides of SDC documentation which need to be clarified or completed after 2 years of practice.
I beleive we need to get rid of those PHP Namespace properties instead of extending their usage.
So, in my humble opinion, no need to add 100 lines of code for this, because:
- They were a mistake from the early days of SDC.
- They are a "drupalism", non-compliant with JSON Schema standard.
- SDC is doing weird stiff to skip JSON schema validation for them.
- They create a coupling between UI components and CMS application.
So, it may be better to create an issue to raise a warning each time someone is using those.
I believe we don't need to specify both prop_types: ['string', 'url']
.
We target the more specific, so URL, and it will also be available for the more generic, string
Can you try?
Which PHP object are you sending as prop?
Hi Kristen,
Since October:
- I am getting in touch with
sdc_styleguide
maintainer from time to time, adopting the shared story format seems still planned but no date shared yet. - UI Patterns → 2.0.0 was released in February with great traction, being the first stable module using the story format
- steepbase → module has also adopted this format in January, and I hope more will join this year.
this is not JSON schema compliant:
allowed_bundles:
image: 'field_media_image'
image_style: 'large'
We need to find a way of expressing this data in a JSON schema way.
Let's try to move fast on this issue.
So, we agreed each field item must be a single "object" and if we want a collection of them we configure the field as multivalued.
What would be this object ? I am proposing it to have at least 2 field properties:
public static function propertyDefinitions(FieldStorageDefinitionInterface $field_definition) {
$properties['source_id'] = DataDefinition::create('string')
->setLabel(new TranslatableMarkup('ID'));
$properties['source'] = MapDataDefinition::create()
->setLabel(new TranslatableMarkup('Data'));
return $properties;
}
Mapping exactly the current UI Patterns 2 form data structure.
For example, a component source:
source_id: component
source:
component:
component_id: 'ui_suite_bootstrap:accordion_item'
variant_id: null
slots: []
props: { }
A block:
source_id: block
source:
plugin_id: announce_block
A block with configuration:
source_id: block
source:
plugin_id: 'views_block:content_recent-block_1'
'views_block:content_recent-block_1':
id: 'views_block:content_recent-block_1'
label: 'Recent content'
label_display: ''
provider: views
views_label: 'Recent'
items_per_page: '5'
An entity field:
source_id: entity_field
source:
derivable_context: 'field:node:article:status'
'field:node:article:vid':
value:
add_more_button: ''
'field:node:article:status':
value:
add_more_button: ''
Fabien, Christian, Mikael, are you OK with those 2 first field properties? Which other properties do you need?
These processors can be configured during the mapping process. This can also help to reduce the mapping if a config field exists.
Interesting but it looks like a distinct issue, no?
Thinking aloud: Is it limited to slot sources ? Why not opening this alter to prop sources?
Discussed with Mikael. We are not altering the source but the source value:
hook_component_slot_value_alter
(component_slot_value
)hook_sdc_slot_value_alter
(sdc_slot_value
)hook_ui_patterns_slot_value_alter
(ui_patterns_slot_value
)hook_slot_value_alter
(slot_value
) if the previous ones are too long
But I still prefer the "slot_source" one ;)
Naming is hard :)
We are allowing the alteration of a slot source value:
protected function buildSlot(array $build, string $slot_id, array $definition, array $configuration, array $contexts): array {
...
foreach ($configuration["sources"] as $source_configuration) {
...
$source_value = $source->getValue($slot_prop_type) ?? [];
$this->moduleHandler->alter('component_built', $source_value, $source_configuration);
}
So, instead of hook_component_built_alter
(component_built
), we can propose hook_component_slot_source_alter
(component_slot_source
)? Is it too long?
[attributes] string value found .."
This issue is well known.
Your attributes
value is going through the SDC validator, which is normal when using UI Patterns 2.
But the validator receive a PHP object which is casted as a string and raise an error because it doesn't expects a string, and this is not normal because UI Patterns 2 is preventing that.
What is happening if you skip header.twig
and call directly your component from region-header.html.twig
?
Also, it is better to use "with_context: false" with {% embed 'my_components:header' ...
Duplicate: ✨ [2.0.1] StoryPluginManager::getComponentStories() must return sorted results Active
So, moved to 2.0.1.
No need to implement the full CategorizingPluginManagerInterface
because stories are not goruped
Some people also want weighted results, but this will be another task.
yes, I forgot about the other one. Thanks
I think everybody have differnt use cases in mind. We should merge our ideas.
Indeed. Different use cases doesn't necessary mean different implemntations.
ok i will work on clarifying the scope
This is an important error to raise in my humble opinion.
This prop schema doesn't tell enough about the prop and the expected data. It is not useful for the component users and for tools loading the component into Drupal, like UI Patterns 2 → or Experience Builder → .
What kind of data is expected in each object item? Which properties ?
Do I need to give a more specific definition, such a the class or an interface?
No PHP classes. Let's stick with legit JSON schema ;)
Do you know what to add?
Some references:
From Liam in 🐛 The "empty is not needed" test is not correct Active
This condition:
{% if variable is not empty %}
raised the warning "The exact same as just testing the variable, empty is not needed."But if I switched it to:
{% if variable %}
then the condition would fail ifvariable
was zero.
Let's check if empty
is really same as just testing the variable before setting our "liberal" list of warnings.
Hi Liam,
Thanks for your feedback. Let's discuss about this in the more generic issue: 📌 Adopt a more liberal approach for TwigValidatorRuleTestExpr Active
is it related to 📌 Ternary and default tests update with Twig 3.19 Active ?
It is a very nice SDC component! I love the facts you are not using Twig blocks fro your slot, iri-reference
for URL prop, and both the definition and the template are very clean 👍
A few feedbacks however...
Little cleanups
Slots are not typed, so you need to remove "type" property:
slots:
content:
type: string
description: Message content.
It seems you are using BEM methodology for the class naming, so this:
{%
set classes = [
'toolbar-message',
'toolbar-message--type--' ~ type,
]
%}
Would be better written like:
{%
set classes = [
'toolbar-message',
'toolbar-message--' ~ type,
]
%}
data-drupal-tooltip
Don't use a slot as an attribute value. A slot can have anything, including markup, and may break the browser DOM.
setAttribute('data-drupal-tooltip', content).
So, what can we do? The obvious options would be:
- Make content a string prop instead of a slot, especially if only plain text is expected here
- Create a new string prop for this attribute value
However, the best would be to remove those attributes from the template:
setAttribute('data-drupal-tooltip', content).setAttribute('data-drupal-tooltip-class', 'admin-toolbar__tooltip')
Because they tell us about the context how/where the component is used instead of being proper parts of the component.
I would inject them from outside (non tested proposal):
'#type' => 'component',
'#component' => 'navigation:message',
'#props' => [
'type' => 'warning',
'url' => $url,
'attributes' => new Attribute({
'data-drupal-tooltip': content, # raw? processed? or something else?
'data-drupal-tooltip-class': 'admin-toolbar__tooltip'
})
],
Side note
I am recommending executing drush sdcv umami
from
https://www.drupal.org/project/sdc_devel/ →
to automatically get useful warning and errors.
There are still some rough edges (unclear messages, false positives...) but this tools is already usable.
Hello Liam,
sdc_devel validator is working on the AST (abstract syntax tree) from the already parsed template, so sometimes 2 different template syntaxes are parsed as the same nodes and trigger the same warnings/errors.
However, it seems those expressions produces clearly different nodes:
is not null
is parsed as https://github.com/twigphp/Twig/blob/3.x/src/Node/Expression/Test/NullTe...variable ??
is parsed as https://github.com/twigphp/Twig/blob/3.x/src/Node/Expression/Binary/Null...
@mogtofu33: What do you thing about that?
i will have a look
@doxigo Thanks a lot for you in investigation. Can you tell us more about the root cause on Drupal Core side?
In UIP2, I would say the prop type is string
Sure about that? Why not a specific prop type?
What would be the JSON schema of this prop type?.
Hi Liam,
We are currently changing our mind and agreeing with you. That's why we have such a ticket.
2.1.0 is becoming busy. I propose to move this to 2.2.0 with the other source plugins additions.
Why not use #children as 3 lines above.
I still believe #children
is an internal property of the render API and we are not supposed to use it.
Except 3 messy snippets, it is used by Drupal Core only in the Render API internals:
- web/core/modules/views_ui/src/ViewEditForm.php
- web/core/modules/layout_builder/src/Controller/ChooseSectionController.php
- web/core/modules/system/src/Controller/DbUpdateController.php
- web/core/lib/Drupal/Core/Render/Renderer.php (many times!)
- web/core/includes/form.inc (many times!)
- web/core/includes/theme.inc (a few times)
Thats wrong. Nobody will understand it:).
You are talking about presenter templates, right? Nobody understand them already :) and they will disappear soon hopefully. that's why we are extra careful and future-proofing in SDC templates, but we care less in presenter templates.