Provide a common API for displaying JavaScript messages

There would need to be a style to make the div invisible if empty though, so that JS can reveal it. Otherwise you might get an empty container, depending on the theme.

I'd like that, it should be fairly simple.

So initial patch for review :)-

Patch is not working due to white space problem in patch "content_edit_ajax_message-7342336-7.patch" !!!

Hi Team,

I've updated the toolbar-view.js file to show status message under div as a solution for displaying status message on field edit. Attaching patch for review.

I've updated the toolbar-view.js file to show status message under div as a solution for displaying status message on field edit. Attaching patch for review :)-

The current patch won't work for core. We need something more generic.

If we're going down this road we need to add a JS API to handle messages. Like it was said in #1 we'd need all theme to remove the if ($messages) {} so that the DOM element is always available. We could be using that for the tabledrag and views warning about unsaved changes too.

Ideally I wouldn't mind to have Drupal.announce use that message area to put all it's stuff for screen readers too. Would keep everything at the same place.

Totally not ready but putting NR for attention.

Patch contains what I mean. Needs to be polished and all, probably merged with drupal.announce too actually.

adds 2 methods:

// Add a message to the message area. Used the same as drupal_set_message().
Drupal.message(message, type);

// Remove everything in the message area.
Drupal.message.clear();

The message area is designated with a data-drupal-messages attribute because id of the zone is different in default template and the seven theme. Also bartik adds a wrapper we don't want to have to deal with.

I don't think we want a fancy "dismiss message" functionality. If you want to get rid of the messages, just reload the page like it's done currently. simple > fancy.

Needs to be polished and all, probably merged with drupal.announce too actually.

(bolding added).

Yes, indeed. We've already solved an issue with aria-live and near-simultaneous messages: #1959306: Drupal.announce does not handle multiple messages at the same time; Improve the utility so that simultaneously calls are read..

I'd be fine with renaming announce to message or with merging a visual output component to announce in addition to the audio output it currently supports.

I see mention of aria-live in comments in the patch.

+ * Builds a div element with the aria-live attribute and attaches it
+ * to the DOM.

but I don't see the attribute in the code yet.

+Drupal.theme.message = function (message, type) {
+  return '<div class="messages ' + type +'">' + message + '</div>';
+};

vineet.osscube, do you feel like you can take a shot at merging the code here in this issue into the existing announce method?

yeah I copied the annouce file and removed all the comments, not all of them it seems. It'll need commenting.

merged in announce.js, not too bad. At least I don't have the whole library mess like before.

+++ b/core/misc/message.jsundefined
@@ -0,0 +1,38 @@
+  messagesElement.style.display = 'block';

Hmm, Do we really need to do this?

Wouldn't it be better to do a simple class toggling.

yeah that was a very rough patch. new one is better.

Also a lot of people are going to dis-like the forcing of the message block to be printed at all times.

print only when it fails :)

Great to see this nicely integrated with Drupal.announce :)

