Problem/Motivation

MessageCommand was introduced in #3086096: Add a generic Ajax Message command. It is helpful to show status messages in AJAX contexts. Sadly it uses a totally different "render pipeline" than the active theme does. In fact it uses none, but builds the message area in JavaScript, without using the Theme's twig templates. That's how it's implemented.

From a Themers and Theme Prespective and from "Component" thinking, this is wrong. AJAX messages should use the same templates, as regular messages do.
That's why I'd rate this as bug from outer perspective. Still I think it was a (discussable) design decision.

This results in issues like this: #3165452: Override Drupal.theme.message to to make sure JS messages get rendered correctly

Of course, I know talk is cheap and this is hard to resolve, because the messages are added dynamically and you can't tell if the message area already exists. But these problems can be solved.

Implementation:
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/misc/message....
https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Ajax%21Me...

The devel maintainer also points out that currently
dsm() / dpm() are broken and could be fixed in this issue: https://gitlab.com/drupalspoons/devel/-/issues/525#note_2039843855

Steps to reproduce

Show a regular status message using PHP in a custom theme, where the message area was modified in twig
Compare the result to a status message added by JavaScript. They differ a lot, if the JS-built dom structure isn't the same as the one in Twig.

Proposed resolution

Return the theme rendered message wrapper and the messages in the AJAX result. Only add the wrapper on the page, if none is present.
After adding or if the message wrapper is already existing, add the rendered messages.

This way the theme handles the message output, like it would do on a regular call, but AJAX builds the message from the theme results without a difference.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3396318

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

Anybody created an issue. See original summary.

anybody’s picture

anybody’s picture

catch’s picture

luke.leber’s picture

Injecting here that this would make styling file upload errors quite a bit easier!

It'd pay to take a hard look at the file module to determine how feasible the approach in the IS actually is.

catch’s picture

I took a look at how this could work.

The main thing we want, is to use #theme 'status_messages' to render the messages in PHP, before they're sent to JavaScript.

I think something like this would probably do it:

1. Add a new RenderedMessagesCommand - it would take an array of messages instead of a single one.

2. messages.js would need to support a new 'rendered' option (in the options array) where it just appends the markup string it was given, instead of passing it through Drupal.theme.message

3. RenderedMessagesCommand would take the messages array, pass it through '#theme' 'status_messages', add the 'rendered' option, and return otherwise the same structure as MessagesCommand.

4. To solve #3456176: 10.3 upgrade now missing status-message theme suggestions, we'd then use RenderedMessagesCommand in big pipe instead of MessageCommand.

It doesn't allow us to remove Drupal.theme.message though, because we provide a pure JavaScript API to add messages, and those can't be rendered in PHP, so the underlying issue that themes need to theme JavaScript messages separately is still there. That makes me wonder how useful it really is compared to opening issues against themes that don't implement message styling. e.g. I just found #3100083: Add js message theme override to match Umami message markup.

luke.leber’s picture

We've started to use drupalSettings + js_settings_alter to provide server-side rendered templates to the frontend.

Our Drupal.theme.messages implementation pulls the markup from drupalSettings. Might not be robust enough for core, but has worked rather well for us...and even covers the obscure file.module validation cruft too.

umit’s picture

commented on the wrong topic

catch’s picture

@luke.leber #3456176: 10.3 upgrade now missing status-message theme suggestions shows pretty conclusively that the current situation with separate js and twig theming isn't robust, so I think it's a good idea to try this - could you extract your logic into an MR? Seems OK for bc too given we'd be changing the default behaviour and js theme overrides should still work the same way.

catch’s picture

Status: Active » Needs work

daniel korte made their first commit to this issue’s fork.

tomefa’s picture

I get the status message problem from https://www.drupal.org/project/drupal/issues/3396318 and just try this MR.
It doesn't fix the issue, it still using block--system-messages-block.html.twig

bkosborne’s picture

