Use the `all-props` component for testing form backend + frontend integration

This component already exists, for the very reasons cited here: tests/modules/sdc_test_all_props/components/all-props/all-props.component.yml.

It's existed for \Drupal\Tests\experience_builder\Kernel\SdcPropToFieldTypePropTest, precisely for — but long prior to us having the ability to generate widgets, just finding viable storage candidates.

(It has even existed before the 0.x branch was created!)

And I'm literally on the path towards making exactly the intent of this issue feasible: #3461499-16: Introduce new `PropShapeInput` config entity type → .

So, can you please check if anything is missing from that all-props component? If you can't find anything missing, we can mark this issue as obsolete and unpostpone the issue it is blocking.

Great, thank you for pointing that out!

@bnjmnm can you confirm if this meets the requirements or if more work is needed?

If this winds up providing all the necessary fields, then there should be at least an install hook added to sdc_test_all_propsso it is available as a Page Builder component as soon as the module is installed.

I attempted to add the "All Props" SDC as a Page Builder component and got
ValueError: "" is not a valid backing value for enum Drupal\experience_builder\SdcPropJsonSchemaType in Drupal\experience_builder\SdcPropJsonSchemaType::from()

If the error 👆 is addressed then I can review how this renders in the edit form and recommend if anything additional is needed.

