Răsfoiți Sursa

Migrate `window.addMessage` to a Stimulus Controller `w-message`

- Introduce a new controller `MessagesController` to contain the dynamic updating of messages
- Ensure the document updated label does not repeat every time a document is updated
- Using the Stimulus controller with templates approach, icons can easily be pre-loaded for each message type
- Ensure that messages are consistently cleared when new ones are added (this was done ad-hoc across some usage and not others)
- Fixes #9493
4the4ryushin 2 ani în urmă
părinte
comite
eb5bb5a9c8

+ 0 - 1
.eslintrc.js

@@ -168,7 +168,6 @@ module.exports = {
     {
       files: ['wagtail/**/**'],
       globals: {
-        addMessage: 'readonly',
         buildExpandingFormset: 'readonly',
         cancelSpinner: 'readonly',
         escapeHtml: 'readonly',

+ 3 - 0
CHANGELOG.txt

@@ -14,6 +14,7 @@ Changelog
  * Copy page form now updates the slug field dynamically with a slugified value on blur (Loveth Omokaro)
  * Ensure selected collection is kept when navigating from documents or images listings to add multiple views & upon upload (Aman Pandey, Bojan Mihelac)
  * Keep applied filters when downloading form submissions (Suyash Srivastava)
+ * Messages added dynamically via JavaScript now have an icon to be consistent with those supplied in the page's HTML (Aman Pandey)
  * Fix: Ensure `label_format` on StructBlock gracefully handles missing variables (Aadi jindal)
  * Fix: Adopt a no-JavaScript and more accessible solution for the 'Reset to default' switch to Gravatar when editing user profile (Loveth Omokaro)
  * Fix: Ensure `Site.get_site_root_paths` works on cache backends that do not preserve Python objects (Jaap Roes)
@@ -31,6 +32,7 @@ Changelog
  * Fix: Add missing log information for `wagtail.schedule.cancel` (Stefan Hammer)
  * Fix: Fix timezone activation leaking into subsequent requests in `require_admin_access()` (Stefan Hammer)
  * Fix: Fix dialog component's message to have rounded corners at the top side (Sam)
+ * Fix: When multiple documents are uploaded and then subsequently updated, ensure that existing success messages are cleared correctly (Aman Pandey)
  * Docs: Add code block to make it easier to understand contribution docs (Suyash Singh)
  * Docs: Add new "Icons" page for icons customisation and reuse across the admin interface (Coen van der Kamp)
  * Docs: Fix broken formatting for MultiFieldPanel / FieldRowPanel permission kwarg docs (Matt Westcott)
@@ -58,6 +60,7 @@ Changelog
  * Maintenance: Use shared header template for `ModelAdmin` header (Aman Pandey)
  * Maintenance: Move models and forms for `wagtailsearch.Query` to `wagtail.contrib.search_promotions` (Karl Hobley)
  * Maintenance: Migrate `initErrorDetection` (tabs error counts) to a Stimulus Controller `w-count` (Aman Pandey)
+ * Maintenance: Migrate `window.addMessage` behaviour to a global event listener & Stimulus Controller approach with `w-messages` (Aman Pandey)
 
 
 4.2.1 (xx.xx.xxxx) - IN DEVELOPMENT

+ 214 - 0
client/src/controllers/MessagesController.test.js

@@ -0,0 +1,214 @@
+import { Application } from '@hotwired/stimulus';
+import { MessagesController } from './MessagesController';
+
+jest.useFakeTimers();
+
+describe('MessagesController', () => {
+  let application;
+
+  afterEach(() => {
+    jest.clearAllTimers();
+  });
+
+  describe('default behaviour', () => {
+    const addedListener = jest.fn();
+
+    document.addEventListener('w-messages:added', addedListener);
+
+    beforeAll(() => {
+      application?.stop();
+      document.body.innerHTML = `
+    <div
+      class="messages"
+      data-controller="w-messages"
+      data-action="w-messages:add@document->w-messages#add"
+    >
+      <ul data-w-messages-target="container"></ul>
+      <template data-w-messages-target="template">
+        <li class="success"><span></span></li>
+      </template>
+    </div>`;
+
+      application = Application.start();
+      application.register('w-messages', MessagesController);
+    });
+
+    it('should not add elements when connected by default', () => {
+      expect(document.querySelectorAll('li')).toHaveLength(0);
+      expect(addedListener).not.toHaveBeenCalled();
+    });
+
+    it('should allow for a message to be added via the add method', () => {
+      const text = 'first message text';
+
+      document.dispatchEvent(
+        new CustomEvent('w-messages:add', { detail: { text } }),
+      );
+
+      // the item should be added
+      const item = document.querySelector('li');
+
+      expect(item.classList.toString()).toEqual('success');
+      expect(item.lastElementChild.textContent).toEqual(text);
+
+      expect(addedListener).toHaveBeenCalledTimes(1);
+    });
+
+    it('should allow for a second message to be added', async () => {
+      expect(document.querySelectorAll('li')).toHaveLength(1);
+
+      const text = 'second message text';
+
+      document.dispatchEvent(
+        new CustomEvent('w-messages:add', { detail: { text } }),
+      );
+
+      expect(document.querySelectorAll('li')).toHaveLength(2);
+
+      // the item should be added
+      const item = document.querySelector('li:last-child');
+
+      expect(item.classList.toString()).toEqual('success');
+      expect(item.lastElementChild.textContent).toEqual(text);
+
+      expect(addedListener).toHaveBeenCalledTimes(2);
+    });
+
+    it('should fall back to default (first) status, if invalid type is provided', async () => {
+      expect(document.querySelectorAll('li')).toHaveLength(2);
+      const text = 'third message text';
+      document.dispatchEvent(
+        new CustomEvent('w-messages:add', {
+          detail: { text, type: 'invalid' },
+        }),
+      );
+      expect(document.querySelectorAll('li')).toHaveLength(3);
+      const item = document.querySelector('li:last-child');
+      expect(item.classList.toString()).toEqual('success');
+      expect(item.lastElementChild.textContent).toEqual(text);
+    });
+  });
+
+  describe('additional behaviour', () => {
+    beforeAll(() => {
+      application?.stop();
+      document.body.innerHTML = `
+    <div
+      class="messages"
+      data-controller="w-messages"
+      data-action="w-messages:add@document->w-messages#add"
+      data-w-messages-added-class="new"
+      data-w-messages-show-class="appear"
+    >
+      <ul data-w-messages-target="container"></ul>
+      <template data-w-messages-target="template" data-type="success">
+        <li class="success"><span></span></li>
+      </template>
+      <template data-w-messages-target="template" data-type="error">
+        <li class="error"><strong></strong></li>
+      </template>
+      <template data-w-messages-target="template" data-type="warning">
+        <li class="warning"><span></span></li>
+      </template>
+    </div>`;
+
+      application = Application.start();
+      application.register('w-messages', MessagesController);
+    });
+
+    it('should not add any classes when connected by default', () => {
+      expect(document.querySelector('.messages').classList.toString()).toEqual(
+        'messages',
+      );
+
+      expect(document.querySelectorAll('li')).toHaveLength(0);
+    });
+
+    it('should allow for a message to be added via the add method', () => {
+      const text = 'first message text';
+
+      document.dispatchEvent(
+        new CustomEvent('w-messages:add', { detail: { text } }),
+      );
+
+      // set the new class on the container
+      expect(
+        document.querySelector('.messages').classList.contains('new'),
+      ).toBe(true);
+
+      // the item should be added
+      const item = document.querySelector('li');
+
+      expect(item.classList.toString()).toEqual('success');
+      expect(item.lastElementChild.textContent).toEqual(text);
+
+      // it should add a shown class to the message after the timeout
+      jest.runAllTimers();
+
+      expect(document.querySelector('.messages').classList.toString()).toEqual(
+        'messages new appear',
+      );
+    });
+
+    it('should allow for a second message to be added with a specific status', async () => {
+      expect(document.querySelectorAll('li')).toHaveLength(1);
+
+      const text = 'second message text';
+
+      document.dispatchEvent(
+        new CustomEvent('w-messages:add', {
+          detail: { text, type: 'warning' },
+        }),
+      );
+
+      expect(document.querySelectorAll('li')).toHaveLength(2);
+
+      // the item should be added
+      const item = document.querySelector('li:last-child');
+
+      expect(item.classList.toString()).toEqual('warning');
+      expect(item.lastElementChild.textContent).toEqual(text);
+    });
+
+    it('should allow for any last child in the matched template to have content replaced', async () => {
+      expect(document.querySelectorAll('li')).toHaveLength(2);
+
+      const text = 'third message text';
+
+      document.dispatchEvent(
+        new CustomEvent('w-messages:add', {
+          detail: { text, type: 'error' },
+        }),
+      );
+
+      expect(document.querySelectorAll('li')).toHaveLength(3);
+
+      // the item should be added
+      const item = document.querySelector('li:last-child');
+
+      expect(item.classList.toString()).toEqual('error');
+      // note: finding the strong element
+      expect(item.querySelector('strong').textContent).toEqual(text);
+    });
+
+    it('should allow for items to be cleared when adding a new one', () => {
+      expect(document.querySelectorAll('li')).toHaveLength(3);
+
+      const text = 'new message text';
+
+      document.dispatchEvent(
+        new CustomEvent('w-messages:add', {
+          detail: { clear: true, text, type: 'warning' },
+        }),
+      );
+
+      expect(document.querySelectorAll('li')).toHaveLength(1);
+
+      const item = document.querySelector('li');
+
+      expect(item.classList.toString()).toEqual('warning');
+
+      expect(item.lastElementChild.textContent).toEqual(text);
+    });
+  });
+});

+ 90 - 0
client/src/controllers/MessagesController.ts

@@ -0,0 +1,90 @@
+import { Controller } from '@hotwired/stimulus';
+
+/**
+ * Adds the ability for a controlled element to pick an element from a template
+ * and then clone that element, adding it to the container.
+ * Additionally, it will allow for clearing all previously added elements.
+ *
+ * @example
+ * <div
+ *   data-controller="w-messages"
+ *   data-action="w-messages:add@document->w-messages#add"
+ *   data-w-messages-added-class="new"
+ *   data-w-messages-show-class="appear"
+ * >
+ *   <ul data-w-messages-target="container"></ul>
+ *   <template data-w-messages-target="template">
+ *     <li data-message-status="error-or-success"><span></span></li>
+ *  </template>
+ * </div>
+ */
+export class MessagesController extends Controller<HTMLElement> {
+  static classes = ['added', 'show'];
+  static targets = ['container', 'template'];
+  static values = {
+    showDelay: { default: 100, type: Number },
+  };
+
+  declare readonly addedClass: string;
+  declare readonly containerTarget: HTMLElement;
+  declare readonly hasAddedClass: boolean;
+  declare readonly hasContainerTarget: boolean;
+  declare readonly hasShowClass: boolean;
+  declare readonly showClass: string;
+  declare readonly templateTarget: HTMLTemplateElement;
+  declare readonly templateTargets: HTMLTemplateElement[];
+
+  declare showDelayValue: number;
+
+  add(
+    event?: CustomEvent<{
+      /** Flag for clearing or stacking messages */
+      clear?: boolean;
+      /** Content for the message, HTML not supported. */
+      text?: string;
+      /** Message status level, based on Django's message types. */
+      type?: 'success' | 'error' | 'warning' | string;
+    }>,
+  ) {
+    const { clear = false, text = '', type } = event?.detail || {};
+
+    if (this.hasAddedClass) {
+      this.element.classList.add(this.addedClass);
+    }
+
+    if (clear) this.clear();
+
+    /** if no type provided, return the first template target, otherwise try to find
+     * a matching target, finally fall back on the first template target if nothing
+     * is found.
+     */
+    const template =
+      (type &&
+        this.templateTargets.find(({ dataset }) => dataset.type === type)) ||
+      this.templateTarget;
+
+    const content = template.content.firstElementChild?.cloneNode(true);
+
+    if (content instanceof HTMLElement) {
+      const textElement = content.lastElementChild;
+
+      if (textElement instanceof HTMLElement && text) {
+        textElement.textContent = text;
+      }
+
+      this.containerTarget.appendChild(content);
+
+      this.dispatch('added');
+
+      if (this.hasShowClass) {
+        setTimeout(() => {
+          this.element.classList.add(this.showClass);
+        }, this.showDelayValue);
+      }
+    }
+  }
+
+  clear() {
+    this.containerTarget.innerHTML = '';
+  }
+}

+ 2 - 0
client/src/controllers/index.ts

@@ -3,6 +3,7 @@ import type { Definition } from '@hotwired/stimulus';
 // Order controller imports alphabetically.
 import { ActionController } from './ActionController';
 import { CountController } from './CountController';
+import { MessagesController } from './MessagesController';
 import { ProgressController } from './ProgressController';
 import { SkipLinkController } from './SkipLinkController';
 import { SlugController } from './SlugController';
@@ -16,6 +17,7 @@ export const coreControllerDefinitions: Definition[] = [
   // Keep this list in alphabetical order
   { controllerConstructor: ActionController, identifier: 'w-action' },
   { controllerConstructor: CountController, identifier: 'w-count' },
+  { controllerConstructor: MessagesController, identifier: 'w-messages' },
   { controllerConstructor: ProgressController, identifier: 'w-progress' },
   { controllerConstructor: SkipLinkController, identifier: 'w-skip-link' },
   { controllerConstructor: SlugController, identifier: 'w-slug' },

+ 0 - 14
client/src/entrypoints/admin/core.js

@@ -10,20 +10,6 @@ import { initTooltips } from '../../includes/initTooltips';
 /** initialise Wagtail Stimulus application with core controller definitions */
 window.Stimulus = initStimulus({ definitions: coreControllerDefinitions });
 
-/* generic function for adding a message to message area through JS alone */
-function addMessage(status, text) {
-  $('.messages')
-    .addClass('new')
-    .empty()
-    .append('<ul><li class="' + status + '">' + text + '</li></ul>');
-  const addMsgTimeout = setTimeout(() => {
-    $('.messages').addClass('appear');
-    clearTimeout(addMsgTimeout);
-  }, 100);
-}
-
-window.addMessage = addMessage;
-
 window.escapeHtml = escapeHtml;
 
 window.initTagField = initTagField;

+ 33 - 0
docs/releases/5.0.md

@@ -26,6 +26,7 @@ Support for adding custom validation logic to StreamField blocks has been formal
  * Copy page form now updates the slug field dynamically with a slugified value on blur (Loveth Omokaro)
  * Ensure selected collection is kept when navigating from documents or images listings to add multiple views & upon upload (Aman Pandey, Bojan Mihelac)
  * Keep applied filters when downloading form submissions (Suyash Srivastava)
+ * Messages added dynamically via JavaScript now have an icon to be consistent with those supplied in the page's HTML (Aman Pandey)
 
 ### Bug fixes
 
@@ -45,6 +46,7 @@ Support for adding custom validation logic to StreamField blocks has been formal
  * Add missing log information for `wagtail.schedule.cancel` (Stefan Hammer)
  * Fix timezone activation leaking into subsequent requests in `require_admin_access()` (Stefan Hammer)
  * Fix dialog component's message to have rounded corners at the top side (Sam)
+ * When multiple documents are uploaded and then subsequently updated, ensure that existing success messages are cleared correctly (Aman Pandey)
 
 ### Documentation
 
@@ -79,6 +81,7 @@ Support for adding custom validation logic to StreamField blocks has been formal
  * Use shared header template for `ModelAdmin` header (Aman Pandey)
  * Move models and forms for `wagtailsearch.Query` to `wagtail.contrib.search_promotions` (Karl Hobley)
  * Migrate `initErrorDetection` (tabs error counts) to a Stimulus Controller `w-count` (Aman Pandey)
+ * Migrate `window.addMessage` behaviour to a global event listener & Stimulus Controller approach with `w-messages` (Aman Pandey)
 
 ## Upgrade considerations
 
@@ -207,6 +210,36 @@ Stimulus [targets](https://stimulus.hotwired.dev/reference/targets) and [actions
 * `<button ... data-action="w-progress#activate:once" ...>` - only trigger the progress behaviour once
 * `<button ... data-action="readystatechange@document->w-progress#activate:once" data-w-progress-duration-value="5000" disabled ...>` - disabled on load (once JS starts) and becomes enabled after 5s duration
 
+### JavaScript `window.addMessages` replaced with event dispatching
+
+The undocumented `window.addMessage` function is no longer available and will throw an error if called, if similar functionality is required use DOM Event dispatching instead as follows.
+
+```js
+// old
+window.addMessage('success', 'Content has updated');
+```
+
+```js
+// new
+document.dispatchEvent(
+    new CustomEvent('w-messages:add', {
+        detail: { text: 'Content has updated', type: 'success' },
+    }),
+);
+// new (clearing existing messages before adding a new one)
+document.dispatchEvent(
+    new CustomEvent('w-messages:add', {
+        detail: {
+            clear: true,
+            text: 'All content has updated',
+            type: 'success',
+        },
+    }),
+);
+// message types 'success', 'error', 'warning' are supported
+```
+
+Note that this event name may change in the future and this functionality is still not officially supported.
 
 ### Changes to StreamField `ValidationError` classes
 

+ 14 - 5
wagtail/admin/templates/wagtailadmin/base.html

@@ -8,9 +8,9 @@
     <main class="content-wrapper w-overflow-x-hidden" id="main">
         <div class="content">
             {# Always show messages div so it can be appended to by JS #}
-            <div class="messages" role="status">
-                {% if messages %}
-                    <ul>
+            <div class="messages" role="status" data-controller="w-messages" data-action="w-messages:add@document->w-messages#add" data-w-messages-added-class="new" data-w-messages-show-class="appear">
+                <ul data-w-messages-target="container">
+                    {% if messages %}
                         {% for message in messages %}
                             {% message_level_tag message as level_tag %}
                             <li class="{% message_tags message %}">
@@ -27,8 +27,17 @@
                                 {{ message|safe }}
                             </li>
                         {% endfor %}
-                    </ul>
-                {% endif %}
+                    {% endif %}
+                </ul>
+                <template data-w-messages-target="template" data-type="success">
+                    <li class="success">{% icon name="success" classname="messages-icon" %}<span></span></li>
+                </template>
+                <template data-w-messages-target="template" data-type="success">
+                    <li class="error">{% icon name="warning" classname="messages-icon" %}<span></span></li>
+                </template>
+                <template data-w-messages-target="template" data-type="warning">
+                    <li class="warning">{% icon name="warning" classname="messages-icon" %}<span></span></li>
+                </template>
             </div>
 
             {% block content %}{% endblock %}

+ 6 - 2
wagtail/admin/templates/wagtailadmin/pages/index.html

@@ -86,7 +86,9 @@
 
                         // Post
                         $.post(url, {csrfmiddlewaretoken: CSRFToken}, function(){
-                            addMessage('success', '"' + $(movedElement).data('page-title') + '" has been moved successfully.')
+                            const text = `"${$(movedElement).data('page-title')}" has been moved successfully.`;
+                            const event = new CustomEvent('w-messages:add', { detail: { clear: true, text,  type: 'success' } });
+                            document.dispatchEvent(event);
                         })
                     }
                 });
@@ -117,7 +119,9 @@
                         url += `?position=${(index)}`;
                         let CSRFToken = $('input[name="csrfmiddlewaretoken"]', orderform).val();
                         $.post(url, {csrfmiddlewaretoken: CSRFToken}, function(){
-                            addMessage('success', `"${$(currentlySelected).data('page-title')}" has been moved successfully from ${currentPosition +1} to ${index +1}.`)
+                            const text = `"${$(currentlySelected).data('page-title')}" has been moved successfully from ${currentPosition + 1} to ${index + 1}.`;
+                            const event = new CustomEvent('w-messages:add', { detail: { clear: true, text, type: 'success' } });
+                            document.dispatchEvent(event);
                         }).done(function() {
                             currentlySelected = undefined;
                         })

+ 6 - 2
wagtail/documents/static_src/wagtaildocs/js/add-multiple.js

@@ -163,8 +163,12 @@ $(function () {
       url: this.action,
     }).done(function (data) {
       if (data.success) {
-        var statusText = $('.status-msg.update-success').text();
-        addMessage('success', statusText);
+        var text = $('.status-msg.update-success').first().text();
+        document.dispatchEvent(
+          new CustomEvent('w-messages:add', {
+            detail: { clear: true, text, type: 'success' },
+          }),
+        );
         itemElement.slideUp(function () {
           $(this).remove();
         });

+ 6 - 3
wagtail/images/static_src/wagtailimages/js/add-multiple.js

@@ -19,7 +19,6 @@ $(function () {
       maxFileSize: window.fileupload_opts.errormessages.max_file_size,
     },
     add: function (e, data) {
-      $('.messages').empty();
       var $this = $(this);
       var that = $this.data('blueimp-fileupload') || $this.data('fileupload');
       var li = $($('#upload-list-item').html()).addClass('upload-uploading');
@@ -204,8 +203,12 @@ $(function () {
       url: this.action,
     }).done(function (data) {
       if (data.success) {
-        var statusText = $('.status-msg.update-success').text();
-        addMessage('success', statusText);
+        var text = $('.status-msg.update-success').first().text();
+        document.dispatchEvent(
+          new CustomEvent('w-messages:add', {
+            detail: { clear: true, text, type: 'success' },
+          }),
+        );
         itemElement.slideUp(function () {
           $(this).remove();
         });