#6 would solve this for rendering messages that are invoked from the backend, but not solve the problem where purely front-end rendered messages still use a different template/rendering solution. The MR solves this. It's a bit weird to use drupalSettings for storing the template. feels like there should be a better way to deliver the template down.

luke.leber’s picture

Agree with #14. MR !9329 is very close to what we use in our custom theme, but I'm not convinced it's core-worthy. It's pretty messy and would be hard to predict / guarantee b/c for folks that do strange things to their messages templates.

There would have to be at minimum additional API set up to dynamically fetch the supported status headings in order to even offer full backwards compatibility support.

catch’s picture

Oh I didn't realise I hadn't commented properly here since #6. The MR is great and much, much better than #6 - I posted that comment before even knowing the MR solution was a possibility. Looks like a couple of real test failures though. using drupalSettings does feel weird but I don't have a better idea. It would be good to get the MR green so that this is easier to review though, even if it's not committable with those limitations. But also I'm not sure the limitations here are worse than the current bifurcated theming of status messages?

catch’s picture

nod_’s picture

Comming from #3476471: Replace js and twig messages with webcomponent Umami-message and in that issue it does esentially the same thing through a <template> tag added to the DOM somewhere (if we ignore the whole webcomponent/ShadowDOM part).

Instead of putting that in drupalsettings, I'd rather have a <template> dom node on the page to query when needed

bkosborne’s picture

Ohhh yes, a template element seems most appropriate here.

matthieuscarset’s picture

Following the issue because I was actually implementing a custom template for Drupal messages. My use case is to use a single-directory component for message items. I naturally use the same logic as explained at the beginning and in the current MR: to pass the template to JS through the drupalSettings.

Unless I'm wrong, this is how we do things in Drupal for years. Let JS knows about things from the back via drupalSettings 🤷

Using a WebComponent (e.g. template) sounds like a good idea but It seems a bit too much to me to implement something different that what we're used to. If we go this way, doesn't it retire the whole Drupal JS API / drupalSettings in some ways? Does it means that we should use WebComponent for everything that can be rendered both in backend and frontend basically?

I +1 to pass the template with a placeholder through drupalSettings.

anybody’s picture

Issue summary: View changes

Just added information to the issue summary that this also supposed to fix Devels current dsm() incompatibility with BigPipe.
Currently kints output in messages can't be expanded and is useless because of this. https://gitlab.com/drupalspoons/devel/-/issues/525#note_2039843855
which is reported to be a regression from #3379885: Use MessagesCommand in BigPipe to remove special casing of the messages placeholder

catch’s picture

template element sounds good here.

If we go this way, doesn't it retire the whole Drupal JS API / drupalSettings in some ways?

A lot of things in drupalSettings aren't rendered, but also drupalSettings has a lot of problems - it's a big array blog in the header of the page and the output isn't cacheable. If we can put some things into discreet elements instead that will be an improvement.

wouter.dierick@gmail.com’s picture

I implemented this for a custom project:

- add your messages html or render ui_patterns/sdc component on every page inside a <template> tag
- extend the core/drupal.message library with my own library
- add new library and JS with custom Drupal.theme.message implementation (start from web/core/misc/message.js )
- The JS will copy html from the <template> tag, insert type, text inside the html and render it in the <div data-drupal-messages> element.

Works good, but by overriding the core js this is quite sensitive for core updates...

vensires’s picture

Issue tags: +GreeceWinterSprint2024
andypost’s picture

finnsky’s picture

https://www.drupal.org/project/drupal/issues/3476471
Merged now. So it can be used as testing area for `template` and Shadow Dom approach

carlitus’s picture

With this MR it's working fine for this: https://www.drupal.org/project/drupal/issues/3456176, with Drupal 10.4

Sorry, I looked at it wrong, it doesn't work

prudloff made their first commit to this issue’s fork.

mohammedodeh’s picture

StatusFileSize
new3.25 KB

Reroll the patch to version 10.4.7

prudloff’s picture