(I also noticed sdc_test_all_props.info.ymlhas a dependency on sdc:sdc which shouldn't be needed now that it is in core)

Addressed "" is not a valid backing value and digging into additional errors that surfaced once that was addressed.

I'm getting an error on any props of type: integer

InvalidArgumentException: Field test-integer is unknown. in Drupal\Core\Entity\ContentEntityBase->getTranslatedField() (line 616 of  
                                         /Users/ben.mullins/Sites/drupal/core/lib/Drupal/Core/Entity/ContentEntityBase.php).      

Which happens after calling getWidget() in formTemporaryRemoveThisExclamationExclamationExclamation

I'll test this with 📌 Support complex SDC prop shapes: introduce (Storable)PropShape to compute field type storage settings Fixed — that issue should make this one much easier to push forward 🤞

#7 reproduced, and now with a failing test:

• commit: https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...
• CI: https://git.drupalcode.org/project/experience_builder/-/pipelines/233901

Working on it, and as of commit e83470a84c89907bcdd73898485e5a78f03a383f, you should be unblocked here, @bnjmnm. For now, it'll require you to have 📌 Support complex SDC prop shapes: introduce (Storable)PropShape to compute field type storage settings Fixed applied.

But really, the point here is that such an "all possible shapes" component exists (which it already does) and has a working right sidebar (which will be the case after 📌 Support complex SDC prop shapes: introduce (Storable)PropShape to compute field type storage settings Fixed ).

So: there's nothing left to be done here for now, except wait until #3461499 is done, and then revisit.

I've been working on expanding the test coverage over at #3461499 to verify that we truly can generate all the necessary widgets for the all-props component. I've already found a number of edge cases that indicate why @bnjmnm could not possibly have gotten this to work 😅

This is definitely hard-blocked on #3461499, and soft-blocked on #3463999 — we won't quite need that to be finished too, because I can just attach a sample config entity here 👍

Stay tuned.

📌 Support complex SDC prop shapes: introduce (Storable)PropShape to compute field type storage settings Fixed is in.

LOLOL an hour worth of debugging only to find out that SDC prop names may contain dashes but Twig variable names must not: https://github.com/twigphp/Twig/issues/1967 — for example

    {{ test-REQUIRED-string }}

is compiled by Twig to:

        // line 5
        yield $this->extensions['Drupal\Core\Template\TwigExtension']->escapeFilter($this->env, ((($context["test"] ?? null) - ($context["REQUIRED"] ?? null)) - ($context["string"] ?? null)), "html", null, true);

… which means that the DX of SDC is leaving a lot to be desired: if Twig doesn't support this, and SDC supports only Twig, then I expect SDC to not allow this either. This is so utterly absurd 🤪

Tagging to file an upstream SDC bug report.

Voila, @bnjmnm is now unblocked 😊

@bnjmnm Feel free to merge when you like — this is ready as far as I'm concerned.

If you want, you can add more props to test against. But if this suffices, then I think it's ready for you to confirm that you can trigger the above (by installing sdc_test_all_props), then you and others working on the front end can test/improve field widgets using this SDC 😊

I can't add this as a Page Component, but I suspect it will be easy to address as the error specifies a line that was committed only yesterday

 Warning: Undefined array key "#markup" in Drupal\experience_builder\Controller\SdcController->preview() (line 370 of                                            
                                          /Users/ben.mullins/Sites/drupal/modules/custom/experience_builder/src/Controller/SdcController.php) #0                                                          
                                          /Users/ben.mullins/Sites/drupal/core/includes/bootstrap.inc(108):

Odd. Will double-check tomorrow with a fresh install.

composer require drush/drush
vendor/bin/drush si standard --account-name=root --account-pass=root --site-name=LiveDemoWhatCouldGoWrong --yes
vendor/bin/drush pm:install experience_builder --yes
vendor/bin/drush pm:install sdc_test_all_props --yes

Followed by creating node 1, then going to /xb, results in the screenshot I posted in #17. I didn't need to change anything.

Any chance you just checked out this branch and already had the sdc_test_all_props installed, which would've caused the optional config to not get installed, which would explain ? 🤔

(Fixed the warning in #19 though!)

Starting from scratch gets the component to install and I can see it in the XB UI and it does its job nicely

The problem I ran into earlier (or something similar to it) seems to be happening still - the edit/add form for this SDC isn't working. This is less urgent since it installs with the module now, but here's what I'm running into when I try to edit in admin/structure/component/edit/sdc_test_all_props:

If I try to edit the all-props component when I click submit on the form, the submission is actually blocked by JS and there are console errors: An invalid form control with name='sdc_test_all_props+all-props[<<daterange field|UUID field>>][field_type]' is not focusable. ​. It was reasonably easy to conclude this is due to the date range and UUID props not having field types selected - but still required more sleuthing than I'd expect a normal user to do (I could see this being out of scope here since it is a test-only component)

Setting the field types in the form allowed me to submit the form, but I got the error

Error: Call to a member function massageFormValuesTemporaryRemoveThisExclamationExclamationExclamation() on null in  
                                          Drupal\experience_builder\Form\ComponentEditForm->copyFormValuesToEntity() (line 317 of                              
                                          /Users/ben.mullins/Sites/drupal/modules/custom/experience_builder/src/Form/ComponentEditForm.php).    

Since this is test only and VERY helpful to have, I think it's reasonable to address the edit form issues in another issue. For the time being, I recommend adding something specific to the edit form for this component that warns about this and perhaps makes the edit fields unavailable "This test-only component is provided as-is and cannot be edited" so we don't run into contributors losing their (or our) time assuming this would work.

the edit/add form for this SDC isn't working.

+

[…] when I click submit on the form, the submission is actually blocked by JS and there are console errors […]

Ahhh, now I get why you said it wasn't working 😅

That's known. Because configuring each component's field type + widget etc in detail using a UI is non-desirable per @lauriii. That's why 📌 Support complex SDC prop shapes: introduce (Storable)PropShape to compute field type storage settings Fixed ignored that limitation, and why 📌 Auto-create/update Component config entities for all discovered SDCs that meet XB's minimum criteria Fixed is next, and why once that is done, 📌 [PP-2] Remove Component config entity's add/edit form Postponed will happen.

TL;DR: the current ComponentEditForm only works for components whose props are simple enough, not for arbitrarily complex SDC props.

I think it's reasonable to address the edit form issues in another issue

Ahead of you — it will disappear shortly, in 📌 [PP-2] Remove Component config entity's add/edit form Postponed 😊

For the time being, I recommend adding something specific to the edit form for this component that warns about this and perhaps makes the edit fields unavailable "This test-only component is provided as-is and cannot be edited" so we don't run into contributors losing their (or our) time assuming this would work.

Good point, we should make sure people don't waste time until #3464025 lands, done:

… and just to make sure: other components do not get this warning:

I tried this out and it works great. If anything is missing NBD - we just add to it later - as-is this unlocks a bunch of things we've been aiming to do.

#25++

This makes doing 📌 Component props form: map textarea, bool, and select elements to React components Fixed simpler, see #3462310-14: Component props form: make form elements match design → — and in fact, with this now in, a superset of the 5 things listed in #3462310 is now readily available 👍

See y'all in 📌 Component props form: map textarea, bool, and select elements to React components Fixed !

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