Allow specifying default props values when opting an SDC in for XB

Wim Leers → credited larowlan → .

FYI: @larowlan also observed the need for default values for components independently, at https://git.drupalcode.org/project/experience_builder/-/merge_requests/2....

📌 "Developer-created components": mark which SDCs should be exposed in XB Fixed landed, but a new blocker was identified: 📌 Add ::calculateDependencies() to ComponentTreeItem Active .

Looks like 📌 Add ::calculateDependencies() to ComponentTreeItem Active is largely done, so unpostponing: work here can begin with just building on top of that MR, while the test coverage for that MR is sorted out. 👍

📌 Add ::calculateDependencies() to ComponentTreeItem Active is in!

Let's restrict the issue scope like I proposed:

I think it might be best to not do any of that in this issue, and instead restrict this issue to only supporting the field types that have a single scalar property.

Small iterations that can land quickly result in more visible progress and fewer conflicts 👍

I'll update the issue summary and will create the follow-ups, you just focus on the implementation work, Felix 😄

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

To clarify the scope/intent of the issue, the attached patch showcases the additional configuration I want to see this issue add support for, and for which a UI should be built.

The result will be that components, when dragged onto the canvas, will start out with these default values, which will ensure they'll be visible from the start.

+++ b/config/schema/experience_builder.schema.yml
@@ -13,6 +14,61 @@ experience_builder.component.*:
+                Choice:
+                  # @todo Currently, FieldForComponentSuggester assumes an entity context. We need to extract a new FieldTypeForComponentSuggester service out of that *first*. Until then, hardcode a list of possible field type choices.
+                  - string
+                  - uri
+                  - file
+                  - image

Created 📌 FieldForComponentSuggester must be able to work without an entity context Needs review to unblock this — ready for review.

