Issue #3030647 by tim.plunkett, phenaproxima, xjm, DyanneNova, bnjmnm, tedbow, dead_arm: Do not add a section when editing an empty layout, or differentiate between new layouts and existing empty layouts

core/modules/layout_builder/layout_builder.module

@@ -293,6 +293,14 @@ function layout_builder_plugin_filter_layout__layout_builder_alter(array &$defin
   }
 }
 
+/**
+ * Implements hook_plugin_filter_TYPE_alter().
+ */
+function layout_builder_plugin_filter_layout_alter(array &$definitions, array $extra, $consumer) {
+  // Hide the blank layout plugin from listings.
+  unset($definitions['layout_builder_blank']);
+}
+
 /**
  * Implements hook_system_breadcrumb_alter().
  */

core/modules/layout_builder/layout_builder.post_update.php

@@ -95,6 +95,13 @@ function layout_builder_post_update_routing_entity_form() {
   // Empty post-update hook.
 }
 
+/**
+ * Clear caches to discover new blank layout plugin.
+ */
+function layout_builder_post_update_discover_blank_layout_plugin() {
+  // Empty post-update hook.
+}
+
 /**
  * Clear caches due to routing changes to changing the URLs for defaults.
  */

core/modules/layout_builder/src/Element/LayoutBuilder.php

@@ -12,7 +12,6 @@ use Drupal\Core\Url;
 use Drupal\layout_builder\Context\LayoutBuilderContextTrait;
 use Drupal\layout_builder\LayoutTempstoreRepositoryInterface;
 use Drupal\layout_builder\OverridesSectionStorageInterface;
-use Drupal\layout_builder\Section;
 use Drupal\layout_builder\SectionStorageInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
@@ -140,20 +139,10 @@ class LayoutBuilder extends RenderElement implements ContainerFactoryPluginInter
     if ($this->layoutTempstoreRepository->has($section_storage)) {
       $this->messenger->addWarning($this->t('You have unsaved changes.'));
     }
-    // Only add sections if the layout is new and empty.
-    elseif ($section_storage->count() === 0) {
-      $sections = [];
-      // If this is an empty override, copy the sections from the corresponding
-      // default.
-      if ($section_storage instanceof OverridesSectionStorageInterface) {
-        $sections = $section_storage->getDefaultSectionStorage()->getSections();
-      }
-
-      // For an empty layout, begin with a single section of one column.
-      if (!$sections) {
-        $sections[] = new Section('layout_onecol');
-      }
-
+    // If the layout is an override that has not yet been overridden, copy the
+    // sections from the corresponding default.
+    elseif ($section_storage instanceof OverridesSectionStorageInterface && !$section_storage->isOverridden()) {
+      $sections = $section_storage->getDefaultSectionStorage()->getSections();
       foreach ($sections as $section) {
         $section_storage->appendSection($section);
       }

core/modules/layout_builder/src/Entity/LayoutBuilderEntityViewDisplay.php

@@ -104,7 +104,14 @@ class LayoutBuilderEntityViewDisplay extends BaseEntityViewDisplay implements La
    * {@inheritdoc}
    */
   protected function setSections(array $sections) {
-    $this->setThirdPartySetting('layout_builder', 'sections', array_values($sections));
+    // Third-party settings must be completely unset instead of stored as an
+    // empty array.
+    if (!$sections) {
+      $this->unsetThirdPartySetting('layout_builder', 'sections');
+    }
+    else {
+      $this->setThirdPartySetting('layout_builder', 'sections', array_values($sections));
+    }
     return $this;
   }
 
@@ -143,9 +150,7 @@ class LayoutBuilderEntityViewDisplay extends BaseEntityViewDisplay implements La
       }
       else {
         // When being disabled, remove all existing section data.
-        while (count($this) > 0) {
-          $this->removeSection(0);
-        }
+        $this->removeAllSections();
       }
     }
   }

core/modules/layout_builder/src/Form/RevertOverridesForm.php

