🇫🇷France @just_like_good_vibes

looks ok for merge. then i just rebased to prepare merge

just_like_good_vibes → made their first commit to this issue’s fork.

i updated the code, would you test again?

back to it

Option 5

just_like_good_vibes → created an issue.

let's note, we clean the data for normalization but not everything.

g4mbini → credited just_like_good_vibes → .

- we need a comment into the convert to indicates normalization will clean bad strings

- update noramlization to clean properly

ok, seen during the weekly

we will deprecate the "HTML class source", still usable the same way.

but, the recommended source is Attributes, and the processing of the value is more elaborate.
we check if "=" sign is inside the value,
- if yes : in the attributes string format
- otherwide : a list of classes

so validation and getPropValue are changing for the better :)

let's update also the default source

i don't know why it's for me, i send it back to christian :)

i added a few minor changes.
and removed the unwanted tests. please review Christian :)

should we rename some props ?

we need to address this more carefully, the initial work posted by sourav_paul did not carefully check for existing attributes in the tags.

just_like_good_vibes → created an issue.

finally...

just_like_good_vibes → made their first commit to this issue’s fork.

ok i pushed a better fix

ok Christian, for review please :)
it should be ready for merge

ready for review :)

ready for review, who wants to take it?

i think we already have everything except :

- using a view : using field formater inside a views row field. --> belongs to ui_patterns_field_formatters
- using a component block in the UI -> we have this in layout-builder but not in ui place blocks --> belongs to ui_patterns_blocks

looks like 🐛 Offcanvas component - Error when using custom ID Active is fixing the issue, can someome check please ?

ok now it's better :)

hmm no, it's not normal.
i need to investigate and i will post a correction

i just rebased the MR

ok i added them :)

don't worry, i already fixed

Hello, i wonder if, like me, other people are needing this?
it would be great to continue the work.

hello,
in fact this is a core bug about treating correctly unicode in pcre patterns.

the bad code is in FormElementBase, function validatePattern.

Our regex supports unicode, but drupal form and "#pattern" processing is not enough advanced to allow it.

i will submit a patch

please test now :)

i will fix this

i will take this

did some massive work over there, please review :)

i think we should improve two prop types : ListPropType and NumberPropType.
A the moment, their normalization is not strong enough imho.

let's tackle this in separate tickets.

just_like_good_vibes → changed the visibility of the branch 3469665-test-proptypeinterfacenormalize to hidden.

hello, it looks like now it works, i can see the checkboxes.
could you please check @smustgrave ? thank. you :)

i just gave a test with a fresh 11.1.0 drupal and the last experience_builder from dev branch.

i did not receive the mentioned errors.
i managed to check the UI and almost nothing worked. My components were appearing but they are still in such an early stage, it seems difficult to check anything yet.

hmm yes, i see sorry. i forgot that one.
so yes, we have this little mechanism but it covers only some sources, the ones deriving from SourcePluginPropValue.
Those sources are a bit specific : They have only a 'value' setting key, and they may use it in their form.
in those particular cases, we process the default value indicated in the prop json schema.
We can keep this behavior for those sources, but what about other sources ?

list of sources deriving from SourcePluginPropValue :

• Drupal\ui_patterns\Plugin\UiPatterns\Source\CheckboxesWidget
• Drupal\ui_patterns\Plugin\UiPatterns\Source\CheckboxWidget
• Drupal\ui_patterns\Plugin\UiPatterns\Source\ClassAttributeWidget
• Drupal\ui_patterns\Plugin\UiPatterns\Source\NumberWidget
• Drupal\ui_patterns\Plugin\UiPatterns\Source\SelectWidget
• Drupal\ui_patterns\Plugin\UiPatterns\Source\SelectsWidget
• Drupal\ui_patterns\Plugin\UiPatterns\Source\TextfieldWidget
• Drupal\ui_patterns\Plugin\UiPatterns\Source\UrlWidget