+++ b/core/misc/announce.jsundefined
@@ -25,6 +28,9 @@
+      if (!messagesElement) {
+        messagesElement = document.querySelector('[data-drupal-messages]');
+      }
       // Create only one aria-live element.
       if (!liveElement) {
         liveElement = document.createElement('div');
@@ -32,7 +38,7 @@

@@ -32,7 +38,7 @@
         liveElement.className = 'element-invisible';
         liveElement.setAttribute('aria-live', 'polite');
         liveElement.setAttribute('aria-busy', 'false');
-        document.body.appendChild(liveElement);
+        messagesElement.parentNode.insertBefore(liveElement, messagesElement);

- What if a theme specifically doesn't want a <div data-drupal-messages />? Shouldn't that still be allowed?
- When a theme specifically doesn't have such an element, this will break Drupal.announce() too, because it now effectively depends on such an element.

+++ b/core/misc/announce.jsundefined
@@ -41,34 +47,47 @@
+    if (messages.length) {
+      text = [];
+      for (var n = 0, nl = messages.length; n < nl; n++) {
+        message = messages.pop();

I don't like that Drupal.announce() is now also handling the rendering for Drupal.message().

AFAICT there are valid use cases to use Drupal.announce() *without* showing a visible message, so I don't think we should let this internal overlap occur.

It also seems to imply a single instance, is that something we want to force people into, or would supporting multiple messages areas be a bit over the top?

about [drupal-data-messages] I think it should be required for theme to have an element with the attribute all the time.

Technically it's not Drupal.announce() that takes care of displaying, it's announce(). If I rename it processQueues() would that help? it's what it's doing after all. If you want announce and not a message then don't fill both queues at the same time :p

I'd say we keep it in only one place. If contrib want to use it somewhere else they'll probably want to be able to "stick" one message over pages, close a specific message, make it show up fancy and all. All sort of stuff we're not supporting with the current patch. I'm pretty sure someone will throw together a module with some jQuery plugin to handle that (and they'll just need to override Drupal.message()).

Updated patch, there is a problem in how the debounce is actually used.

I separated the messages and announce processing so that should make you happy Wim :) They're even in two files just in case contrib wants to mess with it.

ok little small patch creation issue :p

BTW to see what's happening try that in the JS console:

Drupal.message('good message');
Drupal.message('Bad message', 'error');
Drupal.message('Careful message', 'warning');
Drupal.message('Whatever', 'whatever');

Patch doesn't apply anymore since removal of tpl.php files for TWIG. Tagging for reroll.

Re-rolled.

I think some page.html.twig changes are missing. Thanks for the re-roll.

reroll with template changes.

At DrupalCon Portland 2013 I was assigned to manually test this patch. Patch 25 worked for me but changes happened rapidly and I was unsure how to reset my git branch to the latest.

Patch 25 tests I completed:

System:
OS X 10.8.3 (12D78)

Browsers tested:
Safari
6.0.4 (8536.29.13) - PASSED

Firefox
13.0.1 and 21.0 - PASSED

Opera
12.15 build 1748 - PASSED

UPDATE: I did some reading and figured out how to update my branch and apply the patch but now I get this:

MBP:drupal user$ patch -p1 < core-js-status-messages-77245-33.patch
patching file core/misc/announce.js
The next patch would create the file core/misc/message.js,
which already exists! Assume -R? [n] yes
patching file core/misc/message.js
patching file core/modules/system/system.module
Hunk #1 succeeded at 1212 (offset -13 lines).
patching file core/modules/system/templates/page.html.twig
patching file core/themes/bartik/templates/page.html.twig
patching file core/themes/seven/templates/page.html.twig
MBP:drupal user$ git status
# On branch issue_77245
# Changes not staged for commit:
# (use "git add ..." to update what will be committed)
# (use "git checkout -- ..." to discard changes in working directory)
#
# modified: core/misc/announce.js
# modified: core/modules/system/system.module
# modified: core/modules/system/templates/page.html.twig
# modified: core/themes/bartik/templates/page.html.twig
# modified: core/themes/seven/templates/page.html.twig
#
# Untracked files:
# (use "git add ..." to include in what will be committed)
#
# core-js-status-messages-77245-33.patch
# core/modules/system/system.module.orig
# sites/default/files/
# sites/default/settings.php
no changes added to commit (use "git add" and/or "git commit -a")

I've exhausted my current level of skills. Hope this helps.

The important part is the testing. For git as long as the test bot doesn't complain it's all good.

Once i apply #33 using following command,

In Terminal:
git apply core-js-status-messages-77245-33.patch

My patch file is in my root folder. Its named as "core-js-status-messages-77245-33.patch"

I got the following error:
error: patch failed: core/misc/announce.js:16
error: core/misc/announce.js: patch does not apply

Is there any mistake in unix command?

#36: No, Drupal 8 has just changed so much already that the patch no longer is applicable. It needs to be rerolled. Hence setting to "needs work".

#33 Patch rerolled. Refer (https://drupal.org/patch/reroll)

#38: core-js-status-messages-77245-38.patch queued for re-testing.

reroll

Re-rolled.

Messaging functionality works fine for me, but I have not tested screen reading yet.

Patch is good to go, pinged morten about adding empty div everywhere.

Can we please not use ID's if they are not absolutely extremely nessesary - they make the css work further down the road harder.

1. +++ b/core/modules/system/templates/page.html.twig
   @@ -104,7 +104,7 @@
   -  {{ messages }}
   +  <div id="messages" data-drupal-messages>{{ messages }}</div>
   

   please dont use id's unless its absolutely nessesary, id to make the life of the css harder.
   maybe a class="drupalsystem--messages" instead

   Im generally not happy for having empty divs floating around unless theres a value inside of em. if we can test on {{ messages }} and test if its empty its fine with me even that its a bit cluttered

2. +++ b/core/themes/bartik/templates/page.html.twig
   @@ -128,11 +128,9 @@
   +  <div id="messages"><div class="section clearfix" data-drupal-messages>
   +    {{ messages }}
   +  </div></div> <!-- /.section, /#messages -->
   

   bartik is its own monster (that the frontend team will take on later based on the principles of d8 theming) anyways please dont add more id's in use a class like drupalsystem--messages instead

This should be the only drupal system messages div on the page, so for the purposes of JavaScript, having an ID makes finding it easier. The document keeps a list of ID'ed elements on the page and looking them up is very fast.

For the purposes of styling, I agree, we shouldn't be using the ID. I don't see any CSS in this patch, so it's not introducing any ID-keyed styling.

--- /dev/null
+++ b/core/misc/message.jsundefined
@@ -0,0 +1,58 @@
+/**
+ *
+ */

The messages.js file is missing the file doc block.

+++ b/core/misc/message.jsundefined
@@ -0,0 +1,58 @@
+    // Save the text and priority into a closure variable. Multiple simultaneous
+    // announcements will be concatenated and read in sequence.

This is a copy-paste block of comment from announce.js. It should be updated.

I will fix it ;)

I guess this patch contains all @mortendk and @jessebeach suggestions. Tried to comment the function but I guess it's too verbose. I accept more suggestions regarding that :)

Re-rolled with the latest changes and updated page.html.twig for Seven too.

Also I have a question about the return of Drupal.theme.message function:

return '<div class="messages messages--' + type + '">' + message + '</div>';

It includes a new div so I'm not sure if we should update it according to @mortendk suggestions.

You can't put the if messages. The element has to be on the page all the time, othewise JS can't find it and the whole thing can't work.

Also I have a question about the return of Drupal.theme.message function:

return '<div class="messages messages--' + type + '">' + message + '</div>';

It includes a new div so I'm not sure if we should update it according to @mortendk suggestions.

rteijeiro, I'm not sure how you're suggesting to update this div. Can you elaborate?

Also I have a question about the return of Drupal.theme.message function:

return '<div class="messages messages--' + type + '">' + message + '</div>';

It includes a new div so I'm not sure if we should update it according to @mortendk suggestions.

rteijeiro, I'm not sure how you're suggesting to update this div. Can you elaborate?

@nod_: Ok, I will fix it :'(

@jessebeach: I mean include the "drupalsystem--messages" class suggested by @mortendk instead the "messages" class.

urgh just rmemebered the updated name conversions for css classes

the classname should be prefixed with a js- (js-drupalmessages) that will tell the themers to stay away from messing iwth that class & follow our standards (https://drupal.org/node/1887918#separate-concerns)

@mortendk: The js-drupalmessages class should be in the tpl div or in the div returned by Drupal.theme.message function in the message.js file?

I will try to ping you in IRC in order to get this issue fixed ;)

Talked some more with mortendk, here is the reroll.

This patch adds a (usually empty) <div data-drupal-messages> on all pages for all themes to be able to print errors and warnings from the JS (to finally fix #1232416: Drupal alerts "An AJAX HTTP request terminated abnormally" during normal site operation, confusing site visitors/editors).

I did not touch the classes/ids or anything because it's purely a JS patch. To mess with the markup a follow-up is needed, as long as the data- attribute is still around, the JS will work regardless of what happens to the markup/classes/ids.

The patch also fixes a bad use of debounce in announce.js.

It looks good. Patch applies well and it creates the div placeholder for messages.

Maybe RTBC now?

+++ b/core/misc/announce.js
@@ -42,38 +42,43 @@
-    var priority = 'polite';
-    var announcement;
+    var priority, announcement;

Looks like this breaks the default priority, making it default to "undefined".

Also, the initial implementation didn't seem to follow the spec, as that says that "off" is the default, not "polite".

ref: http://www.w3.org/TR/wai-aria/states_and_properties#aria-live

Also, I was under the impression the idea was to have the messages trigger the announce, otherwise it would be impossible for a11y users to know there was an error, wouldn't it?

Making .message use .annonce is interesting. for now I just want whatever to close down the HTTP:0 issue

update for #62 patch

rerolled to fixed the polite thing raised by seutje.

The patch in #67 simply does not include message.js, hence the test failures.

Thanks @Win Leers for the help :)

Added message.js file to #67 patch

+++ b/core/modules/system/system.module
@@ -983,6 +984,19 @@ function system_library_info() {
+  ¶

Extra white space.

remove whitespace commented in #74

Tested the patch in #75 by calling the following javascript from the chrome dev tool:

Drupal.message('Testing javascript message error', 'error');
Drupal.message('Testing javascript message status', 'status');
Drupal.message('Testing javascript message warning', 'warning');

This worked beautifully and added the message to the page (tested with bartik, stark and seven themes). The patch also applies cleanly so maybe this can be marked RTBC.

I read through all the comments, and while I didn't see any specifics on what needed manual testing, it appears that adequate manual testing has been done, so I'm removing the "needs manual testing" tag. Please put it back if I'm wrong!

Ah yes manual testing was in #76. Because no code is using it yet, needs to be triggered manually :)

RTBC per #76.

I'm sorry, there are a bunch of nitpicks left:

1. +++ b/core/misc/message.js
   @@ -0,0 +1,58 @@
   + *
   

   Missing file-level docs.

2. +++ b/core/misc/message.js
   @@ -0,0 +1,58 @@
   +
   +
   

   2 newlines, should probably be 1.

3. +++ b/core/misc/message.js
   @@ -0,0 +1,58 @@
   +   * to the DOM.
   ...
   +   * Builds a div element with the aria-live attribute and attaches it
   

   Incorrect wrapping: more of the text fits on the first line.

4. +++ b/core/misc/message.js
   @@ -0,0 +1,58 @@
   +  Drupal.theme.message = function (message) {
   

   Docs missing.

5. +++ b/core/misc/message.js
   @@ -0,0 +1,58 @@
   +  Drupal.message = function (message, type) {
   

   Docs missing.

6. +++ b/core/misc/message.js
   @@ -0,0 +1,58 @@
   +  function processMessages () {
   

   I *think* that we also want docs for this.

This patch should fix the nitpicks described in #79. However there are certainly better comments to add than what I have written.

1. +++ b/core/misc/message.js
   @@ -41,6 +45,15 @@
   +   * Display a message on the page.
   
   @@ -23,7 +24,10 @@
   +   * Display all queued messages in one pass instead of one after the other.
   

   s/Display/Displays/

2. +++ b/core/misc/message.js
   @@ -41,6 +45,15 @@
   +   * @param message
   ...
   +   * @param type
   
   @@ -51,6 +64,16 @@
   +   * @param message
   ...
   +   * @return
   

   Parameter types missing

3. +++ b/core/misc/message.js
   @@ -51,6 +64,16 @@
   +   *   This function return a string.
   

   Inconsistent with other JS theme functions in core.

4. +++ b/core/misc/message.js
   @@ -1,5 +1,7 @@
   + * @file
     *
   + * Message API.
   

   Extraneous empty line in the middle.

Thanks @Wim Leers for the quick review.

I'm not sure about the new @return but I found this in toolbar.js and dropbuttons.js.

Thanks, looks good now! :)

This is a reroll because the indentation standard changed for JavaScript files. No need for commit credit because of this reroll.

Reroll seems same as last patch so back to RTBC then.

few questions:

- Why no functions to remove messages posted by Drupal.message?
- Why not filter the undefined messages.
eg:

Drupal.message();
//return: <div class="messages messages--status">undefined</div>

- What if there's a lightbox windows. How to display messages there?
- Should it "throw new Error" that block whole scripts from being executed?
- It will be more useful if debouncedProcessMessages() can be triggered via events.

Nothing to remove messages because I don't want this script to be complicated. The current errors we get from PHP can't be removed, you have to reload to page to get them to disapear. This is intented to display ajax errors (as you can see by the dependency), nothing else in core. If we give the ability to remove messages it's a lot more complex to handle, there are UX concerns and that always takes time to solve.

The lightbox question is a good point. Didn't think of that.

The throw won't stop anything else from executing. We catch attach behaviors is wrapped in a try/catch, that won't prevent anything else from running.

About events not sure it's very useful. Instead of Drupal.message() we'd call $().trigger('message', 'mytext'); ? Events are not the fastest thing ever, we add a dep on jQuery and I don't see how it'd be better. Do you have something in mind?

Here is a reroll to prevent undefined messages to show up.

• This is intented to display ajax errors (as you can see by the dependency),

  It would be more important to have a function to remove the message then. Even an UI changes with "Close" button.

  Imagine there's a uploader that only allow to upload JPG and 100kb of the max size. And then you uploaded a PNG.

• First error: wrong image type

  And then you upload a JPG and files size larger than 100

  Second error: max file size problem

  Now, you have 2 error messages. Do you still upload a wrong type ??

• About events not sure it's very useful. Instead of Drupal.message() we'd call $().trigger('message', 'mytext'); ? Events are not the fastest thing ever, we add a dep on jQuery and I don't see how it'd be better. Do you have something in mind?

  Eg. we could register a function to remove the message after 10s...

• Also,
  if it accepts selectors, it could reuse on other modules, eg. Using on tabledrag to display message (tableDragChangedWarning)

• +++ b/core/misc/message.js
  @@ -0,0 +1,82 @@
  +  Drupal.behaviors.drupalMessage = {
  

  And I think we can remove the behaviors. and setup it on first message call.

Looked a bit into this and it needs to be at least converted to use libraries.yml. I can look more into it tomorrow.

The message adding is going to break after console div is removed in this issue https://www.drupal.org/node/2213583 . The messages could be probably added before region-content or after main-content link?

Also now with core.libraries.yml the js needs to be added as dependency to other script to be included everywhere. Which one should it be? I didn't make a patch yet, but the content would be pretty much this in core.libraries.yml:

drupal.message:
version: VERSION
js:
misc/message.js: {}
dependencies:
- core/drupal
- core/drupal.debounce

Is this still being worked on for 8.0 or is it bumped to 8.1 now?

This related issue landed some time ago: #2289917: Convert "messages" page element into blocks.

I would like to have it in 8.0.x but it needs someone to work on it.

This is basically a reroll of #94.

The patch from is so old, that I won't try to actually make an interdiff.txt. The JS from the patch is untouched, the changes is the "new" way of defining a library, attaching it to the system_messages and changes to new templates, since system message has changed a lot since the patch from a year ago.

We shouldn't attached the drupal.message library by default, also fixes some JSLint issues.

It's more than reroll. @see #95.

Regarding #95

I don't believe that having the messages disappear after a certain amount of time, is a good idea, since we don't know how complex and how many messages will appear, and thus how long they should be on the page.

I don't see that message.js will have any sensible way of determine which messages should be removed, when. We could add a function to remove all messages generated by message.js or provide a context or something similar to that you could remove all validation errors for a specific form upon resubmitting an ajax form. Then it would be up to the scripts invoking the messages, to handle the removal when it makes sense.

Behaviors provides a nice error handling, if we don't have use then, we would need to code our own handling of a case where we don't have a place to put messages.

I've added the concept of context, so it's possible to specify where messages are coming from. I've added a method to remove a messages of a certain type/context, though this could be done manually as well.

This should handle the concerns from #95.

It still uses behaviors as I believe this is the best option we have atm.

- On maintenance mode, upload a file
(same problem on unpatch)

@droplet what we are making is a framework, the patch itself only makes the framework available - it's not actually used anywhere (yet).

To test this

1. You need to include the library on the page, basically add $build['#attached'] = ['library' => ['core/drupal.message']]; somewhere, see patch #105 for more info (this was removed, since we don't want to load a lot of javascript by default on pages)
2. The javascript code to add a message would look like this: Drupal.message.add('the message', 'the type status/warning/error, 'optional context')

Do these messages go into the block mentioned in #102?

Just a re-roll.

The last patch misses message.js

Re-rolled patch 111 https://www.drupal.org/node/77245#comment-9922432

You need to include the library on the page

#type=messages element should add this library

+1 for #124.

1. +++ b/core/core.libraries.yml
   @@ -225,6 +225,15 @@ drupal.machine-name:
   +drupal.message:
   +  version: VERSION
   +  js:
   +    misc/message.js: {}
   +  dependencies:
   +    - core/jquery
   +    - core/drupal
   +    - core/drupal.debounce
   

   Why no dependency on core/drupal.announce?

2. +++ b/core/themes/classy/templates/misc/status-messages.html.twig
   @@ -21,7 +21,7 @@
   -{{ attach_library('classy/messages') }}
   

   This would be a regression today. So, if you do this, then we should add this to classy.info.yml:

   libraries-extend:
     core/drupal.message:
       - classy/messages
   

Is this really addressing the issue? The announce part is good, because it reads the text, so it doesn't matter where on the page the action was initiated from.

The text is added to the top of the page and the top of the page may not be in view. Imagine this comment form using it. I submit through AJAX and the confirmation is all at the top. Sure in the case of comments you can get feedback by prepending the message to the form or adding the comment in it's place. But you have situations where this is not possible (the action is initiated from a table cell, that cell may be to narrow to display a message; or a subscribe form in the footer, you don't want a message to brake the footer layout, you just want a small message appear informing the user the subscription was successful). Something like https://material.angularjs.org/latest/demo/toast

Rerolled.

#126: Yeah, but isn't that for contrib to do?

1. +++ b/core/misc/message.js
   @@ -0,0 +1,104 @@
   +        messagesElement = document.querySelector('[data-drupal-messages]');
   

   We can use jQuery selector here.

2. +++ b/core/misc/message.js
   @@ -0,0 +1,104 @@
   +        message, n, nl;
   

   Let's rename n,nl to better helpful name.

Updates following things as given in previous comments #131.

+++ b/core/misc/message.js
@@ -0,0 +1,104 @@
+ messagesElement = document.querySelector('[data-drupal-messages]');
We can use jQuery selector here.
+++ b/core/misc/message.js
@@ -0,0 +1,104 @@
+ message, n, nl;

Please review.

message.js is missing from the patch.

I'm tagging this for accessibility as it's required to get inline form errors as a fully fledged part of Core.

The patch from #132 needs a re-roll, today's test says it fails to apply.

On it

Here is a re-roll. I've also re-added the messages.js file that was missing in patch #132.

Patched against 8.3.x branch.

Trying to fix the twig template, the indentation went a bit wonky and div conditional went missing for the closing div.
Also undid the context removal but I don't think that's needed for this patch.

Bigpipe test should fail because the removal of the library, what's the reason behind that in the classy template?

Is that intended to remove the JS in #140 and above patches..

the blocker is #91

I agreed with #126 also. We could jump out of the box. Here's an JS lib we can use:
http://codeseven.github.io/toastr/demo.html

@droplet, good catch, it likely was a mistake because it wasn't mentioned. I've added it back into the latest patch.

Also probably blocked a bit on changing Classy markup which is API.

Just wanted to encourage someone to write tests for this patch so we can hopefully meet this deadline:
https://www.drupal.org/node/2504847#comment-11750733

Adding my thoughts about message visibility and toast in #126 and #147.

I agree that when, for example, you are scrolled down and a message appears at the top of the page they are not really accessible.
This can however be addressed when we get Inline Form Errors in core (#2504847: [meta] Roadmap for stabilizing Inline Form Errors module (IFE)).
What we are addressing here is that we have a central place to put the messages with JS, and keep them there.
That is a small enough change, and can later be extended with things like toasts.
Let's do things in small enough steps so that we can at least go on.
The above also covers @droplet's concern in #91.

Ok trying to get this going again.

I was working on #2773601: Display "You are now in edit mode" prompt when user enters edit mode but hope we can get this done first so that issue can just use the core messages library.

I correct some eslint errors in messages.js. Also a couple logic errors.

Also had to update \Drupal\Core\Render\Element\StatusMessages::renderMessages because it was not showing the place holder if there were no messages.

I also add a test. But it is failing for me now. Not sure why.
If you enable the test module js_message_test you can see the show and remove message links. The module works for me but test doesn't. Not sure why

1. +++ b/core/misc/announce.js
   @@ -50,21 +50,23 @@
   +      for (var i = 0; i < il; i++) {
   

   announcements.reverse().forEach()

2. +++ b/core/misc/announce.js
   @@ -50,21 +50,23 @@
          }
   

   Set announcements back to an empty array at the end.

3. +++ b/core/misc/announce.js
   @@ -94,27 +101,23 @@
   +   * @param {String} text
   

   string

4. +++ b/core/misc/announce.js
   @@ -94,27 +101,23 @@
   +   * @param {String} priority
   

   string

5. +++ b/core/misc/message.js
   @@ -0,0 +1,106 @@
   +        if (!$('[data-drupal-messages]').length) {
   

   Capture this selector and store outside the behavior.

6. +++ b/core/misc/message.js
   @@ -0,0 +1,106 @@
   +      for (messagesIndex = 0, messagesLength = messages.length; messagesIndex < messagesLength; messagesIndex++) {
   

   array.forEach()

7. +++ b/core/misc/message.js
   @@ -0,0 +1,106 @@
   +   * @param {String} message
   

   string

8. +++ b/core/misc/message.js
   @@ -0,0 +1,106 @@
   +   * @param {String} type
   

   string

9. +++ b/core/misc/message.js
   @@ -0,0 +1,106 @@
   +   * @param {String} context
   

   string

10. +++ b/core/misc/message.js
    @@ -0,0 +1,106 @@
    +   * @param {Object} message
    

    Update JSDoc format for object/key descriptions.

11. +++ b/core/misc/message.js
    @@ -0,0 +1,106 @@
    +   * @return {String}
    

    string

@drpal thanks for the review!

1,2 I think are out of scope :(

6. had to use messages.reverse().forEach() to keep message order

fixed all the rest.

Also in Drupal.theme.message there is no need for the context parameter.

Got JsMessageTest passing it was missing a library dependency in the test module.

The other tests that are failing I think are because of the changes to \Drupal\Core\Render\Element\StatusMessages::renderMessages but I haven't had time to look at that yet.

1. +++ b/core/misc/announce.js
   @@ -94,27 +101,23 @@
   +   * @param {String} text
   

   string

2. +++ b/core/misc/announce.js
   @@ -94,27 +101,23 @@
   +   * @param {String} priority
   

   string

3. +++ b/core/misc/announce.js
   @@ -94,27 +101,23 @@
   +    if (typeof text === 'string') {
   

   In addition to checking that text is a string, should we throw an error if text is not passed it, or is not actually a string?

4. +++ b/core/misc/message.js
   @@ -0,0 +1,109 @@
   +      debouncedProcessMessages(messages);
   

   No need to pass messages.

5. +++ b/core/misc/message.js
   @@ -0,0 +1,109 @@
   +    $('.messages--' + type + '.js-messages-context--' + context).remove();
   

   It might be nice to have the ability to remove all messages of a given context regardless of type.

   Drupal.message.remove = function (context, type) {
     context = context || 'default';
     $((type ? '.messages--' + type : '') + '.js-messages-context--' + context).remove();
   };
   

Do want give the option to specify the element where the messages should go?

In #2785047: In Outside In mode, form validation messages should appear in the off-canvas tray, not the main page I would like to just the new messages library but it will not work because the messages div will not be on the tray.

1. +++ b/core/misc/message.js
   @@ -0,0 +1,109 @@
   +  Drupal.behaviors.drupalMessage = {
   +    attach: function () {
   +      if (!messagesElement) {
   +        var $messagesWrapper = $(messageWrapperSelector);
   +        if (!$messagesWrapper.length) {
   +          throw new Error(Drupal.t('There is no element with a [data-drupal-messages] attribute'));
   +        }
   +        messagesElement = $messagesWrapper[0];
   +      }
   +    }
   +  };
   

   Then we wouldn't want to do this on attach but when I message is added to check if container is available.

2. +++ b/core/misc/message.js
   @@ -0,0 +1,109 @@
   +  Drupal.message.add = function (message, type, context) {
   

   Here we could add containerSelector which would default to '[data-drupal-messages]'.

I think beyond the Settings Tray module any interaction in a dialog is going to run into this same problem. I think upcoming Media work will need this ability because the browser will be in a dialog.

@drupal thanks for review in #159

1-2 fixed
3. Won't if (typeof text === 'string') { not be true if it wasn't passed. I just added an else here and threw an error.
4. fixed
5. Yes I think this could be good. If a module wants to remove all messages it added but not other modules they could use context.
I added this and added test coverage.

Just test fixes.

BigPipe test will still fail. Looking into that.

+++ b/core/themes/classy/templates/misc/status-messages.html.twig
@@ -21,7 +21,7 @@
-{{ attach_library('classy/messages') }}

Why would classy/messages no longer be necessary?

+++ b/core/themes/classy/templates/misc/status-messages.html.twig
@@ -21,7 +21,7 @@
+<div data-drupal-messages>
 {% block messages %}
 {% for type, messages in message_list %}
   {%
@@ -54,3 +54,4 @@

@@ -54,3 +54,4 @@
   {% set attributes = attributes.removeClass(classes) %}
 {% endfor %}
 {% endblock messages %}
+</div>
 

This changed the markup for status messages. BigPipe's expectations therefore need to be updated as well.

This fixes the "no-js" BigPipe fail, and part of the other BigPipe fail.

+++ b/core/themes/classy/templates/misc/status-messages.html.twig
@@ -21,7 +21,7 @@
-{{ attach_library('classy/messages') }}

Why would classy/messages no longer be necessary?

Because this asset library is no longer being attached in the template, that's another BigPipe expectation we need to update: when BigPipe is using JS, it no longer needs to update the AJAX page state nor load the CSS file in this asset library.

I am posting this fix separately (from #165) because I'm not at all convinced that removing that asset library is correct. So then it's easy to revert this change.

Finally, if this change is correct, then you will need to expand BigPipe's test coverage with a case where it is explicitly loading some CSS or JS. Because this is effectively reducing BigPipe's test coverage.

1. +++ b/core/misc/message.js
   @@ -0,0 +1,110 @@
   +   * Builds a div element with the aria-live attribute and attaches it to the DOM.
   

   LIne exceeding 80 characters

2. +++ b/core/misc/message.js
   @@ -0,0 +1,110 @@
   +      // Save the text and priority into a closure variable. Multiple simultaneous
   

   Line exceeding 80 characters

Fixed coding standard error in the patch, please give me suggestion if i am wrong

@Pavan B S thanks for the patch but you have a bunch of unrelated files in there.

Re #164 I think this is mistake. I can confirm with the library the regular messages are not theme correctly.

Ok added back "classy/messages" library.
Update BigPipeTest

Ok so this implements the ability to send messages to different divs. See my idea in #160

This will be needed for the Settings Tray module.