Status: Needs work » Needs review

I cleaned up and fixed the tests in the MR for the drupalSettings approach.
I had to change multiple selectors in tests because now tests using the stark theme don't have access to the messages--[type] class anymore (this class was always added by the JS but is not in the base status-messages.html.twig template.

If we decide to use the web component approach instead we should probably start a new MR.

fathershawn’s picture

We are well on the way to changing BigPipe's placeholder insertion process in #3526267: Remove core/drupal.ajax dependency from big_pipe/big_pipe. A fundamental difference is that HTMX expects HTML. Which means that we neither need nor want to assemble HTML in javascript for messages. When I commented to @catch that what we really needed was a theme hook for an individual message he sent me here :)

In the refactored BigPipe approach I'm sending the messages, that are generated by rendering a placeholder, as HTML wrapped in a <template>. At the moment I still need javascript to find the wrapper element for the messages so they can be moved into place. I think that this work could be simplified down to storing a CSS selector in drupalSettings. Here's my proposal:

  1. Core stores the CSS selector for the default wrapper element in settings.
  2. Themes can store a different selector in their theme.info.yml file.
  3. If the active theme has an override in replaces the default.
  4. We use status-message.html.twig to theme individual messages and #theme => 'status_message' when constructing a render array.
  5. Inserting the resulting html dynamically into the designated wrapper becomes very simple
fathershawn’s picture

This problem comes from the Ajax API which we are actively working to replace in #3404409: [Plan] Gradually replace Drupal's AJAX system with htmx. We decided to further decompose our refactoring of BigPipe, moving the PHP side to #3533769: Refactor big_pipe module to remove dependencies on Ajax classes. I would still like feedback on the proposal #32 as we will need to render individual messages so that we can return HTML placeholders for messages when it is time to refactor.

smustgrave’s picture

Concern here is this could be a breaking change for some. We can see all the tests that had to be updated to get green.

fathershawn’s picture

I've been thinking about the power and flexibility HTMX gives through using CSS selectors and looking again at this MR. I think we can adapt what's here because we can use a selector to get all the children of a message type wrapper or all the children of the outer message wrapper.

We don't have to target the existing Ajax API, so we don't have to fix any tests that are using the current Ajax API.

Option 1: append new messages as new children of the element with `data-drupal-messages`. This is the simplest but the markup is not as nice. Each batch of messages would repeat the message type wrappers. But nothing needs to exist on the page except the element with `data-drupal-messages`.

Option 2: add a data attribute to the message type wrapper element, such as `data-drupal-message-type="error"`. This would allow us to select messages of each type and insert them into the existing wrappers. The downside is the wrapper elements need to exist even if the are empty.

You all have been thinking about this longer though. So here is the core question: If you could use CSS to select the messages from a new HTML response and then use CSS to select the element to recieve those messages, how would you all set up the markup?

smustgrave’s picture

Just wanted to follow up if anyone has a response to @fathershawn above?

needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new90 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

tim-diels made their first commit to this issue’s fork.

tim-diels’s picture

Status: Needs work » Needs review

I rebased, hope I didn't break anything.

tim-diels’s picture

Status: Needs review » Needs work

I broke the tests :sad:

prudloff’s picture

Status: Needs work » Needs review

I fixed the failing test.

smustgrave’s picture

Status: Needs review » Needs work

Surprised the bot never picked this up but appears to need a manual rebase.

tom konda made their first commit to this issue’s fork.

tom konda’s picture

Status: Needs work » Needs review

I rebased MR !9329.

tom konda’s picture

Status: Needs review » Needs work

Need to fix OpenTelemetryAuthenticatedPerformanceTest.php because of commit bd24c65d.

tom konda’s picture

Status: Needs work » Needs review

Fixed OpenTelemetryAuthenticatedPerformanceTest.php.

smustgrave’s picture

Status: Needs review » Needs work

Appears to be 1 thread open and seems valid, should be easy enough and don't think needs to expand test coverage.