other noticeable sources (i also used the current ones from ui_icons and i filtered out the ones with slot in the prop_types)

• Drupal\ui_icons_patterns\Plugin\UiPatterns\Source\IconSource
• Drupal\ui_patterns\Plugin\UiPatterns\Source\AttributesWidget
• Drupal\ui_patterns\Plugin\UiPatterns\Source\BreadcrumbSource
• Drupal\ui_patterns\Plugin\UiPatterns\Source\EntityFieldSource
• Drupal\ui_patterns\Plugin\UiPatterns\Source\EntityLinksSource
• Drupal\ui_patterns\Plugin\UiPatterns\Source\EntityReferencedSource
• Drupal\ui_patterns\Plugin\UiPatterns\Source\FieldPropertySource
• Drupal\ui_patterns\Plugin\UiPatterns\Source\ListTextareaWidget
• Drupal\ui_patterns\Plugin\UiPatterns\Source\MenuSource
• Drupal\ui_patterns\Plugin\UiPatterns\Source\PathSource
• Drupal\ui_patterns_field_formatters\Plugin\UiPatterns\Source\FieldLabelSource
• Drupal\ui_patterns_views\Plugin\UiPatterns\Source\ViewTitleSource

hey hey, what about Option 4) we keep the strip tags behavior in string prop types and we do not introduce a new prop type to inject HTML into props. BUT, we implement this : if the JSON schema definition has contentMediaType: "text/html" , then we allow HTML to be placed into the string prop, and the normalization and preprocess on our side behaves in a different manner.

now i vote for option 4

To add to this "brainstorm", imagine a slot with a default value which would be an insane render array or any random markup.
it is not realistic to have any slot source to be able to support that value as being "casted" into a source form.

Honestly, i would rather let sources be free to implement or not the default value handling, and we need maybe to add some little things to the current code :

> do we show/indicate the default value in prop/slot description ?
> do we add a "DefaultValue" source which job is to empty the source settings and let the default value be injected for sure ?

I am not sure about the mechanism of default value we need/want, and the exact one we have today.
To my knowledge, we were not relying at all in sources, on the default value in the prop being edited.
but i may be wrong.
Right now, each source is independent and it would be each source responsibility to read from the prop definition, and fill the defaultSettings of the source, in order to make that value appear in the form.

so right now, default value in the definition seems not used in sources.

but what do we want to support ?

Question 1) : should the default value in prop definition appear on the source form that is editing that prop ?

Question 2) : If it appears, should we have "revert to default?" mechanism, maybe a "default value" checkbox, a defaultValue source or whatever?

Question 3) : When should we really apply default value? in other words, when is a source value really "null" and be replaced to the default. What if we want to really put null as a value?

i find the remark interesting, but still we need to decide what to do :

- Before 🐛 LinkPropType normalization...suite Active , string prop type was unfiltered. String prop types are supposed to have only string and not tags, but it seems some people were abusing the original intent of prop types and (mistakenly) used them to convey html markup. We were like authorizing this (through twig usage or through weird sources like token ? )

- After 🐛 LinkPropType normalization...suite Active , which is now : we are stripping tags. Is-it a mistake? should we re-allow tags inside strings? (by the way, tags from forms won't work, like input for example, twig will filter them i guess) ?

Option 1) : we keep the strip tags. I would say then "work as expected". People are not supposed to free-ride string props to put markup in it. They should re-think their SDC component model and use slots for that. The benefit of that strip tags filtering is the ability to have a predictable behavior of string prop types. from a twig usage perspective, people can inject "stuff" (e.g. html) that may be the result of a rendering, and that stuff is cleaned by our normalization process to become nice string.

Option 2) : We remove the strip tags. We open the gate to strange things. but some people would be happy to not rewrite their SDC component, and ui patterns will support their strange usage. still this is not a real html support yet, would we allow also form elements ? (in that case a specific treatment in preprocess should be added).

