[1.0.0-beta1] Remove Island's specific Form classes

Thanks you goz. That looks exciting.

However:

• Why did you add IslandPluginFormTrait to ResetButton?
• Instead of adding $current_island_id on onUpdate() method, is there an other way of passing the data? Ideally, this MR will only modified the 4 island plugins with forms.
• Can you fix the pipeline feedbacks first? https://git.drupalcode.org/project/display_builder/-/merge_requests/13/p... (only the stuff related to your MR)

Why did you add IslandPluginFormTrait to ResetButton?

You are right, i miss this one.
It's a plugin, i should add plugin form logic here to.

Instead of adding $current_island_id on onUpdate() method, is there an other way of passing the data? Ideally, this MR will only alter the 4 island plugins with forms.

Unfortunatelly not, or may be using another static service...

Everything is green except css lint. Yipee !

I take a look on ResetButton and we are good

MR can be reviewed

The warning in the pipeline is not about this work. So ok for review.

Currently it seems the form lost it's state, at least for UI Styles and UI Tokens.

From an empty display builder with ui suite bootstrap:

• Add a button
• Add a token inside, fill a text
• Apply a style and a token to the button and the token (font h2, border color and background color for example)
• They are applied properly, but not kept in the instance form
• Refresh the page
• They are applied properly, but not kept in the instance form

Expected is to have the style and token selected in the instance form with a message (current behavior on 1.0.x).

Still have the issue.
In current BlockStylesForm::buildForm, the $data['styles'] contains the selected value as:

[
  "styles" => [
    "selected" => [
      "typography" => "display-1"
    ]
  ]
]

On this branch, the code is now in UiStylesPanel::buildForm as $this->data['styles']. Problem is it's nested in:

  "_third_party_settings" => [
    "ui_styles" => [
      "styles" => [
        "selected" =>  [
          "typography" => "display-1"
        ]
      ]
    ]
  ]
]

So it does not apply.

It's strange, in the last commit, i take care of taking value from _third_party_settings.

In IslandPluginBase::build() :

    // First, get specific data for the plugin.
    if (isset($data['_third_party_settings'][$this->getPluginId()])) {
      $this->data = $data['_third_party_settings'][$this->getPluginId()];
    }

I was reproducing #13, i don't reproduce it anymore.
Are you sure you are up to date with the MR ?
Or may be you make another test than #13 which fails ?

My bad it's working better. The switch between the 2 branches made a false positive.
But I have an other problem that I thought was related but not, when loading a fixture like bootstrap demo it has warning when rendering for each element:

Warning: Uninitialized string offset 0 in Drupal\Core\Render\Element::property() (line 29 of /var/www/html/web/core/lib/Drupal/Core/Render/Element.php)

But this code do not touch the rendering, so I guess there is a change somewhere in the data of the builder saved, will investigate.

Moving for now to alpha2 to have time to finalize.

I don't reproduce, in an instance without this MR, nor in instance with this MR.
I hadn't loading fixtures before, may be you were on an existed fixture ?

I will also have a look

when loading a fixture like bootstrap demo it has warning when rendering for each element:
Warning: Uninitialized string offset 0 in Drupal\Core\Render\Element::property() (line 29 of /var/www/html/web/core/lib/Drupal/Core/Render/Element.php)

From a fresh install, on commit d6a3f04d46a21c15e66a8b62fe25b8a9e3c98ebe (HEAD -> 3529103-1.0.0-beta1-remove-islands, display_builder-3529103/3529103-1.0.0-beta1-remove-islands):

1. I install & set default ui_suite_bootstrap from /admin/appearance
2. I go to /admin/structure/display-builder/instance/add
3. I create a demo from [display_builder_devel] Ui suite bootstrap demo
4. I also have Warning: Uninitialized string offset 0 in Drupal\Core\Render\Element::property() (line 26 of core/lib/Drupal/Core/Render/Element.php).

My bad, digging for the error, it seems i previously met this, but certainly fix it in ui_patterns or elsewhere which is not in display_buider... and forgot about it.

The following error

     Warning: Uninitialized string offset 0 in Drupal\Core\Render\Element::property() (line 29 of /var/www/html/web/core/lib/Drupal/Core/Render/Element.php)

comes from a radio element with an option with empty key :

{
  "#type": "radios",
  "#title": "Typography",
  "#options": {
    "": "- None -",
    "h1": "<span class=\"ui-styles-source-select-plugin-option h1\">\u0391\u03b1<\/span>Heading 1",
(...)
  },
  (...)
  "#id": "edit-styles-wrapper-text-ui-styles-typography",
  "#name": "styles[wrapper][text][ui_styles_typography]",
  "#value": "",
  "": {
    "#type": "radio",
    "#title": "- None -",
    "#return_value": "",
    "#default_value": "",
    (...)
    "#name": "styles[wrapper][text][ui_styles_typography]",
    "#value": "",
    "#ajax_processed": false,
    "#sorted": true,
    "#isDisplayBuilder": true
  },
  (...)
  "#isDisplayBuilder": true
}

Strangely, i have this issue only for this MR, not from 1.0.x and in 1.0.x, this radios element is not loaded (so no error). BUT the radios element is not displayed, so i guess the MR process something which should not

Difference between 2 versions :

In 1.0.x, UiStylesPanel build form is called only when island is panel island is displayed when clicking on the icon.
In MR, UiStylesPanel build form is called when page is loaded

So MR change more things that expected

I rebased my branch to not get other commits.

This issue was very hard, hope there will be no more errors.
Fingers crossed.

Still have the issue, as soon as UI Styles island is enabled, I have multiple warning.

I have no warnings :/

Can you give me more informations to reproduce ?

Which theme do you use, do you start from a fixture, on which component ?
May be a screencast will help

Thanks

I will review too. Team work :)

From a fresh install, on commit c2d2277ce764be85497a01810aa6e991996f3a00:

1. I install & set default ui_suite_bootstrap from /admin/appearance
2. I go to /admin/structure/display-builder/instance/add
3. I create a demo from [display_builder_devel] Ui suite bootstrap demo
4. I still have Warning: Uninitialized string offset 0 in Drupal\Core\Render\Element::property() (line 26 of core/lib/Drupal/Core/Render/Element.php). just by loading the builder

Still have the issue, as soon as UI Styles island is enabled, I have multiple warning.

If I disable UI Styles panel in /admin/structure/display-builder/default, no more warning when I load the builder.

I i enable back UI Styles panel in /admin/structure/display-builder/default, warnings are back.

Sorry for the false positive, i forgot to remove some tests in core which make my instance not relevant.
I spend a big part of my weekend on it... and burn too much neurons.

Finally, took less time this afternoon to fix it once i reproduce again.

Thanks for the hard work, let's go with that!

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