Issue #2785047 by tedbow, effulgentsia, Jo Fitzgerald, webchick, Wim Leers,...

Issue #2785047 by tedbow, effulgentsia, Jo Fitzgerald, webchick, Wim Leers, tkoleary, tim.plunkett, timmillwood, drpal, rootwork: In Outside In mode, form validation messages should appear in the off-canvas tray, not the main page

core/modules/outside_in/src/Block/BlockEntityOffCanvasForm.php

@@ -4,9 +4,14 @@
 
 use Drupal\block\BlockForm;
 use Drupal\block\BlockInterface;
+use Drupal\Component\Utility\Html;
+use Drupal\Core\Ajax\AjaxResponse;
+use Drupal\Core\Ajax\RedirectCommand;
+use Drupal\Core\Ajax\ReplaceCommand;
 use Drupal\Core\Block\BlockPluginInterface;
 use Drupal\Core\Form\FormStateInterface;
 use Drupal\Core\Plugin\PluginWithFormsInterface;
+use Drupal\Core\Url;
 
 /**
  * Provides form for block instance forms when used in the off-canvas dialog.
@@ -111,4 +116,79 @@ protected function getPluginForm(BlockPluginInterface $block) {
     return $block;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function buildForm(array $form, FormStateInterface $form_state) {
+    $form = parent::buildForm($form, $form_state);
+    $form['actions']['submit']['#ajax'] = [
+      'callback' => '::submitFormDialog',
+    ];
+    $form['#attached']['library'][] = 'core/drupal.dialog.ajax';
+
+    // static::submitFormDialog() requires data-drupal-selector to be the same
+    // between the various Ajax requests. A bug in
+    // \Drupal\Core\Form\FormBuilder prevents that from happening unless
+    // $form['#id'] is also the same. Normally, #id is set to a unique HTML ID
+    // via Html::getUniqueId(), but here we bypass that in order to work around
+    // the data-drupal-selector bug. This is okay so long as we assume that this
+    // form only ever occurs once on a page.
+    // @todo Remove this workaround once https://www.drupal.org/node/2897377 is
+    //   fixed.
+    $form['#id'] = Html::getId($form_state->getBuildInfo()['form_id']);
+
+    return $form;
+  }
+
+  /**
+   * Submit form dialog #ajax callback.
+   *
+   * @param array $form
+   *   An associative array containing the structure of the form.
+   * @param \Drupal\Core\Form\FormStateInterface $form_state
+   *   The current state of the form.
+   *
+   * @return \Drupal\Core\Ajax\AjaxResponse
+   *   An AJAX response that display validation error messages or redirects
+   *   to a URL
+   *
+   * @todo Repalce this callback with generic trait in
+   *   https://www.drupal.org/node/2896535.
+   */
+  public function submitFormDialog(array &$form, FormStateInterface $form_state) {
+    $response = new AjaxResponse();
+    if ($form_state->hasAnyErrors()) {
+      $form['status_messages'] = [
+        '#type' => 'status_messages',
+        '#weight' => -1000,
+      ];
+      $command = new ReplaceCommand('[data-drupal-selector="' . $form['#attributes']['data-drupal-selector'] . '"]', $form);
+    }
+    else {
+      if ($redirect_url = $this->getRedirectUrl()) {
+        $command = new RedirectCommand($redirect_url->setAbsolute()->toString());
+      }
+      else {
+        // Settings Tray always provides a destination.
+        throw new \Exception("No destination provided by Settings Tray form");
+      }
+    }
+    return $response->addCommand($command);
+  }
+
+  /**
+   * Gets the form's redirect URL from 'destination' provide in the request.
+   *
+   * @return \Drupal\Core\Url|null
+   *   The redirect URL or NULL if dialog should just be closed.
+   */
+  protected function getRedirectUrl() {
+    // \Drupal\Core\Routing\RedirectDestination::get() cannot be used directly
+    // because it will use <current> if 'destination' is not in the query
+    // string.
+    if ($this->getRequest()->query->has('destination') && $destination = $this->getRedirectDestination()->get()) {
+      return Url::fromUserInput('/' . $destination);
+    }
+  }
+
 }

core/modules/outside_in/tests/modules/outside_in_test/outside_in_test.info.yml

+name: 'Settings Tray Test'
+type: module
+description: 'Provides Settings Tray test functionality.'
+package: Testing
+version: VERSION
+core: 8.x
+dependencies:
+  - block
+  - outside_in

core/modules/outside_in/tests/modules/outside_in_test/src/Plugin/Block/ValidationErrorBlock.php

+<?php
+
+namespace Drupal\outside_in_test\Plugin\Block;
+
+use Drupal\Core\Block\BlockBase;
+use Drupal\Core\Form\FormStateInterface;
+
+/**
+ * Provides a 'Block with validation error' test block.
+ *
+ * @Block(
+ *   id = "outside_in_test_validation",
+ *   admin_label = @Translation("Block with validation error")
+ * )
+ */
+class ValidationErrorBlock extends BlockBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  public function build() {
+    return ['#markup' => '<span>If I had more time this would be very witty :(.</span>'];
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function validateConfigurationForm(array &$form, FormStateInterface $form_state) {
+    parent::validateConfigurationForm($form, $form_state);
+    $form_state->setError($form['label'], 'Sorry system error. Please save again.');
+  }
+
+}

