From 5781cdba2960b5d3e376f352f6aedea5f0ce052d Mon Sep 17 00:00:00 2001 From: webchick Date: Wed, 3 Oct 2018 14:17:58 -0700 Subject: [PATCH] Issue #2919795 by tim.plunkett, dead_arm, tedbow: Add visual hints that Layout Builder work is in tempstore and will not be lost, or take effect until saved --- .../Controller/LayoutBuilderController.php | 12 +++ .../src/LayoutTempstoreRepository.php | 9 ++ .../LayoutTempstoreRepositoryInterface.php | 11 +++ .../src/Functional/LayoutBuilderTest.php | 14 +-- .../LayoutBuilderUiTest.php | 93 +++++++++++++++++++ .../Unit/LayoutTempstoreRepositoryTest.php | 6 ++ core/tests/Drupal/Tests/WebAssert.php | 28 ++++++ 7 files changed, 161 insertions(+), 12 deletions(-) create mode 100644 core/modules/layout_builder/tests/src/FunctionalJavascript/LayoutBuilderUiTest.php diff --git a/core/modules/layout_builder/src/Controller/LayoutBuilderController.php b/core/modules/layout_builder/src/Controller/LayoutBuilderController.php index c01fea544b..3b56d4acb7 100644 --- a/core/modules/layout_builder/src/Controller/LayoutBuilderController.php +++ b/core/modules/layout_builder/src/Controller/LayoutBuilderController.php @@ -2,6 +2,7 @@ namespace Drupal\layout_builder\Controller; +use Drupal\Core\Ajax\AjaxHelperTrait; use Drupal\Core\DependencyInjection\ContainerInjectionInterface; use Drupal\Core\Messenger\MessengerInterface; use Drupal\Core\Plugin\PluginFormInterface; @@ -24,6 +25,7 @@ class LayoutBuilderController implements ContainerInjectionInterface { use LayoutBuilderContextTrait; use StringTranslationTrait; + use AjaxHelperTrait; /** * The layout tempstore repository. @@ -90,6 +92,11 @@ public function layout(SectionStorageInterface $section_storage, $is_rebuilding $this->prepareLayout($section_storage, $is_rebuilding); $output = []; + if ($this->isAjax()) { + $output['status_messages'] = [ + '#type' => 'status_messages', + ]; + } $count = 0; for ($i = 0; $i < $section_storage->count(); $i++) { $output[] = $this->buildAddSectionLink($section_storage, $count); @@ -114,6 +121,11 @@ public function layout(SectionStorageInterface $section_storage, $is_rebuilding * Indicates if the layout is rebuilding. */ protected function prepareLayout(SectionStorageInterface $section_storage, $is_rebuilding) { + // If the layout has pending changes, add a warning. + 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. if (!$is_rebuilding && $section_storage->count() === 0) { $sections = []; diff --git a/core/modules/layout_builder/src/LayoutTempstoreRepository.php b/core/modules/layout_builder/src/LayoutTempstoreRepository.php index 39725afc7e..69676861db 100644 --- a/core/modules/layout_builder/src/LayoutTempstoreRepository.php +++ b/core/modules/layout_builder/src/LayoutTempstoreRepository.php @@ -45,6 +45,15 @@ public function get(SectionStorageInterface $section_storage) { return $section_storage; } + /** + * {@inheritdoc} + */ + public function has(SectionStorageInterface $section_storage) { + $id = $section_storage->getStorageId(); + $tempstore = $this->getTempstore($section_storage)->get($id); + return !empty($tempstore['section_storage']); + } + /** * {@inheritdoc} */ diff --git a/core/modules/layout_builder/src/LayoutTempstoreRepositoryInterface.php b/core/modules/layout_builder/src/LayoutTempstoreRepositoryInterface.php index 4972a47f6f..67dc59ca99 100644 --- a/core/modules/layout_builder/src/LayoutTempstoreRepositoryInterface.php +++ b/core/modules/layout_builder/src/LayoutTempstoreRepositoryInterface.php @@ -35,6 +35,17 @@ public function get(SectionStorageInterface $section_storage); */ public function set(SectionStorageInterface $section_storage); + /** + * Checks for the existence of a tempstore version of a section storage. + * + * @param \Drupal\layout_builder\SectionStorageInterface $section_storage + * The section storage to check for in tempstore. + * + * @return bool + * TRUE if there is a tempstore version of this section storage. + */ + public function has(SectionStorageInterface $section_storage); + /** * Removes the tempstore version of a section storage. * diff --git a/core/modules/layout_builder/tests/src/Functional/LayoutBuilderTest.php b/core/modules/layout_builder/tests/src/Functional/LayoutBuilderTest.php index be2dd5d25e..1e4f240869 100644 --- a/core/modules/layout_builder/tests/src/Functional/LayoutBuilderTest.php +++ b/core/modules/layout_builder/tests/src/Functional/LayoutBuilderTest.php @@ -91,7 +91,7 @@ public function testLayoutBuilderUi() { // The body field is only present once. $assert_session->elementsCount('css', '.field--name-body', 1); // The extra field is only present once. - $this->assertTextAppearsOnce('Placeholder for the "Extra label" field'); + $assert_session->pageTextContainsOnce('Placeholder for the "Extra label" field'); // Save the defaults. $assert_session->linkExists('Save Layout'); $this->clickLink('Save Layout'); @@ -106,7 +106,7 @@ public function testLayoutBuilderUi() { // The body field is only present once. $assert_session->elementsCount('css', '.field--name-body', 1); // The extra field is only present once. - $this->assertTextAppearsOnce('Placeholder for the "Extra label" field'); + $assert_session->pageTextContainsOnce('Placeholder for the "Extra label" field'); // Add a new block. $assert_session->linkExists('Add Block'); @@ -526,14 +526,4 @@ public function testBlockPlaceholder() { $assert_session->pageTextContains($block_content); } - /** - * Asserts that a text string only appears once on the page. - * - * @param string $needle - * The string to look for. - */ - protected function assertTextAppearsOnce($needle) { - $this->assertEquals(1, substr_count($this->getSession()->getPage()->getContent(), $needle), "'$needle' only appears once on the page."); - } - } diff --git a/core/modules/layout_builder/tests/src/FunctionalJavascript/LayoutBuilderUiTest.php b/core/modules/layout_builder/tests/src/FunctionalJavascript/LayoutBuilderUiTest.php new file mode 100644 index 0000000000..f0c4e42ab1 --- /dev/null +++ b/core/modules/layout_builder/tests/src/FunctionalJavascript/LayoutBuilderUiTest.php @@ -0,0 +1,93 @@ +drupalPlaceBlock('local_tasks_block'); + + $this->createContentType(['type' => 'bundle_with_section_field']); + + $this->drupalLogin($this->drupalCreateUser([ + 'configure any layout', + 'administer node display', + 'administer node fields', + ])); + } + + /** + * Tests the message indicating unsaved changes. + */ + public function testUnsavedChangesMessage() { + $assert_session = $this->assertSession(); + $page = $this->getSession()->getPage(); + + // Enable layout builder. + $this->drupalPostForm( + static::FIELD_UI_PREFIX . '/display/default', + ['layout[enabled]' => TRUE], + 'Save' + ); + $page->clickLink('Manage layout'); + $assert_session->addressEquals(static::FIELD_UI_PREFIX . '/display-layout/default'); + + // Add a new section. + $page->clickLink('Add Section'); + $assert_session->assertWaitOnAjaxRequest(); + $assert_session->pageTextNotContains('You have unsaved changes.'); + $page->clickLink('One column'); + $assert_session->assertWaitOnAjaxRequest(); + $assert_session->pageTextContainsOnce('You have unsaved changes.'); + + // Reload the page. + $this->drupalGet(static::FIELD_UI_PREFIX . '/display-layout/default'); + $assert_session->pageTextContainsOnce('You have unsaved changes.'); + + // Cancel the changes. + $page->clickLink('Cancel Layout'); + $this->drupalGet(static::FIELD_UI_PREFIX . '/display-layout/default'); + $assert_session->pageTextNotContains('You have unsaved changes.'); + + // Add a new section. + $page->clickLink('Add Section'); + $assert_session->assertWaitOnAjaxRequest(); + $assert_session->pageTextNotContains('You have unsaved changes.'); + $page->clickLink('One column'); + $assert_session->assertWaitOnAjaxRequest(); + $assert_session->pageTextContainsOnce('You have unsaved changes.'); + + // Save the changes. + $page->clickLink('Save Layout'); + $this->drupalGet(static::FIELD_UI_PREFIX . '/display-layout/default'); + $assert_session->pageTextNotContains('You have unsaved changes.'); + } + +} diff --git a/core/modules/layout_builder/tests/src/Unit/LayoutTempstoreRepositoryTest.php b/core/modules/layout_builder/tests/src/Unit/LayoutTempstoreRepositoryTest.php index 0c74d01763..fb46e608ae 100644 --- a/core/modules/layout_builder/tests/src/Unit/LayoutTempstoreRepositoryTest.php +++ b/core/modules/layout_builder/tests/src/Unit/LayoutTempstoreRepositoryTest.php @@ -16,6 +16,7 @@ class LayoutTempstoreRepositoryTest extends UnitTestCase { /** * @covers ::get + * @covers ::has */ public function testGetEmptyTempstore() { $section_storage = $this->prophesize(SectionStorageInterface::class); @@ -30,12 +31,15 @@ public function testGetEmptyTempstore() { $repository = new LayoutTempstoreRepository($tempstore_factory->reveal()); + $this->assertFalse($repository->has($section_storage->reveal())); + $result = $repository->get($section_storage->reveal()); $this->assertSame($section_storage->reveal(), $result); } /** * @covers ::get + * @covers ::has */ public function testGetLoadedTempstore() { $section_storage = $this->prophesize(SectionStorageInterface::class); @@ -50,6 +54,8 @@ public function testGetLoadedTempstore() { $repository = new LayoutTempstoreRepository($tempstore_factory->reveal()); + $this->assertTrue($repository->has($section_storage->reveal())); + $result = $repository->get($section_storage->reveal()); $this->assertSame($tempstore_section_storage->reveal(), $result); $this->assertNotSame($section_storage->reveal(), $result); diff --git a/core/tests/Drupal/Tests/WebAssert.php b/core/tests/Drupal/Tests/WebAssert.php index 93667b1687..4f7a7d292c 100644 --- a/core/tests/Drupal/Tests/WebAssert.php +++ b/core/tests/Drupal/Tests/WebAssert.php @@ -3,6 +3,7 @@ namespace Drupal\Tests; use Behat\Mink\Exception\ExpectationException; +use Behat\Mink\Exception\ResponseTextException; use Behat\Mink\WebAssert as MinkWebAssert; use Behat\Mink\Element\TraversableElement; use Behat\Mink\Exception\ElementNotFoundException; @@ -569,4 +570,31 @@ public function hiddenFieldValueNotEquals($field, $value, TraversableElement $co $this->assert(!preg_match($regex, $actual), $message); } + /** + * Checks that current page contains text only once. + * + * @param string $text + * The string to look for. + * + * @see \Behat\Mink\WebAssert::pageTextContains() + */ + public function pageTextContainsOnce($text) { + $actual = $this->session->getPage()->getText(); + $actual = preg_replace('/\s+/u', ' ', $actual); + $regex = '/' . preg_quote($text, '/') . '/ui'; + $count = preg_match_all($regex, $actual); + if ($count === 1) { + return; + } + + if ($count > 1) { + $message = sprintf('The text "%s" appears in the text of this page more than once, but it should not.', $text); + } + else { + $message = sprintf('The text "%s" was not found anywhere in the text of the current page.', $text); + } + + throw new ResponseTextException($message, $this->session->getDriver()); + } + } -- GitLab