Option 3) : We keep Option1), but we introduce the new prop type Pierre was talking about. Then, weird usage of string props with tags, would work if and only if SDC component definition would be patched with contentMediaType: "text/html" . That would be a compliance requirement of ui patterns 2. more elegant solution with a nasty new prop type.

Honestly, i like option 3) but it is still an open door to strange things with props. But strange things are going into a specific prop type at least instead of free-riding the nice String prop type.

we need to "vote", for the best option.

i take it back to check and finish for merge

just_like_good_vibes → created an issue.

one comment about the "with_context=false".
I knew the linter wanted to put "with_context: false", but it is written everywhere that the right code portion is "with_context=false".
so i was like confused.
what do you guys think?

you mean adding an automatic test for stories? or which test(s)?

Hello Christian,
you set in review but the MR is not green ?
do i still need to review it?

great!

hello,
oups i think i already added things in 🐛 LinkPropType normalization...suite Active

just_like_good_vibes → created an issue.

thanks for the feedback :)

just_like_good_vibes → created an issue.

And finally, i have corrected and give more strength to our normalization processes.

My advice, is that we could be more systematic to this approach, and factorize code a lot to do that process of "cleaning" what is injected into the components.
but, well, let's continue for now with that version, which seems to cope well with all our the submitted problematic usecases (i hope i did not forget any).

new additions with feedback from multiple persons :) thank you all

see 🐛 LinkPropType normalization...suite Active , we stay closed this issue and work in a new clean one.

according to comments from 🐛 [2.0.0-rc1] normalization of slots triggers some unwanted filtering Active ,
there are new work to do on normalization.
i have opened a new separate issue https://www.drupal.org/project/ui_patterns/issues/3492211 🐛 LinkPropType normalization...suite Active

just_like_good_vibes → created an issue.

hello,
this belongs to another ticket, because it is linkproptype normalization and not slot

Issue is rebased :)

an error occured here, a pattern appeared instead of component. we reopen to merge the error.

just_like_good_vibes → made their first commit to this issue’s fork.

another one, more recent and with more information is a duplicate.
📌 [1.1.0] Replace component() function by include() Active

I just submitted the new proposition.
When no default value is defined for an enum :
- if enum has type compatible with string, and if not required, the default value is now empty string
- otherwise default value is first enum value

yes we need to figure out the options.

First, how things are declared using SDC, in json schema

- type
- required
- default

The current code in normalize is trying to get the "default", like this :

return static::normalizeEnumValue($value, $enum) ?? static::enumDefaultValue($definition);

We are discussing the possibilities of changing the enumDefaultValue returned.

when there is not "default" declared in json schema, and when the type is explicitely defined (and would be further checked by SDC)
we indeed return the first enum value.
Perhaps we could refine this :
- if required is defined and true in the definition, we returned the first enum value (and not empty string)
- otherwise : depending on the type, we try to return a default 'empty' value .
-- type can be an array or a scalar if i am not wrong. when string is a compatible type, we return empty string,
-- otherwise, what do we return ? first enum value?

one again, we are in an edge case, where :
- the definition has not indicated a default value,
- the value passed is not part of the enum declared
- the enum is explicitely typed and the value will be validated.
- we try to guess a default valid value to avoid a failed validation, and the first enum is not good.

in the case of enum of type string, when declared without required, one would expect that it means optional with default value being empty string.

"I want the prop to remain optional, so without value."
for this, it is a bit more tricky to do.
if you prop is of type "string", and you don't declare any "default",
and you still want an empty string as default :) ?

is empty string a possible enum value ?
and also i would rather put empty string as default...
because sometimes enum could have different types, mixed types..etc

i am curious about what Pierre thinks about that. maybe i am completly wrong in my approach.

just_like_good_vibes → created an issue.

ready for review (and merge :) )

ok :)

just_like_good_vibes → created an issue.