From c8806a02991f04233f859d2ef8870363172a2186 Mon Sep 17 00:00:00 2001
From: bnjmnm <benm@umich.edu>
Date: Wed, 7 Jun 2023 15:08:50 -0400
Subject: [PATCH] Issue #3364713 by lauriii, bnjmnm, andy-blum, Dave Reid,
cosmicdreams: Claro: Messages can be malformed when JS creates messages and
PHP messages already exist
---
.../js_message_test.routing.yml | 8 ++
.../Controller/JSMessageTestController.php | 16 +++
.../modules/olivero_test/olivero_test.module | 19 ----
.../nightwatch_testing.info.yml | 1 +
.../Tests/Claro/claroDrupalMessageTest.js | 101 ++++++++++++++++++
.../Tests/Olivero/oliveroDrupalMessageTest.js | 4 +-
core/themes/claro/js/messages.js | 5 +-
.../templates/misc/status-messages.html.twig | 84 ++++++++-------
8 files changed, 175 insertions(+), 63 deletions(-)
create mode 100644 core/tests/Drupal/Nightwatch/Tests/Claro/claroDrupalMessageTest.js
diff --git a/core/modules/system/tests/modules/js_message_test/js_message_test.routing.yml b/core/modules/system/tests/modules/js_message_test/js_message_test.routing.yml
index 4c325e84611e..a3e5f44d009e 100644
--- a/core/modules/system/tests/modules/js_message_test/js_message_test.routing.yml
+++ b/core/modules/system/tests/modules/js_message_test/js_message_test.routing.yml
@@ -5,3 +5,11 @@ js_message_test.links:
_title: 'JsMessageLinks'
requirements:
_access: 'TRUE'
+
+js_message_test.links_with_system_messages:
+ path: '/js_message_test_link_with_system_messages'
+ defaults:
+ _controller: '\Drupal\js_message_test\Controller\JSMessageTestController::messageLinksWithSystemMessages'
+ _title: 'JsMessageLinks'
+ requirements:
+ _access: 'TRUE'
diff --git a/core/modules/system/tests/modules/js_message_test/src/Controller/JSMessageTestController.php b/core/modules/system/tests/modules/js_message_test/src/Controller/JSMessageTestController.php
index c6cd8b4670f6..b242656daeba 100644
--- a/core/modules/system/tests/modules/js_message_test/src/Controller/JSMessageTestController.php
+++ b/core/modules/system/tests/modules/js_message_test/src/Controller/JSMessageTestController.php
@@ -29,6 +29,22 @@ public static function getMessagesSelectors() {
return ['', '[data-drupal-messages-other]'];
}
+ /**
+ * Displays links to show messages via JavaScript with messages from backend.
+ *
+ * @return array
+ * Render array for links.
+ */
+ public function messageLinksWithSystemMessages() {
+ // Add PHP rendered messages to the page.
+ $messenger = \Drupal::messenger();
+ $messenger->addStatus('PHP Status');
+ $messenger->addWarning('PHP Warning');
+ $messenger->addError('PHP Error');
+
+ return $this->messageLinks();
+ }
+
/**
* Displays links to show messages via JavaScript.
*
diff --git a/core/modules/system/tests/modules/olivero_test/olivero_test.module b/core/modules/system/tests/modules/olivero_test/olivero_test.module
index 2f6ccb442031..5c2c0c4a44e7 100644
--- a/core/modules/system/tests/modules/olivero_test/olivero_test.module
+++ b/core/modules/system/tests/modules/olivero_test/olivero_test.module
@@ -15,25 +15,6 @@ function olivero_test_preprocess_field_multiple_value_form(&$variables) {
}
}
-/**
- * Implements hook_preprocess().
- */
-function olivero_test_preprocess_page(&$variables) {
- $route = \Drupal::routeMatch()->getRouteName();
-
- switch ($route) {
- case 'js_message_test.links':
- $messenger = \Drupal::messenger();
- $messenger->addStatus('PHP Status');
- $messenger->addWarning('PHP Warning');
- $messenger->addError('PHP Error');
- break;
-
- default:
- break;
- }
-}
-
/**
* Implements hook_preprocess_html().
*/
diff --git a/core/profiles/nightwatch_testing/nightwatch_testing.info.yml b/core/profiles/nightwatch_testing/nightwatch_testing.info.yml
index 6e3f05ad741c..c13d545635ba 100644
--- a/core/profiles/nightwatch_testing/nightwatch_testing.info.yml
+++ b/core/profiles/nightwatch_testing/nightwatch_testing.info.yml
@@ -5,3 +5,4 @@ version: VERSION
hidden: true
install:
- js_testing_log_test
+ - nightwatch_theme_install_utility
diff --git a/core/tests/Drupal/Nightwatch/Tests/Claro/claroDrupalMessageTest.js b/core/tests/Drupal/Nightwatch/Tests/Claro/claroDrupalMessageTest.js
new file mode 100644
index 000000000000..e4366650142f
--- /dev/null
+++ b/core/tests/Drupal/Nightwatch/Tests/Claro/claroDrupalMessageTest.js
@@ -0,0 +1,101 @@
+const mainContent = '.region-content';
+const mainMessagesContainer =
+ '[data-drupal-messages] > .messages-list__wrapper';
+const secondaryMessagesContainer = '[data-drupal-messages-other]';
+
+const mainButtons = {
+ addStatus: '#add--status',
+ removeStatus: '#remove--status',
+ addError: '#add--error',
+ removeError: '#remove--error',
+ addWarning: '#add--warning',
+ removeWarning: '#remove--warning',
+ clearAll: '#clear-all',
+};
+
+const secondaryButtons = {
+ addStatus: '[id="add-[data-drupal-messages-other]-status"]',
+ removeStatus: '[id="remove-[data-drupal-messages-other]-status"]',
+ addError: '[id="add-[data-drupal-messages-other]-error"]',
+ removeError: '[id="remove-[data-drupal-messages-other]-error"]',
+ addWarning: '[id="add-[data-drupal-messages-other]-warning"]',
+ removeWarning: '[id="remove-[data-drupal-messages-other]-warning"]',
+};
+
+module.exports = {
+ '@tags': ['core', 'claro'],
+ before(browser) {
+ browser
+ .drupalInstall()
+ .drupalInstallModule('js_message_test')
+ .drupalEnableTheme('claro');
+ },
+ after(browser) {
+ browser.drupalUninstall();
+ },
+ 'Verify default placement of javascript-created messages': (browser) => {
+ browser
+ .drupalRelativeURL('/js_message_test_link_with_system_messages')
+ .waitForElementVisible(mainContent)
+ .assert.elementPresent(mainMessagesContainer)
+
+ // We should load 3 messages on page load from \Drupal::messenger()
+ .assert.elementCount(`${mainMessagesContainer} > .messages-list__item`, 3)
+
+ // We should have one message of each type
+ .assert.elementCount(`${mainMessagesContainer} > .messages--status`, 1)
+ .assert.elementCount(`${mainMessagesContainer} > .messages--warning`, 1)
+ .assert.elementCount(`${mainMessagesContainer} > .messages--error`, 1)
+
+ // Trigger new messages via javascript
+ .click(mainButtons.addStatus)
+ .click(mainButtons.addWarning)
+ .click(mainButtons.addError)
+
+ // We should have 6 total messages
+ .assert.elementCount(`${mainMessagesContainer} > .messages-list__item`, 6)
+
+ // We should have 2 messages of each type
+ .assert.elementCount(`${mainMessagesContainer} > .messages--status`, 2)
+ .assert.elementCount(`${mainMessagesContainer} > .messages--warning`, 2)
+ .assert.elementCount(`${mainMessagesContainer} > .messages--error`, 2);
+ },
+
+ 'Verify customized placement of javascript-created messages': (browser) => {
+ browser
+ .drupalRelativeURL('/js_message_test_link_with_system_messages')
+ .waitForElementVisible(mainContent)
+ .assert.elementPresent(secondaryMessagesContainer)
+
+ // We should load 3 messages on page load from \Drupal::messenger()
+ .assert.elementCount(
+ `${secondaryMessagesContainer} > .messages-list__item`,
+ 0,
+ )
+
+ // Trigger new messages via javascript
+ .click(secondaryButtons.addStatus)
+ .click(secondaryButtons.addWarning)
+ .click(secondaryButtons.addError)
+
+ // We should have 6 total messages
+ .assert.elementCount(
+ `${secondaryMessagesContainer} > .messages-list__item`,
+ 3,
+ )
+
+ // We should have 2 messages of each type
+ .assert.elementCount(
+ `${secondaryMessagesContainer} > .messages--status`,
+ 1,
+ )
+ .assert.elementCount(
+ `${secondaryMessagesContainer} > .messages--warning`,
+ 1,
+ )
+ .assert.elementCount(
+ `${secondaryMessagesContainer} > .messages--error`,
+ 1,
+ );
+ },
+};
diff --git a/core/tests/Drupal/Nightwatch/Tests/Olivero/oliveroDrupalMessageTest.js b/core/tests/Drupal/Nightwatch/Tests/Olivero/oliveroDrupalMessageTest.js
index 958f33381f03..9e145eded55c 100644
--- a/core/tests/Drupal/Nightwatch/Tests/Olivero/oliveroDrupalMessageTest.js
+++ b/core/tests/Drupal/Nightwatch/Tests/Olivero/oliveroDrupalMessageTest.js
@@ -35,7 +35,7 @@ module.exports = {
},
'Verify default placement of javascript-created messages': (browser) => {
browser
- .drupalRelativeURL('/js_message_test_link')
+ .drupalRelativeURL('/js_message_test_link_with_system_messages')
.waitForElementVisible(mainContent)
.assert.elementPresent(mainMessagesContainer)
@@ -63,7 +63,7 @@ module.exports = {
'Verify customized placement of javascript-created messages': (browser) => {
browser
- .drupalRelativeURL('/js_message_test_link')
+ .drupalRelativeURL('/js_message_test_link_with_system_messages')
.waitForElementVisible(mainContent)
.assert.elementPresent(secondaryMessagesContainer)
diff --git a/core/themes/claro/js/messages.js b/core/themes/claro/js/messages.js
index 196568742af7..26632ae6bce1 100644
--- a/core/themes/claro/js/messages.js
+++ b/core/themes/claro/js/messages.js
@@ -25,7 +25,10 @@
const messagesTypes = Drupal.Message.getMessageTypeLabels();
const messageWrapper = document.createElement('div');
- messageWrapper.setAttribute('class', `messages messages--${type}`);
+ messageWrapper.setAttribute(
+ 'class',
+ `messages messages--${type} messages-list__item`,
+ );
messageWrapper.setAttribute(
'role',
type === 'error' || type === 'warning' ? 'alert' : 'status',
diff --git a/core/themes/claro/templates/misc/status-messages.html.twig b/core/themes/claro/templates/misc/status-messages.html.twig
index 91207a54ccd3..7708a745b2fc 100644
--- a/core/themes/claro/templates/misc/status-messages.html.twig
+++ b/core/themes/claro/templates/misc/status-messages.html.twig
@@ -23,50 +23,52 @@
*/
#}
<div data-drupal-messages class="messages-list">
-{% for type, messages in message_list %}
- {%
- set classes = [
- 'messages-list__item',
- 'messages',
- 'messages--' ~ type,
- ]
- %}
- {%
- set is_message_with_title = status_headings[type]
- %}
- {%
- set is_message_with_icon = type in ['error', 'status', 'warning']
- %}
+ <div class="messages-list__wrapper">
+ {% for type, messages in message_list %}
+ {%
+ set classes = [
+ 'messages-list__item',
+ 'messages',
+ 'messages--' ~ type,
+ ]
+ %}
+ {%
+ set is_message_with_title = status_headings[type]
+ %}
+ {%
+ set is_message_with_icon = type in ['error', 'status', 'warning']
+ %}
- <div role="contentinfo" aria-labelledby="{{ title_ids[type] }}"{{ attributes.addClass(classes)|without('role', 'aria-label') }}>
- {% if type == 'error' %}
- <div role="alert">
- {% endif %}
- {% if is_message_with_title or is_message_with_icon %}
- <div class="messages__header">
- {% if is_message_with_title %}
- <h2 id="{{ title_ids[type] }}" class="messages__title">
- {{ status_headings[type] }}
- </h2>
+ <div role="contentinfo" aria-labelledby="{{ title_ids[type] }}"{{ attributes.addClass(classes)|without('role', 'aria-label') }}>
+ {% if type == 'error' %}
+ <div role="alert">
+ {% endif %}
+ {% if is_message_with_title or is_message_with_icon %}
+ <div class="messages__header">
+ {% if is_message_with_title %}
+ <h2 id="{{ title_ids[type] }}" class="messages__title">
+ {{ status_headings[type] }}
+ </h2>
+ {% endif %}
+ </div>
{% endif %}
- </div>
- {% endif %}
- <div class="messages__content">
- {% if messages|length > 1 %}
- <ul class="messages__list">
- {% for message in messages %}
- <li class="messages__item">{{ message }}</li>
- {% endfor %}
- </ul>
- {% else %}
- {{ messages|first }}
+ <div class="messages__content">
+ {% if messages|length > 1 %}
+ <ul class="messages__list">
+ {% for message in messages %}
+ <li class="messages__item">{{ message }}</li>
+ {% endfor %}
+ </ul>
+ {% else %}
+ {{ messages|first }}
+ {% endif %}
+ </div>
+ {% if type == 'error' %}
+ </div>
{% endif %}
</div>
- {% if type == 'error' %}
- </div>
- {% endif %}
+ {# Remove type specific classes. #}
+ {% set attributes = attributes.removeClass(classes) %}
+ {% endfor %}
</div>
- {# Remove type specific classes. #}
- {% set attributes = attributes.removeClass(classes) %}
-{% endfor %}
</div>
--
GitLab