Component props form: map textarea, bool, and select elements to React components

#3461422: Evolve component instance edit form to become simpler: generate a Field Widget directly

Fixing d.o's classical "unpublish if cross posting" bug 😬

(Clarifying issue title because "styles" could be interpreted as referring to the styling of components, which is not what this is about at all.)

I postponed this on #4, because that should make this clearer/simpler to achieve, because all of the extraneous wrapping markup will disappear in #3461422.

But … based on what I see in the attached design, most of this is actually purely matter of updating ui/src/components/form/*.tsx and the associated CSS/JS? 🤔

(The associated CSS/JS is not yet being loaded, but that'd be A) not to hard to add, B) we can bypass that by simply not yet doing dynamic loading and just always loading needed CSS/JS for #3454094: Milestone 0.1.0: Experience Builder Demo. IOW: just expand the experience_builder/xb-ui asset library in the short term.)

@bnjmnm, where is there a gap in my reasoning?

Just discussed in detail with @jessebaker, we agree that despite the very non-ideal current right-hand sidebar form, it actually is possible to start working on the styling/visual design matching of individual form elements today. See updated issue summary for details 😊

#3461422: Evolve component instance edit form to become simpler: generate a Field Widget directly landed last night, none of the "non-ideal" bits of #8 are true anymore: this is now very easy to act on!

Updated the issue summary accordingly.

Please stop using the term "component edit form" — it's far too easy to confuse with \Drupal\experience_builder\Form\ComponentEditForm.

Of these 5:

1. Plain text
2. Textarea
3. Link
4. Toggle
5. Select

the following are already available:

1. Plain text → StringFieldWidget aka <input>
2. Link → UriWidget aka <input type="url">
3. Toggle → BooleanCheckboxWidget aka <input type="checkbox">

The 3 others will follow once #3463583: Use the `all-props` component for testing form backend + frontend integration lands. And once that's in, there will be many more available.

Actually, probably makes sense to just do everything we can here. "Select" will be unblocked as soon as #3463583: Use the `all-props` component for testing form backend + frontend integration lands, which is on the verge.

Note: Textarea will be trickier, because SDCs currently have no way of signaling that what they want is multi-line text. 😅 So that part will need a follow-up. Also, it's not yet clear whether this truly is "Textarea" or "Rich text". @lauriii should clarify that upon his return next week.

#3463583: Use the `all-props` component for testing form backend + frontend integration is in! 👍

I'll start with this.

Also some general pointers that i need clarification with:-
1) So we have only a few form elements which have a custom(created by us) jsx component ex:- textArea and input. So do we need to create all the custom for elements just like the one created in this MR for Select(not completed robust though)?
2)If we have to create custom elements as asked in point 1 then would just mapping the components in twigToJSXComponentMap load them in the DummyPropsEditForm.
3) And we are missing the Toggle component in the DummyPropsEditForm.Shall that be taken care in a seperate issue?

Assigning it to Ben for a few questions that i have on this.

#19: rather than awaiting @bnjmnm's answer (which happens close to your EOD), I'd prefer you'd instead just implement/try that, so that @bnjmnm can review what you did and then add potentially missing docs.

#19.3: yes, you can leave that out for now. Let's get this MR going with the ones that you can work on 🙏

Assigning it to @bnjmnm as this is where I'm stuck.

Assigning it back to me, probably figured out what needs to be done.

1) So we have only a few form elements which have a custom(created by us) jsx component ex:- textArea and input. So do we need to create all the custom for elements just like the one created in this MR for Select(not completed robust though)?

Yes, create elements.

2)If we have to create custom elements as asked in point 1 then would just mapping the components in twigToJSXComponentMap load them in the DummyPropsEditForm.

For any new components added, you also need to add a .twig file in experience_builder/templates/form that defines the data types variables passed to the template. These are the same variables passed to a regular twig template, and defining the data types lets us know if it's a variable that should immediately react-rendered or if it should be passed as a prop.

Look at the existing files there to see the naming conventions and file contents should be and you'll likely have a good sense of what is needed. Get as far as you can and leave specific/targeted questions here and I'll get them answered asap.

Left a couple of questions on MR.
So the current state of the MR has styling fixed for the following elements:-
1) Plain text.
2) Select

The remaining elements :-
1) Textarea (this is blocked on #15)
2) Toggle (we don't have any toggle element in the DummyPropsForm)
3) Link(A set of questions related to this are already being posted on the MR).

Assigning it to Ben majorly to answer questions related to link element. Also at some point we might need Lauri's feedback for the Textarea.

I'll take care of the initial scaffolding for this then unassign myself. Out of the ones listed, input and select are fairly straightforward but the other 3 could get frustrating without some initial setup - I'll do that initial setup.

The Link in the screenshot is of a single URI field. A link field in drupal is a two textfields -> combination of URI + The link text. Can the requirements be updated to either provide a text+uri link widget example, or clarify the expectations as being a URI field? If it's the former, we'll need to provide a component with a link field, in addition to the string in URI format that is currently there.

For Toggle, I can wire this up and test with a temporarily added field to confirm it works, but since there isn't (as far as I know) a component that has this yet you'll just need to believe me. This is similar to the <Textarea> obstacle mentioned earlier.

IOW: link as used colloquially by non-technical users, when they really mean URL.

(Anecdotally, many non-technical family & friends literally ask "Can you send me that link?", and they expect a URL, not URL+text.)

FYI, this is exactly why \Drupal\experience_builder\JsonSchemaInterpreter\JsonSchemaStringFormat::computeStorablePropShape() does:

      static::URI, static::IRI => new StorablePropShape(shape: $shape, fieldTypeProp: new FieldTypePropExpression('uri', 'value'), fieldWidget: 'uri'),

👆That's expressing that {type: string, format: uri} must use the uri field type (\Drupal\Core\Field\Plugin\Field\FieldType\UriItem), not the link field type (\Drupal\link\Plugin\Field\FieldType\LinkItem). The docs for hook_storage_prop_shape_alter() then show how you can override that to use the link field type nonetheless:

  // Override the default field type + widget for the `format: uri` string shape
  // from the `uri` field type to the `link` field type.
  // @see xb_test_storage_prop_shape_alter_storage_prop_shape_alter()
  // @see \Drupal\Tests\experience_builder\Kernel\HookStoragePropAlterTest
  if ($storable_prop_shape->shape->schema == ['type' => 'string', 'format' => 'uri']) {
    // @see \Drupal\link\Plugin\Field\FieldType\LinkItem::propertyDefinitions()
    $storable_prop_shape->fieldTypeProp = StructuredDataPropExpression::fromString('ℹ︎link␟uri');
    // @see \Drupal\link\Plugin\Field\FieldType\LinkItem::defaultFieldSettings()
    $storable_prop_shape->fieldInstanceSettings = [
      // This shape only needs the URI, not a title.
      'title' => DRUPAL_DISABLED,
    ];
    // @see \Drupal\link\Plugin\Field\FieldWidget\LinkWidget
    $storable_prop_shape->fieldWidget = 'link_default';
  }

👆that is disabling the "title" part of storing a link, at which point the difference with a uri field is tiny.

Added the 3462310-scaffold-contextual-form-inputs MR - it's gonna fail CI for sure, but has textarea, Uri(Link) and Select Working (though note the ComponentsPropForm on the back end is not currently getting options for the select so I'll need to poke around. The regular text field has always worked, so all that's left is Boolean, and I'd like to discuss with some other backend folks about how to best implement

@Utkarsh_33 you are welcome to either merge 3462310-scaffold-contextual-form-inputs into your branch or just work directly on it. I'll be making changes to it as well but we're focusing on different parts, and I'm fine fixing merge conflicts if need be.

Here's a rewording of #31 with the questions a little more explicit

• I'm adding a few things temporarily with form_alter so it is possible to build the components even if there aren't props widgets for them yet. I'd like to provide something so toggle could be worked on - what is something I could add to a $form array that would best resemble what will be rendered once a boolean prop is available in the all props component?
• For the enum/select, the select options are not making it to $form array, formTemporaryRemoveThisExclamationExclamationExclamation provides a select widget with only none as an option even though the all-props component has options. I'm sure I can get that fixed but wanted to first check if anyone here would immediately know where to go before I start digging.

#32:

• This is referring to experience_builder_field_widget_complete_form_alter() in https://git.drupalcode.org/issue/experience_builder-3462310/-/compare/0.... AFAICT. But that is just doing very generic things.

  Ah, no, you're asking for +1 to make that do more things, hackier things…

  +1 for temporary hacks — those are then clear identifiers for follow-up issues that should get rid of those hacks 😊

• You say that {type: string, enum: […]} does not work, but it did work in #3461499: Support complex SDC prop shapes: introduce (Storable)PropShape to compute field type storage settings — see the screenshots there! So I wonder what regressed. 😬

  Same pragmatism here though — and I'm quite hopeful it's a trivial fix :)

RE second bullet, debugged it and found the culprit: #3467712: Follow-up for #3461499: Add fieldInstanceSettings to `(Candidate)StorablePropShape` + `StaticPropSource`. Here's the fix:

diff --git a/src/Controller/SdcController.php b/src/Controller/SdcController.php
index 1a1510f8..ae64c628 100644
--- a/src/Controller/SdcController.php
+++ b/src/Controller/SdcController.php
@@ -143,7 +143,10 @@ final class SdcController extends ControllerBase {
             : $this->getDefaultValueFromPropInfo($prop_info);
         }
         if ($storable_prop_shape->fieldStorageSettings !== NULL) {
-          $keyed_choices[$component_prop->propName]['sourceTypeSettings'] = $storable_prop_shape->fieldStorageSettings;
+          $keyed_choices[$component_prop->propName]['sourceTypeSettings']['storage'] = $storable_prop_shape->fieldStorageSettings;
+        }
+        if ($storable_prop_shape->fieldInstanceSettings !== NULL) {
+          $keyed_choices[$component_prop->propName]['sourceTypeSettings']['instance'] = $storable_prop_shape->fieldInstanceSettings;
         }
       }
       $assets = AttachedAssets::createFromRenderArray([

Looks like bool/toggle wasn't blocked on anything back-end, so I just added it to the SDC all props component

And the fix in #34 got the select to work without a crash.

So with that, the 3462310-scaffold-contextual-form-inputs branch has working versions of all the template types. Now we pass it to @Utkarsh_33 to make them look good + clean up any CS violations I might have done when setting that up.

The current state of the this issue is as follows:-
1) The Linkfield is styled almost close to the figma design.
2) The Select field is also designed almost close but for now hardcoded the width which could be changed later.
3) The Toggle field is a Switch component from redux library. As there was not much info about the toggle switch's design so i have not styled that much.

The main problem is with the Plain text as the styles are not getting applied.I tried using the css modules for styling as suggested by Jesse but that's also not working even after the class is getting correctly attached to the correct element.

None of the Cypress tests are running because

> tsc --noEmit --pretty
src/components/form/Select.tsx:9:40 - error TS7006: Parameter 'option' implicitly has an 'any' type.
9   const defaultValue = options.filter((option) => option.selected)?.[0]?.value;
                                         ~~~~~~
src/components/form/Select.tsx:17:23 - error TS7006: Parameter 'option' implicitly has an 'any' type.
17         {options.map((option, index) => (
                         ~~~~~~
src/components/form/Select.tsx:17:31 - error TS7006: Parameter 'index' implicitly has an 'any' type.
17         {options.map((option, index) => (
                                 ~~~~~
Found 3 errors in the same file, starting at: src/components/form/Select.tsx:9

😅

Re

The main problem is with the Plain text as the styles are not getting applied.I tried using the css modules for styling as suggested by Jesse but that's also not working even after the class is getting correctly attached to the correct element.

This is now resolved.
So I think that all 4 of the fields namely:-
1.Plain text → type: string
2.Link → type: string, format: uri
3.Toggle → type: boolean
4.Select → type: string|integer:number, enum: […]
are now close to the designs. The only field remaining in the scope of this issue is the TextArea field.

Assigning it to @jesseebaker for an initial round of review.

#32 landed in its own issue: #3469729: Follow-up for #3467712: field instance settings lost in ComponentPropsForm.

Thanks for pushing forward with this. I’ve added some design/UI feedback to the MR. I’ve not yet done a code review. I will leave that until the UI is looking correct unless you have any specific parts of the code that you would like feedback on before you continue.

- Remove the dark grey border around inputs/selects - Done
- Spacing between all field types should be the same - Done(do we want a particular value of spacing?)
- Either make the side panel resizable using this handle or remove it.Either way, it needs more margin around it so it doesn't touch the labels.(opened an issue for this here.
- Border for Url element is dark black -Remaining
- Font size between input and select is slightly different. Also padding inside the form element - done(set to 15px)
- Controls at the top are pushed over to the right - Added a temp fix in e667ba58.

Remaining stuff:-
1) Using these toggles does not update the preview
2) Can these be made to sit side by side with the label on the left and the toggle switch on the right( I need help with this. I tried styling using css(inline-flex styling) but that is not working.This is wrapped around the inputBehaviours which is causing it to appear it this way.)
3) Either make the side panel resizable using this handle or remove it.Either way, it needs more margin around it so it doesn't touch the labels.(opened an issue for this here).

Linking the issue @utkarsh_33 created for #47.3.

@bnjmnm Could you help unblock @utkarsh_33 on #47.1+2 or provide pointers? 🙏

Re #47

Using these toggles does not update the preview

They were updating the preview until this commit. which changes the native <input> to a Radix component. I suspect it's a matter of re-shaping the props/attributes to whatever structure Radix needs. In particular there's an onChange callback in the attributes object that is necessary for syncing UI. It looks like this would need to be assigned to <Switch.Thumb> based on a quick scan of the docs (assuming that <Switch> the right component to use - I"m unsure)

Can these be made to sit side by side with the label on the left and the toggle switch on the right( I need help with this. I tried styling using css(inline-flex styling) but that is not working.This is wrapped around the inputBehaviours which is causing it to appear it this way.)

This is not related to inputBehaviors, nothing there is presentational. Like twig, form elements are wrapped with a form element template - in this case FormElement. Also like Twig, this template also determines if/where labels are displayed (note the titleDisplay).

With that established, you have several choices for how the input/label relationship is set up for the <Toggle>.

• Add conditionals to FormElement that add styles to the wrappers to display the label/input next to each other and update the label position option so the label appears first
• Set all these properties in a preprocessor, before it even gets to the front end
• Configure it (either via preprocessor or FormElement so the label doesn't display there and pass that responsibility to the <Toggle> component. and consult the Radix docs for how to build this more granularly.

The styling issue is fixed now(except a small question here).But the preview is not updating.Will try to dig more into that.

Re #49

They were updating the preview until this commit. which changes the native to a Radix component. I suspect it's a matter of re-shaping the props/attributes to whatever structure Radix needs. In particular there's an onChange callback in the attributes object that is necessary for syncing UI. It looks like this would need to be assigned to based on a quick scan of the docs (assuming that the right component to use - I"m unsure)

I tried that but it's not updating it somehow.I also backported to but that was also not working atleast on my local.

But the label and switch are now appearing inline.Left a few more question related to CSS which could be done quickly once clarified.
Marking it NR and assigning it to @bnjmnm(as there are some small doubts) so that we can get this across the finish line soon.

Right now the MR is presenting boolean inputs with a Switch component. The screenshot in the issue summary does not show a Switch. Can either the issue summary or MR be updated to reflect what we should be working on - whichever one is the actual intended goal.

Only @lauriii can answer #55.

Just seeing this issue... I skimmed the comments but there are a lot! o_O

Perhaps it doesn't change your course here, but hopefully everyone knows about the new designs:

https://www.drupal.org/project/experience_builder/issues/3454094#comment...

Re #58

Do we need to write this much custom CSS for this? Can we not use the properties that are provided by Radix Themes

I think we should be using the Radix properties wherever possible and I don't believe there's anything preventing this on a technical level.

#59: So you mean: we should be able to use https://www.radix-ui.com/primitives/docs/components/switch and then use CSS to make it look like #58?

#58:

I was imagining that this element from the design is a boolean:

To be fair, that looks awful lot like the "Settings" vs "Page data" tabs. 😅 (See the screenshot of the design in #3469235: Make "page data" tab in right sidebar work.)

Is this something that i can pick up and try to reduce the custom css that we have added or there are still things that needs clarification😅.

I made use of possible properties that are provided by Radix Themes. This issue still requires clarification from either Ben or Lauri about the Boolean field.

So the only think left on this is the toggle behaviour.It should update the preview as well as the component state but it's not happening.I will try to get it done.
By the mean time assigning it to @balintbrews for a design review on this.

By the mean time assigning it to @balintbrews for a design review on this.

Whenever asking for a visual review, please always at minimum include a screenshot. That might help the reviewer not even having to get this MR running locally, because there may be obvious problems.

IOW: please empathize with the reviewer, and think of ways to facilitate them 🙏😊

So here is a detailed update on what is the current state of this MR.
1) I have added boolean property to my-hero component in this commit.The toggle updates the preview rendered by twig correctly(except for the first time as the value rendered for the first time is not correct).After that every time you toggle the element it updates the preview.
2)There seems some problem in all-props components form introduced in this issue.As soon as we update any value inside all-props it breaks the app giving a 500 error with as

{
    "status": "PARSING_ERROR",
    "originalStatus": 500,
    "data": "The website encountered an unexpected error. Try again later.",
    "error": "SyntaxError: Unexpected token 'T', \"The websit\"... is not valid JSON"
}

.

So i think functionality wise every custom created jsx element is working correctly(except for the all-props component) and I think if @balintbrews can take this for some css clean-up.

I have attached the video recording for demonstrating that the toggle works correctly for the hero component.
In the mean time i would try to find out the root cause for:-
1) Why the initial render of the toggle component in the my-hero component is not rendering the correct value.
2) Also try to figure out how to fix the problem of all-pros form editing that is crashing the app.

The PHP test fails are legit and can be ignored because i added a new boolean prop to all props component just to demonstrate that the toggle updates the preview correctly.

I still was not able to figure out the problem with editing of the all-pops form.If i try to edit or change any property of the form it breaks the app with

{
    "status": "PARSING_ERROR",
    "originalStatus": 500,
    "data": "The website encountered an unexpected error. Try again later.",
    "error": "SyntaxError: Unexpected token 'T', \"The websit\"... is not valid JSON"
}

very less helpful error.It still says that this problem is occuring in file Preview.tsx.
Narrowing it down and finding the solution i thought it must be something related to layout or model which is being used here to set FrameSrcDoc and also analyzed the requests being sent to the backend.

Here are the takeaways from that debugging:-
1)The request sent to the backend is sending the correct state of the props(true or false) ie it updates the model correctly.
2) The error that shows up has very less data or stacktrace which i could have followed to reach to the problem.

Assigning it to Ben for now as Balint in not working on this for now.If @bnjmnm could help me unblock on this i could work on suggestions if any on Monday.
Apart from this I have also attached the video in #69 which shows that how toggle works as expected for the other props components.

So i just figured out the problem in the all-props component is due to the

Drupal\Core\Render\Component\Exception\InvalidComponentException: [test_string_enum] Does not have a value in the enumeration ["foo","bar"] in Drupal\Core\Theme\Component\ComponentValidator->validateProps() (line 203 of core/lib/Drupal/Core/Theme/Component/ComponentValidator.php).

.If we remove the select component then it does update it correctly.
I'll remove the extra props that i added in the my-hero component.

@Utkarsh_33, are you planning to add anything else here? If not, please assign the issue to me, then I'll finish off the styling.

Hi @balintbrews i the only thing left in this MR is the fixing the problem for the select component.I have created a separate issue for it Select component gives an error on selecting none..
Rest everything is working correctly here.

Everything is updating correctly excepts for the components that have select props as a part of it as select breaks the whole app.I think it's in good shape for you to take over.Assigning it to you.
Fell free to assign it back to me if you think we should fix the select component functionality first and then fix the styling issues.

Here is what I propose. Let's review and land this knowing that the styling needs to be improved both in terms of polishing and how we leverage Radix Theme design tokens. Those can be addressed in a follow-up issue instead of getting this stalled. That would make reviewing easier and also to work on the follow-up issue less complex due to all the other changes in the MR. 😇

#3470962: Add boolean prop to `all-props` component landed a trivial supporting piece of this MR. 👍

#70 + #71: for that we have #3470995: OpenAPI validation errors must be provided as a JSON response + #3470321: Surface verbose OpenAPI errors in the UI — for better bug reports and faster DX.

@utkarsh_33 I've proposed a solution for an edge case you encountered here, see #3471511: `enum` data shapes: error when choosing "- None -" in `<select>` — please review 🙏

First unbroke the MR, by resolving a conflict: https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...

Then paired with @bnjmnm, to figure out why things were failing in the build for that commit. Turns out that we're hitting a new edge case for input--SOMETHING.html.twig. We discussed and agreed that it'd make more sense for those two files to be omitted and deferred to follow-up issues that are more tightly scoped — they'll be child issues of #3463842: [META] Redux sync on ALL prop types, not just ones with a single [value] property.

I trust @bnjmnm's judgment which parts of this MR to land and which parts are more reasonable and feasible to land in a follow-up :)

It's not much on its own, but opens up all sorts of excellent followups.

I reworded the title to better describe the work that had been done. Follow-up issue for styling: #3472199: Component props form: style plain text, link, textarea, bool, and select elements.

👍 Thanks, @balintbrews!