core/modules/outside_in/tests/src/FunctionalJavascript/OutsideInBlockFormTest.php

@@ -32,6 +32,7 @@ class OutsideInBlockFormTest extends OutsideInJavascriptTestBase {
     'quickedit',
     'search',
     'block_content',
+    'outside_in_test',
     // Add test module to override CSS pointer-events properties because they
     // cause test failures.
     'outside_in_test_css',
@@ -114,10 +115,9 @@ public function testBlocks($block_plugin, $new_page_text, $element_selector, $la
       if (isset($new_page_text)) {
         $page->pressButton($button_text);
         // Make sure the changes are present.
-        // @todo Use a wait method that will take into account the form submitting
-        //   and all JavaScript activity. https://www.drupal.org/node/2837676
-        //   The use \Behat\Mink\WebAssert::pageTextContains to check text.
-        $this->assertJsCondition('jQuery("' . $block_selector . ' ' . $label_selector . '").html() == "' . $new_page_text . '"');
+        $new_page_text_locator = "$block_selector $label_selector:contains($new_page_text)";
+        $this->assertElementVisibleAfterWait('css', $new_page_text_locator);
+        $web_assert->assertWaitOnAjaxRequest();
       }
 
       $this->openBlockForm($block_selector);
@@ -131,7 +131,7 @@ public function testBlocks($block_plugin, $new_page_text, $element_selector, $la
       // Open block form by clicking a element inside the block.
       // This confirms that default action for links and form elements is
       // suppressed.
-      $this->openBlockForm("$block_selector {$element_selector}");
+      $this->openBlockForm("$block_selector {$element_selector}", $block_selector);
       $web_assert->elementTextContains('css', '.contextual-toolbar-tab button', 'Editing');
       $web_assert->elementAttributeContains('css', '.dialog-off-canvas__main-canvas', 'class', 'js-outside-in-edit-mode');
       // Simulate press the Escape key.
@@ -154,7 +154,7 @@ public function providerTestBlocks() {
     $blocks = [
       'block-powered' => [
         'block_plugin' => 'system_powered_by_block',
-        'new_page_text' => 'Can you imagine anyone showing the label on this block?',
+        'new_page_text' => 'Can you imagine anyone showing the label on this block',
         'element_selector' => 'span a',
         'label_selector' => 'h2',
         'button_text' => 'Save Powered by Drupal',
@@ -162,7 +162,7 @@ public function providerTestBlocks() {
       ],
       'block-branding' => [
         'block_plugin' => 'system_branding_block',
-        'new_page_text' => 'The site that will live a very short life.',
+        'new_page_text' => 'The site that will live a very short life',
         'element_selector' => "a[rel='home']:last-child",
         'label_selector' => "a[rel='home']:last-child",
         'button_text' => 'Save Site branding',
@@ -192,6 +192,7 @@ protected function enableEditMode() {
    * Disables edit mode by pressing edit button in the toolbar.
    */
   protected function disableEditMode() {
+    $this->assertSession()->assertWaitOnAjaxRequest();
     $this->pressToolbarEditButton();
     $this->assertEditModeDisabled();
   }
@@ -225,8 +226,20 @@ protected function assertOffCanvasBlockFormIsValid() {
    *
    * @param string $block_selector
    *   A css selector selects the block or an element within it.
+   * @param string $contextual_link_container
+   *   The element that contains the contextual links. If none provide the
+   *   $block_selector will be used.
    */
-  protected function openBlockForm($block_selector) {
+  protected function openBlockForm($block_selector, $contextual_link_container = '') {
+    if (!$contextual_link_container) {
+      $contextual_link_container = $block_selector;
+    }
+    // Ensure that contextual link element is present because this is required
+    // to open the off-canvas dialog in edit mode.
+    $contextual_link = $this->assertSession()->waitForElement('css', "$contextual_link_container .contextual-links a");
+    $this->assertNotEmpty($contextual_link);
+    // Ensure that all other Ajax activity is completed.
+    $this->assertSession()->assertWaitOnAjaxRequest();
     $this->click($block_selector);
     $this->waitForOffCanvasToOpen();
     $this->assertOffCanvasBlockFormIsValid();
@@ -490,4 +503,26 @@ protected function isLabelInputVisible() {
     return $this->getSession()->getPage()->find('css', static::LABEL_INPUT_SELECTOR)->isVisible();
   }
 
+  /**
+   * Test that validation errors appear in the off-canvas dialog.
+   */
+  public function testValidationMessages() {
+    $page = $this->getSession()->getPage();
+    $web_assert = $this->assertSession();
+    foreach ($this->getTestThemes() as $theme) {
+      $this->enableTheme($theme);
+      $block = $this->placeBlock('outside_in_test_validation');
+      $this->drupalGet('user');
+      $this->enableEditMode();
+      $this->openBlockForm($this->getBlockSelector($block));
+      $page->pressButton('Save Block with validation error');
+      $web_assert->assertWaitOnAjaxRequest();
+      // The outside_in_test_validation test plugin form always has a validation
+      // error.
+      $web_assert->elementContains('css', '#drupal-off-canvas', 'Sorry system error. Please save again');
+      $this->disableEditMode();
+      $block->delete();
+    }
+  }
+
 }