Removing a DS layout does not restore the region of fields

This shouldn't have been a new bug because of Layout Plugin vs core Layout API, in theory it worked the same.

Field Layout has this code in FieldLayoutEntityDisplayTrait, which is used when replacing EntityDisplay classes

  public function setLayoutId($layout_id, array $layout_settings = []) {
    if ($this->getLayoutId() !== $layout_id) {
      // @todo Devise a mechanism for mapping old regions to new ones in
      //   https://www.drupal.org/node/2796877.
      $layout_definition = $this->getLayoutDefinition($layout_id);
      $new_region = $layout_definition->getDefaultRegion();
      $layout_regions = $layout_definition->getRegions();
      foreach ($this->getComponents() as $name => $component) {
        if (isset($component['region']) && !isset($layout_regions[$component['region']])) {
          $component['region'] = $new_region;
          $this->setComponent($name, $component);
        }
      }
    }
...
  }

Which points to #2796877: Devise a mechanism for mapping old regions to new ones when moving between layouts

Eventually this logic will be in EntityDisplayBase, so it will solve the problem...
Not 100% sure what DS needs to do here but will think on it

Wait, do you mean selecting in the UI, then unselecting without saving? If so that's much worse.

If you mean selecting DS in the UI, saving, then unselecting and saving again:
The first save changes the region of the field, which is good. But the second save isn't unselecting, which is bad.

I think we'll need #2851631: Expand EntityDisplayInterface to prepare for native layouts, so that "getDefaultRegion" is available to us.

So yeah, we should reset the region of the core fields when removing a DS layout.

It looks like we at least have a core problem too: moving the 'links' around triggers a js error (so probably all 'extra fields' have this problem).

Facing same problem with latest developmental releases of Drupal 8.x-3.x and Display Suite. I saved the Display Suite Settings to some layout and then revert back to default Layout "None". The field regions get changed and fields are either Disabled or Hidden completely. I am forced to go to Display Suite layout to be able to view them. But there is another problem with DS that I cannot drag hidden Fields to the active regions. Issue: https://www.drupal.org/node/2855327. DS is thus not usable as of now.

DS doesn't need to store the field regions in 3rd party settings anymore. It's in the core schema now.
And that's the problem here. When dragging things around in the UI (once #2855327: Drag-and-drop does not work for some fields is fixed), the Field UI properly updates the region for each field.
But when changing the layout, \Drupal\ds\Form\ChangeLayoutForm does not handle this.

In FieldLayout, every time the layout is changed, this code runs:

    if ($this->getLayoutId() !== $layout_id) {
      // @todo Devise a mechanism for mapping old regions to new ones in
      //   https://www.drupal.org/node/2796877.
      $layout_definition = $this->getLayoutDefinition($layout_id);
      $new_region = $layout_definition->getDefaultRegion();
      $layout_regions = $layout_definition->getRegions();
      foreach ($this->getComponents() as $name => $component) {
        if (isset($component['region']) && !isset($layout_regions[$component['region']])) {
          $component['region'] = $new_region;
          $this->setComponent($name, $component);
        }
      }
    }

That @todo is precisely about building a UI like DS's change layout UI.

A short term fix would be to only handle the "reset to none" case, and manually set each component to have a region of 'content'
The real fix would be to migrate from the 3rd party settings to let core handle it.

I can live with that short term fix for now.

Can someone also confirm the second part I wrote in #6 ? It looks like extra field (or at least 'links') triggers a js error on core only.

DS doesn't need to store the field regions in 3rd party settings anymore. It's in the core schema now.

Hmm. Yes, we should probably drop the 'regions' key in the third party setting and move the region of the DS fields (e.g. title, submitted by) into the properties of each field (where we store plugin_id, weight etc). But I'd do that in a different issue though.

Looking for a patch now.

So, funky stuff in field_ui.inc , it's always nice to look at code from years ago :)

There are two additional submit handlers that DS adds: ds_field_ui_layouts_save() and ds_field_ui_fields_save(), each triggering a save of the display. Because ds_field_ui_layouts_save() is earlier then core, resetting the region through the display didn't work, because core comes after, saving the display for a third time. So instead, I'm manipulating the form_state now to reset the region, defaulting the core fields region to 'content' for now.

Also, we were still saving DS fields into the third party settings as well if no layout was selected, that has been handled now too.

So, this at least makes the fields come back, but after giving this some thought, I'm rather a fan killing that 'regions' key in our display settings. I can look next week for that, unless someone beats me to it. I'm also fine getting this one in first and then try for the refactor, we more or less have one month for 8.3.0.

(wow, we could really use a hook in field ui before $display->save which passes the display, form_state and form, that would make our lives 10x easier, but oh well)

+++ b/includes/field_ui.inc
@@ -353,6 +353,22 @@ function ds_field_ui_layouts_save($form, FormStateInterface $form_state) {
+      foreach ($display->getComponents() as $name => $component) {

could probably be array_keys or so as $component is useless now

Wrote a test, tweaked the layout selector to use #empty_value, and added the array_keys usage from #12.

Didn't realize there was already a test for unsetting the layout. Fixed it and added additional coverage.

committed this patch already .. leaving open for now so I remember creating a new one for moving the region key

Created #2860314: Remove regions key in DS third party setting