finnsky’s picture

Hi everyone!

I've added a backport from
https://www.drupal.org/project/drupal/issues/3476471

Currently, message is a simple web component that uses the same template on both the backend and frontend!

---

- ​​Claro and core use web components.
- Olivero works just as before!!
- The CSS needs some tweaking for Claro. But just a little bit!
- The API and other stuff haven't changed.

finnsky’s picture

Status: Needs work » Needs review
luke.leber’s picture

Question - does using a web component here contribute to CLS or negatively impact LCP? It seems that adding JavaScript as a blocker for final paint will have to affect one or the other negatively given we can't effectively inline JS resources in core yet.

finnsky’s picture

We talked about Declarative Shadow DOM somewhere.
https://web.dev/articles/declarative-shadow-dom

But this method also has its limitations.

I haven't noticed any freezes in either the previous or this MR.

catch’s picture

This will only be used to render messages that are set via an AJAX response so unless I'm missing something with the implementation,it should not affect LCP or CLS any more than it already does. There's already a js dependency for those messages and it already rewrites the mark-up in the messages area.

If we wanted to reduce CLS we'd need to render messages in some kind of off canvas or overlay but that would be a different issue needing design work.

One question about the MR - shouldn't we be able to remove Claro's messages.js entirely? That should exist only to override messages added in the way being replaced here.

finnsky’s picture

it should not affect LCP or CLS any more than it already does

Now both messages appear through one template
https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/tem...
Both are rendered via JavaScript.
We can think about how to minimize latency with some failback techniques.

shouldn't we be able to remove Claro's messages.js entirely?

Good point. Yes, this allows us to unify everything a lot.
And I think that it is possible to remove Claro messages.js and/or Twig
And maybe even Olivero messages.js and/or Twig

catch’s picture

Now both messages appear through one template
https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/tem...
Both are rendered via JavaScript.

The MR isn't attaching core/drupal.messages anywhere it's not already added, so how are the messages actually getting rendered then when the library isn't otherwise on the page?

It would probably have to be added by StatusMessages::renderMessages() only when there are messages to render, but then indeed that is likely to result in more CLS than it currently does per @lleber's point. If we're able to continue rendering messages via PHP in that situation it would be better, although it might mean providing template and non-template versions of the markup then?

fathershawn’s picture

I'm going to try to find time to build out idea 2 in #35 above in the hope that some code will generate discussion.

With HTMX in core I really do think this problem gets much simpler. If we provide a the proper templates, message theming can be overridden like anything else, and will be properly themed when returned in a response.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

finnsky’s picture

The MR isn't attaching core/drupal.messages anywhere it's not already added, so how are the messages actually getting rendered then when the library isn't otherwise on the page?

Yes, perhaps. I didn't think about it.

---

LCP or CLS

I'm thinking that we could perhaps leave the message backend as is but use Twig include in this web component. This way, the goal of getting the same template would be achieved without affecting LCP or CLS.

m4olivei’s picture

Commented on @finnsky 's MR. To reiterate, might be useful to keep the web component approach in mind as @fathershawn explores what this looks like in a world with HTMX at play.

In any case, super interesting to watch this. Would make for a great pattern to follow for how to unify templating between PHP and Javascript.

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs change record

Won't this need to target the new default_admin theme now.

100% will need change record if we touching those templates.

fathershawn’s picture

jcandan’s picture

#3456176: 10.3 upgrade now missing status-message theme suggestions points to here as the appropriate fix, but I want to note that this MR doesn't yet address a message template issue as noted in #3465493-22: Drupal 10.3 changes how to theme status messages..

fskreuz made their first commit to this issue’s fork.

fathershawn’s picture

@jcandan I'm waiting for some work to land to finish but see #3546787: Replace MessageCommand with an HTMX implementation

fathershawn’s picture

#3546787: Replace MessageCommand with an HTMX implementation Is ready for a first review and includes a status-message-item.html.twig template file.