@wim-leers I've got a few questions I'd like to clarify about the scope of this issue:
UI - do imagine this being multi-step process form along the lines of “select component option -> ajax callback loads field type selection -> another ajax callback loads field widget selection -> selecting that results in another ajax callback that appends correct widget form”, rinse and repeat for each required prop?
Is the UI at this stage supposed to be “nice”, or just “working”? Thinking how much time is worth spending on trying to build nice(ish) UX for this using Drupal forms and ajax callbacks.
For entities saved from array of values (tests/config import/etc) - should validate if the “combination of values” is actually valid? For example, if “field type” and “field widget” values are actually compatible?
As per your example - "defaults" property in schema is not marked as nullable (not sure if that's the correct schema constraint) - this means that for all component config entities "defaults" will be made mandatory, right?
Do we need to update `src/Controller/SdcController.php` to include default values in `/xb-components` output as part of this issue?

“Just working” for sure, it can totally be clunky.

I’d encourage using #states to keep it as simple as possible in this first iteration.

Do not bother with validating field type and widget compatibility for now, just add a @todo. Only validate what is easy to validate, which is what I already added!

Nullability: excellent point! I forgot that. My thinking: required SDC props require default values, others don’t. But let’s start out with them all being optional.

Exposing defaults via the controller: need to look closer, will do.

It's not clear to me whether this is for:

- a default value, which would pre-populate the widget for a component, and be saved as entity data.

- a 'show if empty' value (like a fallback image) which is pulled from the layout/xb configuration only when there is no entity data. (views 'no results' behaviour and argument defaults are somewhat similar-ish).

If it's the latter, or even both (?), the terminology might need changing?

#13: I'd expect it to be the former.

If a component is backed by field data, which takes precedence? The default values for the field, or the default values for the SDC?

It's indeed the former.

Would be a better name?

But … isn't that exactly the same behavior as the default value you configure for a FieldConfig (field.field.*.*.*: default_value)? So why is the name confusing?

#15: I'm pretty sure that it would be both easier and would make more sense from UX perspective to use fields default value. The fields default value is specific to the current context, meaning that it could have a value that is more specific than what the component has. The components prop would have a PropExpression as a value so at least in my mental model it should just render the default value from the field that is powering it. I'd imagine in this scenario that if there's no default value for the field, that the default value for the prop would not take effect, because otherwise you run into challenges in the scenarios where you want to be able to leave the value empty.

If we were to do wise versa, we would somehow have to bubble up the default value from the component prop to the field. That seems pretty unexpected consequence to me.

This is all just my hypothesis, we'd definitely do user testing on this later once we have a version of this implemented.

But … isn't that exactly the same behavior as the default value you configure for a FieldConfig (field.field.*.*.*: default_value)? So why is the name confusing?

If it's a default value, then it's not confusing, my 'both' meant if there's an option between default value and 'show on empty' somewhere, which sounds like it's not the case.

However, also see the question in #15 because I think that might be a source of confusion.

#15: An SDC does not have default values. It's got examples, and they're optional. If that were required, we would indeed not need this issue/MR to define default props values!

But we'd still need this issue to specify which field type/widget should be used by default when using/placing an SDC, to populate its props.

To expand what @lauriii wrote in #17:

1. opt in an SDC to XB — that creates this config entity, that with this issue in, will also require you to specify a default field type + default field widget + default value (a StaticPropSource) for each SDC prop
2. consequence: when placing this SDC/component, it is guaranteed to have a visual representation
3. consequence: when placing this SDC/component, you'd be able to edit it using a choice of field type + widget that the Site Builder made, removing the need for the Content Creator to choose it
4. later, by 🌱 Milestone 0.2.0: Experience Builder-rendered nodes Active , the Content Creator will be able to not use a statically defined value (a StaticPropSource) and switch it to use one of the values on the host entity (e.g. the article's image, the article's title … — a DynamicPropSource — which is basically only a PropExpression to retrieve that value, as mentioned by @lauriii) if there is a field in the content type that matches the shape of the SDC prop
5. much later, the Site Builder would be able to specify in the content type template (see 🌱 [later phase] [META] 7. Content type templates — aka "default layouts" — affects the tree+props data model Active ) that a component placed in the default layout has a prop populated by a field (a DynamicPropSource).

IOW: this issue is only a stepping stone to unblock the UI demo targeted for 🌱 Milestone 0.1.0: Experience Builder Demo Active . Because it provides a default that is independent of (content entity) context, it only needs to support StaticPropSource. Once we have a content type template, you'd be able to use a DynamicPropSource. See EndToEndDemoIntegrationTest for a walkthrough of how that works. (Docs for that coming later.)

If we were to do wise versa, we would somehow have to bubble up the default value from the component prop to the field. That seems pretty unexpected consequence to me.

Agreed that would not make sense.

Which is why — as explained in the bullets above — we'd never have a DynamicPropSource in this component config entity that is being worked on here: because there is no (entity) context when opting in an SDC using this component config entity.
DynamicPropSource will be available as soon as there is an entity context, which is true in two places: when creating a content entity, or when defining the default layout/content type template (see 🌱 [later phase] [META] 7. Content type templates — aka "default layouts" — affects the tree+props data model Active ).

The default value being stored here is not a valid (default) value for a field. Try adding a "plain text" field to e.g. the Page node type, and specify a default value. This is what ends up in the config:

default_value:
  -
    value: 'this is the default'

i.e. [0 => ['value' => 'this is the default']]

The logic in the current MR tries to be clever and stores only the first delta's main property:

defaults:
  text:
    field_type: string
    widget_type: string_textfield
    default_value: this is the default
    expression: …

That will not work when we eventually add support for multiple-cardinality fields (and hence also defaults), which will be relevant for SDCs that have props that need lists of values.

That bug hid my excitement — it's really cool to start see this working! 😄

Note that this is a hard blocker to make e.g. the default component work in the UI, because without a default value set for its required image prop, it fails/refuses to render — give 📌 Connect client & server, with zero changes to client (UI): rough working endpoints that mimic the UI's mocks Needs review a try to see that happening, that is what this will unblock! 🚀

I fixed the bug in #20.

🐞 Remaining bug: the expression should be that for the chosen FieldTypePropExpression, because that's what's necessary to construct a StaticPropSource. It should not be the SDC prop expression. So for example, if you chose the string field type, it'd be ℹ︎string_long␟value.

Missing (I did not specify all of these in the issue summary — but reviewing it made me realize this):

1. Form UI: making required SDC props require to have default values
2. Form UI: matching that requiredness: only required SDC props should have #open ⇒ TRUE
3. a new Component::getDefaultStaticPropSource(): StaticPropSource method that converts that default value into an actually usable StaticPropSource! 😄
4. Tests: expand ComponentValidationTest
5. Tests: new ComponentTest kernel test to verify ::getDefaultStaticPropSource() works as expected

Note that updating any API logic that powers the UI is out of scope here, for that we have 📌 [PP-1] HTTP API: update /xb-render-component/{component_id} to use Component config entity's default values Postponed as a follow-up 😊

This not being ready is causing @bnjmnm to make slower progress, so stepping in to finish this while @f.mazeikis is not available.

#22:

1. Form UI: making required SDC props require to have default values
2. Form UI: matching that requiredness: only required SDC props should have #open ⇒ TRUE
3. a new Component::getDefaultStaticPropSource(): StaticPropSource method that converts that default value into an actually usable StaticPropSource! 😄
4. Tests: expand ComponentValidationTest
5. Tests: new ComponentTest kernel test to verify ::getDefaultStaticPropSource() works as expected

The last 3 points are now complete. Now working on the first 2 points.

#23:

Note that updating any API logic that powers the UI is out of scope here, for that we have 📌 [PP-1] HTTP API: update /xb-render-component/{component_id} to use Component config entity's default values Postponed as a follow-up 😊

Actually,

1. Form UI: making required SDC props require to have default values
2. Form UI: matching that requiredness: only required SDC props should have #open ⇒ TRUE

The first one is not currently possible, because as the issue summary describes, it is for example impossible right now to specify/store a default value for an image field.

The second one is better solved by using Vertical Tabs.

So did both.

Follow-ups created.

The issue that @bnjmnm is working on that I've been referring to: 📌 Add component instance edit form to contextual panel RTBC .

Finished manually testing the edit form that was added here. I got the tests passing by specifying the necessary config by hand, that was easier 😅 Now I can confirm it actually works correctly, but I had to fix a bunch of things. I had not yet realized the ComponentEditForm was not yet running config validation, that'd have saved me a lot of time. But … now it is. So any future iterations will be far less painful 🚀

Now this is truly ready to go — and @f.mazeikis has approved the MR in the mean time — I've essentially reviewed his work and built on top of it. His words: 😊

Per #3460232-9: Default props values should support files/images → .

Automatically closed - issue fixed for 2 weeks with no activity.