@@ -104,10 +104,9 @@ class RevertOverridesForm extends ConfirmFormBase {
    */
   public function submitForm(array &$form, FormStateInterface $form_state) {
     // Remove all sections.
-    while ($this->sectionStorage->count()) {
-      $this->sectionStorage->removeSection(0);
-    }
-    $this->sectionStorage->save();
+    $this->sectionStorage
+      ->removeAllSections()
+      ->save();
     $this->layoutTempstoreRepository->delete($this->sectionStorage);
 
     $this->messenger->addMessage($this->t('The layout has been reverted back to defaults.'));

core/modules/layout_builder/src/OverridesSectionStorageInterface.php

@@ -23,4 +23,12 @@ interface OverridesSectionStorageInterface extends SectionStorageInterface {
    */
   public function getDefaultSectionStorage();
 
+  /**
+   * Indicates if overrides are in use.
+   *
+   * @return bool
+   *   TRUE if this overrides section storage is in use, otherwise FALSE.
+   */
+  public function isOverridden();
+
 }

core/modules/layout_builder/src/Plugin/Layout/BlankLayout.php

@@ -0,0 +1,31 @@
+<?php
+
+namespace Drupal\layout_builder\Plugin\Layout;
+
+use Drupal\Core\Layout\LayoutDefault;
+
+/**
+ * Provides a layout plugin that produces no output.
+ *
+ * @see \Drupal\layout_builder\Field\LayoutSectionItemList::removeSection()
+ * @see \Drupal\layout_builder\SectionStorage\SectionStorageTrait::addBlankSection()
+ * @see \Drupal\layout_builder\SectionStorage\SectionStorageTrait::hasBlankSection()
+ *
+ * @internal
+ *   This layout plugin is intended for internal use by Layout Builder only.
+ *
+ * @Layout(
+ *   id = "layout_builder_blank",
+ * )
+ */
+class BlankLayout extends LayoutDefault {
+
+  /**
+   * {@inheritdoc}
+   */
+  public function build(array $regions) {
+    // Return no output.
+    return [];
+  }
+
+}

core/modules/layout_builder/src/Plugin/SectionStorage/OverridesSectionStorage.php

@@ -360,7 +360,17 @@ class OverridesSectionStorage extends SectionStorageBase implements ContainerFac
     $default_section_storage = $this->getDefaultSectionStorage();
     $cacheability->addCacheableDependency($default_section_storage)->addCacheableDependency($this);
     // Check that overrides are enabled and have at least one section.
-    return $default_section_storage->isOverridable() && count($this);
+    return $default_section_storage->isOverridable() && $this->isOverridden();
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function isOverridden() {
+    // If there are any sections at all, including a blank one, this section
+    // storage has been overridden. Do not use count() as it does not include
+    // blank sections.
+    return !empty($this->getSections());
   }
 
 }

core/modules/layout_builder/src/Plugin/SectionStorage/SectionStorageBase.php

@@ -102,6 +102,14 @@ abstract class SectionStorageBase extends ContextAwarePluginBase implements Sect
     return $this;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function removeAllSections($set_blank = FALSE) {
+    $this->getSectionList()->removeAllSections($set_blank);
+    return $this;
+  }
+
   /**
    * {@inheritdoc}
    */

core/modules/layout_builder/src/SectionListInterface.php

@@ -71,4 +71,19 @@ interface SectionListInterface extends \Countable {
    */
   public function removeSection($delta);
 
+  /**
+   * Removes all of the sections.
+   *
+   * @param bool $set_blank
+   *   (optional) The default implementation of section lists differentiates
+   *   between a list that has never contained any sections and a list that has
+   *   purposefully had all sections removed in order to remain blank. Passing
+   *   TRUE will mirror ::removeSection() by tracking this as a blank list.
+   *   Passing FALSE will reset the list as though it had never contained any
+   *   sections at all. Defaults to FALSE.
+   *
+   * @return $this
+   */
+  public function removeAllSections($set_blank = FALSE);
+
 }

core/modules/layout_builder/src/SectionStorage/SectionStorageTrait.php

@@ -31,6 +31,10 @@ trait SectionStorageTrait {
    * {@inheritdoc}
    */
   public function count() {
+    if ($this->hasBlankSection()) {
+      return 0;
+    }
+
     return count($this->getSections());
   }
 
@@ -76,6 +80,11 @@ trait SectionStorageTrait {
    * {@inheritdoc}
    */
   public function insertSection($delta, Section $section) {
+    // Clear the section list if there is currently a blank section.
+    if ($this->hasBlankSection()) {
+      $this->removeAllSections();
+    }
+
     if ($this->hasSection($delta)) {
       // @todo Use https://www.drupal.org/node/66183 once resolved.
       $start = array_slice($this->getSections(), 0, $delta);
@@ -88,13 +97,67 @@ trait SectionStorageTrait {
     return $this;
   }
 
+  /**
+   * Adds a blank section to the list.
+   *
+   * @return $this
+   *
+   * @see \Drupal\layout_builder\Plugin\Layout\BlankLayout
+   */
+  protected function addBlankSection() {
+    if ($this->hasSection(0)) {
+      throw new \Exception('A blank section must only be added to an empty list');
+    }
+
+    $this->appendSection(new Section('layout_builder_blank'));
+    return $this;
+  }
+
+  /**
+   * Indicates if this section list contains a blank section.
+   *
+   * A blank section is used to differentiate the difference between a layout
+   * that has never been instantiated and one that has purposefully had all
+   * sections removed.
+   *
+   * @return bool
+   *   TRUE if the section list contains a blank section, FALSE otherwise.
+   *
+   * @see \Drupal\layout_builder\Plugin\Layout\BlankLayout
+   */
+  protected function hasBlankSection() {
+    // A blank section will only ever exist when the delta is 0, as added by
+    // ::removeSection().
+    return $this->hasSection(0) && $this->getSection(0)->getLayoutId() === 'layout_builder_blank';
+  }
+
   /**
    * {@inheritdoc}
    */
   public function removeSection($delta) {
+    // Clear the section list if there is currently a blank section.
+    if ($this->hasBlankSection()) {
+      $this->removeAllSections();
+    }
+
     $sections = $this->getSections();
     unset($sections[$delta]);
     $this->setSections($sections);
+    // Add a blank section when the last section is removed.
+    if (empty($sections)) {
+      $this->addBlankSection();
+    }
+    return $this;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function removeAllSections($set_blank = FALSE) {
+    $this->setSections([]);
+    if ($set_blank) {
+      $this->addBlankSection();
+    }
     return $this;
   }
 

core/modules/layout_builder/tests/src/Functional/LayoutBuilderTest.php

@@ -869,10 +869,99 @@ class LayoutBuilderTest extends BrowserTestBase {
     $assert_session->elementsCount('css', '.layout', 1);
     $assert_session->elementsCount('css', '.layout--twocol', 1);
 
-    // The default layout is selected for a new object.
+    // No layout is selected for a new object.
     $this->drupalGet('layout-builder-test-simple-config/new');
+    $assert_session->elementNotExists('css', '.layout');
+  }
+
+  /**
+   * Tests removing all sections from overrides and defaults.
+   */
+  public function testRemovingAllSections() {
+    $assert_session = $this->assertSession();
+    $page = $this->getSession()->getPage();
+
+    $this->drupalLogin($this->drupalCreateUser([
+      'configure any layout',
+      'administer node display',
+    ]));
+
+    $field_ui_prefix = 'admin/structure/types/manage/bundle_with_section_field';
+    // Enable overrides.
+    $this->drupalPostForm("$field_ui_prefix/display/default", ['layout[enabled]' => TRUE], 'Save');
+    $this->drupalPostForm("$field_ui_prefix/display/default", ['layout[allow_custom]' => TRUE], 'Save');
+
+    // By default, there is one section.
+    $this->drupalGet('node/1');
     $assert_session->elementsCount('css', '.layout', 1);
-    $assert_session->elementsCount('css', '.layout--onecol', 1);
+    $assert_session->pageTextContains('The first node body');
+
+    $page->clickLink('Layout');
+    $assert_session->elementsCount('css', '.layout', 1);
+    $assert_session->elementsCount('css', '.layout-builder__add-block', 1);
+    $assert_session->elementsCount('css', '.layout-builder__add-section', 2);
+
+    // Remove the only section from the override.
+    $page->clickLink('Remove section');
+    $page->pressButton('Remove');
+    $assert_session->elementsCount('css', '.layout', 0);
+    $assert_session->elementsCount('css', '.layout-builder__add-block', 0);
+    $assert_session->elementsCount('css', '.layout-builder__add-section', 1);
+
+    // The override is still used instead of the default, despite being empty.
+    $page->pressButton('Save layout');
+    $assert_session->elementsCount('css', '.layout', 0);
+    $assert_session->pageTextNotContains('The first node body');
+
+    $page->clickLink('Layout');
+    $assert_session->elementsCount('css', '.layout', 0);
+    $assert_session->elementsCount('css', '.layout-builder__add-block', 0);
+    $assert_session->elementsCount('css', '.layout-builder__add-section', 1);
+
+    // Add one section to the override.
+    $page->clickLink('Add Section');
+    $page->clickLink('One column');
+    $assert_session->elementsCount('css', '.layout', 1);
+    $assert_session->elementsCount('css', '.layout-builder__add-block', 1);
+    $assert_session->elementsCount('css', '.layout-builder__add-section', 2);
+
+    $page->pressButton('Save layout');
+    $assert_session->elementsCount('css', '.layout', 1);
+    $assert_session->pageTextNotContains('The first node body');
+
+    // By default, the default has one section.
+    $this->drupalGet("$field_ui_prefix/display/default/layout");
+    $assert_session->elementsCount('css', '.layout', 1);
+    $assert_session->elementsCount('css', '.layout-builder__add-block', 1);
+    $assert_session->elementsCount('css', '.layout-builder__add-section', 2);
+
+    // Remove the only section from the default.
+    $page->clickLink('Remove section');
+    $page->pressButton('Remove');
+    $assert_session->elementsCount('css', '.layout', 0);
+    $assert_session->elementsCount('css', '.layout-builder__add-block', 0);
+    $assert_session->elementsCount('css', '.layout-builder__add-section', 1);
+
+    $page->pressButton('Save layout');
+    $page->clickLink('Manage layout');
+    $assert_session->elementsCount('css', '.layout', 0);
+    $assert_session->elementsCount('css', '.layout-builder__add-block', 0);
+    $assert_session->elementsCount('css', '.layout-builder__add-section', 1);
+
+    // The override is still in use.
+    $this->drupalGet('node/1');
+    $assert_session->elementsCount('css', '.layout', 1);
+    $assert_session->pageTextNotContains('The first node body');
+    $page->clickLink('Layout');
+    $assert_session->elementsCount('css', '.layout', 1);
+    $assert_session->elementsCount('css', '.layout-builder__add-block', 1);
+    $assert_session->elementsCount('css', '.layout-builder__add-section', 2);
+
+    // Revert the override.
+    $page->clickLink('Revert to defaults');
+    $page->pressButton('Revert');
+    $assert_session->elementsCount('css', '.layout', 0);
+    $assert_session->pageTextNotContains('The first node body');
   }
 
   /**

core/modules/layout_builder/tests/src/FunctionalJavascript/LayoutBuilderTest.php

@@ -241,10 +241,10 @@ class LayoutBuilderTest extends WebDriverTestBase {
 
     $page->pressButton('Save layout');
 
-    // Removing all sections results in the default layout display being used.
+    // Removing all sections results in no layout being used.
     $assert_session->addressEquals($node_url);
-    $assert_session->elementExists('css', '.layout.layout--onecol');
-    $assert_session->pageTextContains('The node body');
+    $assert_session->elementNotExists('css', '.layout');
+    $assert_session->pageTextNotContains('The node body');
   }
 
   /**

core/modules/layout_builder/tests/src/Kernel/LayoutBuilderFieldLayoutCompatibilityTest.php

@@ -73,7 +73,7 @@ class LayoutBuilderFieldLayoutCompatibilityTest extends LayoutBuilderCompatibili
     $this->assertNotEmpty($this->cssSelect('.layout--onecol'));
 
     // Removing the layout restores the original rendering of the entity.
-    $field_list->removeSection(0);
+    $field_list->removeAllSections();
     $this->entity->save();
     $this->assertFieldAttributes($this->entity, $expected_fields);
   }

core/modules/layout_builder/tests/src/Kernel/LayoutBuilderInstallTest.php

@@ -51,7 +51,7 @@ class LayoutBuilderInstallTest extends LayoutBuilderCompatibilityTestBase {
     $this->assertNotEmpty($this->cssSelect('.layout--onecol'));
 
     // Removing the layout restores the original rendering of the entity.
-    $this->entity->get(OverridesSectionStorage::FIELD_NAME)->removeSection(0);
+    $this->entity->get(OverridesSectionStorage::FIELD_NAME)->removeAllSections();
     $this->entity->save();
     $this->assertFieldAttributes($this->entity, $expected_fields);
 

core/modules/layout_builder/tests/src/Kernel/OverridesSectionStorageTest.php

@@ -211,4 +211,36 @@ class OverridesSectionStorageTest extends KernelTestBase {
     $this->assertSame('default', $result['view_mode']->getContextValue());
   }
 
+  /**
+   * @covers ::isOverridden
+   */
+  public function testIsOverridden() {
+    $display = LayoutBuilderEntityViewDisplay::create([
+      'targetEntityType' => 'entity_test',
+      'bundle' => 'entity_test',
+      'mode' => 'default',
+      'status' => TRUE,
+    ]);
+    $display
+      ->enableLayoutBuilder()
+      ->setOverridable()
+      ->save();
+
+    $entity = EntityTest::create();
+    $entity->set(OverridesSectionStorage::FIELD_NAME, [new Section('layout_default')]);
+    $entity->save();
+    $entity = EntityTest::load($entity->id());
+
+    $context = EntityContext::fromEntity($entity);
+    $this->plugin->setContext('entity', $context);
+
+    $this->assertTrue($this->plugin->isOverridden());
+    $this->plugin->removeSection(0);
+    $this->assertTrue($this->plugin->isOverridden());
+    $this->plugin->removeAllSections(TRUE);
+    $this->assertTrue($this->plugin->isOverridden());
+    $this->plugin->removeAllSections();
+    $this->assertFalse($this->plugin->isOverridden());
+  }
+
 }

core/modules/layout_builder/tests/src/Kernel/SectionListTraitTest.php

@@ -0,0 +1,67 @@
+<?php
+
+namespace Drupal\Tests\layout_builder\Kernel;
+
+use Drupal\layout_builder\SectionListInterface;
+use Drupal\layout_builder\SectionStorage\SectionStorageTrait;
+
+/**
+ * @coversDefaultClass \Drupal\layout_builder\SectionStorage\SectionStorageTrait
+ *
+ * @group layout_builder
+ */
+class SectionListTraitTest extends SectionStorageTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function getSectionStorage(array $section_data) {
+    return new TestSectionList($section_data);
+  }
+
+  /**
+   * @covers ::addBlankSection
+   */
+  public function testAddBlankSection() {
+    $this->setExpectedException(\Exception::class, 'A blank section must only be added to an empty list');
+    $this->sectionStorage->addBlankSection();
+  }
+
+}
+
+class TestSectionList implements SectionListInterface {
+
+  use SectionStorageTrait {
+    addBlankSection as public;
+  }
+
+  /**
+   * An array of sections.
+   *
+   * @var \Drupal\layout_builder\Section[]
+   */
+  protected $sections;
+
+  /**
+   * TestSectionList constructor.
+   */
+  public function __construct(array $sections) {
+    $this->setSections($sections);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setSections(array $sections) {
+    $this->sections = array_values($sections);
+    return $sections;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getSections() {
+    return $this->sections;
+  }
+
+}

core/modules/layout_builder/tests/src/Kernel/SectionStorageTestBase.php

@@ -58,7 +58,7 @@ abstract class SectionStorageTestBase extends EntityKernelTestBase {
   abstract protected function getSectionStorage(array $section_data);
 
   /**
-   * @covers ::getSections
+   * Tests ::getSections().
    */
   public function testGetSections() {
     $expected = [
@@ -123,6 +123,32 @@ abstract class SectionStorageTestBase extends EntityKernelTestBase {
     $this->assertSections($expected);
   }
 
+  /**
+   * @covers ::removeAllSections
+   *
+   * @dataProvider providerTestRemoveAllSections
+   */
+  public function testRemoveAllSections($set_blank, $expected) {
+    if ($set_blank === NULL) {
+      $this->sectionStorage->removeAllSections();
+    }
+    else {
+      $this->sectionStorage->removeAllSections($set_blank);
+    }
+    $this->assertSections($expected);
+  }
+
+  /**
+   * Provides test data for ::testRemoveAllSections().
+   */
+  public function providerTestRemoveAllSections() {
+    $data = [];
+    $data[] = [NULL, []];
+    $data[] = [FALSE, []];
+    $data[] = [TRUE, [new Section('layout_builder_blank')]];
+    return $data;
+  }
+
   /**
    * @covers ::removeSection
    */
@@ -137,6 +163,19 @@ abstract class SectionStorageTestBase extends EntityKernelTestBase {
     $this->assertSections($expected);
   }
 
+  /**
+   * @covers ::removeSection
+   */
+  public function testRemoveMultipleSections() {
+    $expected = [
+      new Section('layout_builder_blank'),
+    ];
+
+    $this->sectionStorage->removeSection(0);
+    $this->sectionStorage->removeSection(0);
+    $this->assertSections($expected);
+  }
+
   /**
    * Tests __clone().
    */