Add js message theme override to match Umami message markup

This looks really good. Just a few things to consider before we give it a final test/merge.

• In Drupal 10, we don't need to have the es6 code transpiled for ie11 any more, so that can be removed.
• Can we try remove the weight from the CSS file in the library definition. I don't think that should be needed.

Rerolled patch for 10.1.x.

Tried to fix test errors in #15.

As mentioned in #14.1, Drupal 10 doesn't need to have the es6 code transpiled, so we can just copy the contents from .es6.js into .js file from the D9 patch and remove the .es6.js.
#14.2 - not sure whats the ask.

Thanks!

Thank you for working on this @ankithashetty

Will still need screenshots for this though.

Hi @ankithashetty

Thanks for this work; it's looking really good.

For #14.2, that was my mistake. I thought we were introducing a 'weight' to the CSS file, but looks like we were removing it.

However, we now have a 'messages' library with no messages.css file in it. It seems to have been moved to the general umami global library. Can we add that CSS file back to the messages library instead please, so we keep the CSS and the JS for this component in the same library?

Also, as @smustgrave says, we'll need to get some before/after screenshots to verify this. Then we can move it to RTBC.

I made a slight modification of patch #16.
Message.css was added back to the library, adding an id to the h2 for the aria-labelledby attribute.
I also added a wrapper span around the message text to match the twig markup.
Before/after screenshots are attached.

Thanks just FYI for next time went screenshots are tagged they need to be added to the issue summary.

For the ID is there a possibility there could be 2 of the same on the page? That could introduce an accessibility issue.

I based this modification on claros's messages.js, but you are right, if two ids are the same it can cause accessibility issues.
So we can remove the aria-labelledby attribute since the h2 is visually-hidden but it contains the text needed for screen readers.
Added new patch.

Thanks! Change looks good and since this is a demo profile doubt it will need any kind of change record

Good job!

+++ b/core/profiles/demo_umami/themes/umami/umami.info.yml
@@ -7,7 +7,6 @@ libraries:
-  - umami/messages

If we remove the umami/messages library from the global libraries, it isn't guaranteed anymore that the message styles are loaded when messages are rendered. We probably should add the messages.css styles to the global library since it should be always loaded.

Added messages.css to global library.

I have added patch and interdiff for same. Please review

If we add to global don't think we need

+  core/drupal.message:

Anymore right?

Found one more bug. When Login with wrong password.

https://gyazo.com/6e5b6146b61a8d1d65249614dcb67f78

It not happends on

1. Olivero because it attaches library in template
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/themes/oliver...

2. Claro because it has direct dependency
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/themes/claro/...

Gonna add dependency aswell.

One more bug.
Seems messages_list wrapper missed. So new messages added directly inside existing message
https://gyazo.com/dff6c11b71c3fbe18661379e45e01b8f

Messages fixed + optimized

I also added an experimental branch with web components. I believe that since Umami is an experimental topic, we can easily try this technology here. Browser support suits us.

The benefits we are achieving now:
- Both message methods (Drupal render and javascript theme function) use the same template.

- Drupal has an example of using web components in its core and keeps up with the times

Please review!

The fact this seems to hit performance is it still a task to do?

This hits performance mostly because I've added lost drupal/message dependency

@finnsky this looks very interesting.

I wonder should we do it in 2 stages: get the original JS template MR merged, and then follow-up with a proposal to use Web Components?

---

Thanks to The Confident for sponsoring my time to work on this.

@markconroy yes, 2 stages ok.

Second MR here it is exact fix here.
Let's move Webcomponents into new issue and let community to discuss

smustgrave → changed the visibility of the branch 3100083-web-components to hidden.

Rebasing 8856 as it was 200+ commits back.

Hiding the other MR and opened 📌 Replace js message with webcomponents Active for more discussion

Ok applied the MR after the rebase and no issues/failures came up.

I created a few pages to get that status message "Article has been created" and it looks as it did before.

Don't see anything wrong in the MR, this still needs framework manager sign off but will put into that bucket.

small question in the MR

@nod
this is answer. thank you for review.
https://www.drupal.org/project/drupal/issues/3100083#comment-15692187 📌 Add js message theme override to match Umami message markup Needs review

nod_ → closed merge request !8856

thx.

Committed and pushed 0e07179f7bd to 11.x and 138d5889080 to 11.0.x and ae988430640 to 10.4.x and b89363b18d4 to 10.3.x. Thanks!

Automatically closed - issue fixed for 2 weeks with no activity.