Compatibility between SDC and the Form API

Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to the Core change policies.

(if you need form API assistance message chx on Drupal Slack. Response times should be below 24 hours but usually below 4 hours.)

Hello,

@ghost of drupal past, thanks for the proposal.

First POC, trying to tell Element::children to got looking into #slots if the element is a component make the form element inside the component to be prepared by the Form API, but:

1. It remains duplicated on rendering (one textfield outside the component, one inside)
2. The 2 elements have the same name attribute, so when submitting the form it is the element in the component which value is retrieved but I think it i only a side effect of having duplicated name.
3. "parent" mechanism in the form API may be confused/malformed.

So next step will be to try to put slots as component element child and so adapt SDC (and later table element too).

Edit: with @pdureau we discussed to introduce a new property on render elements that will indicate Element::children() the properties to consider as child like. But previous results makes this approach very side effect prone.

Following, I wrote a number of form components for ui_suite_uswds but really serves no purpose :(

I think I did use the checkbox component for uswds in the checkbox.html.twig file but it was messy. SO yay to here that!

Yesterday session is online: https://www.youtube.com/live/6AvlzYN17Rs?t=25741s

Until presentation support is posted somewhere you can go give a look.

From @nod_ feedbacks, I am:
- creating another branch with containing only core changes.
- creating a dedicating plugin instead of altering ComponentElement
- need to find a way to make submitted values work without using #name

In MR https://git.drupalcode.org/project/drupal/-/merge_requests/11866, I removed what is only for #3508641: Define form elements from SDC so that in this issue, we can focus only on SDC and Form API compatibility.

#name is not usable as it is right now because of Ajax.

Currently poking a field widget which will be able to use components, the settings form is broken.

Ok, we found a way to get rid of the #name. Just need to ensure no numeric key.

Before:

$form['component_card'] = [
    '#type' => 'component',
    '#component' => 'core_sdc_form:card',
    'content' => [
      '#type' => 'component',
      '#component' => 'core_sdc_form:card_body',
      'content' => [
        [
          '#type' => 'textfield',
          '#title' => $this->t('Textfield in content'),
        ],
      ],
    ],
];

After:

$form['component_card'] = [
    '#type' => 'component',
    '#component' => 'core_sdc_form:card',
    'content' => [
      '#type' => 'component',
      '#component' => 'core_sdc_form:card_body',
      'content' => [
        'foo' => [
          '#type' => 'textfield',
          '#title' => $this->t('Textfield in content'),
        ],
      ],
    ],
];

This is looking promising, adding various review tags.

Demo gif in #3508641-12: Define form elements from SDC.

Pushing current work state before cleanup.

With the fix from #3520242: Incorrect render structure in Navigation PageContext, the proposed changes in Renderer and ComponentElement do not break existing tests.

Problem with SDC on component with multiple inputs, solved by using [] on name in the component.

We tried to create a lat/lon component with geofield, not working because of applicative
logic into the field type.

Checkbox in form API directly: OK
Checkboxes in form API directly: OK

Usage of names to handle multiple inputs.

Problem of unchecked checkbox currently handled in Checkbox.php form element, workaround in field widget in UIP 2.

TODO:
- field storage and field setting mapping into the component
- test checkboxes as field widget
- test with multiple values fields in field widget
- handle data type constraints?

Hello,

Is there still enough time to make this issue pass for Core 11.2?

Regarding MR https://git.drupalcode.org/project/drupal/-/merge_requests/11866, What would be the new tests to write?

I would say a form with form elements inside components like:

$form['component_card'] = [
      '#type' => 'component',
      '#component' => 'core_sdc_form:card',
      'header' => [
        '#markup' => (string) $this->t('Card'),
      ],
      'content' => [
        '#type' => 'component',
        '#component' => 'core_sdc_form:card_body',
        'content' => [
          'foo' => [
            '#type' => 'textfield',
            '#title' => $this->t('Textfield in content'),
          ],
          'bar' => [
            '#type' => 'select',
            '#title' => $this->t('Select in content'),
            '#options' => [
              'option_1' => $this->t('Option 1'),
              'option_2' => $this->t('Option 2'),
            ],
            '#empty_option' => $this->t('Empty option'),
          ],
        ],
      ],
      'footer' => [
        '#type' => 'textfield',
        '#title' => $this->t('Textfield in footer'),
      ],
    ];

Check that form elements are correctly rendered. And possible to submit the form.

More stuff?
Kernel or Functional test?

And I forgot, comment 8. Thanks @chx for your help proposal :)

Hi,

I made a MR with the tests only: https://git.drupalcode.org/project/drupal/-/merge_requests/12030

And added the test on the main MR: https://git.drupalcode.org/project/drupal/-/merge_requests/11866

I think the Nightwatch fail on the main MR is not related to the changes.

Nightwatch fails often, you can rerun it.

As for test only, you don't usually need a separate branch, there is a test only job, it's at the bottom of the list of tests.

If you run that it will run your tests without the fix or feature.

It should fail.

@godotislate, review feedbacks addressed. Thanks!

@nicxvan thanks, I totally forgot the test only was already out-of-the-box in pipelines.

And yes, now Nightwatch tests are green.

We should commit #3520242: Incorrect render structure in Navigation PageContext before this issue so that the git history is clearer for people in the future then this can be rebased. I think it's included here so that tests pass.

One question on the MR.

Reviewed the MR, agree with @catch about the type specific logic. We can do this in the render element itself - linked to an example.
If we can't get that done before the alpha deadline it could go in like this with a followup to move it to the element plugin.

Hi,

Thanks everyone for the reviews and feedback.

Looking at it to update MR.

Hi,

Requested changes addressed.

I got a random failure on a CKE5 JS test, after retriggering the job it is ok.

I will test changes against https://git.drupalcode.org/project/drupal/-/merge_requests/11345, to check for #3508641: Define form elements from SDC.

You can re-review in the meantime if you want.

Other MR https://git.drupalcode.org/project/drupal/-/merge_requests/11345 rebased and updated.

It still works!

Build failed. Mayb pipelines had a transient error, and the build needs rerunning?

Also, the MR should be moved out of draft if it is ready.

No, the MR to look for for this issue is https://git.drupalcode.org/project/drupal/-/merge_requests/11866 which is green.

MR https://git.drupalcode.org/project/drupal/-/merge_requests/11345 was to test this issue and #3508641: Define form elements from SDC as it has overlapping changes and to ensure changes in this issue will not be incompatible with the other.

Pushed a new approach that should be more amenable and also stops the Renderer knowing about a special case for ComponentElement plugin.
Given the net difference my changes from my last review were one unset call and the removal of the special case in the renderer I think I'm still eligible to RTBC this so going to do so. We still have some 'Needs {x} review' tags that need to be removed. I assume @pdureau can do the 'Needs frontend framework manager review' one. Would be good to get a +1 from the SDC maintainers too.

Latest on here looks great. Happy to commit this although additional +1s would be good.

Thanks @larowlan!

Indeed much simpler.

I haved test it with #3508641: Define form elements from SDC and #3519934: [2.2.0] Add ui_patterns_field_widgets sub-module, POCs on those issues are still working.

So +1 for me.