Refactor Olivero's JavaScript Drupal behaviors to use once()

To do right after this one lands

once js is in core now :)

1. +++ b/core/themes/olivero/js/messages.es6.js
   @@ -114,8 +112,9 @@
   +        context.querySelectorAll('.messages'),
   

   you can simplify with once('olivero-messages', '.messages', context) :)

2. +++ b/core/themes/olivero/js/navigation.es6.js
   @@ -103,13 +103,13 @@
   +        context.getElementById(navWrapperId),
   

   same here: once('olivero-navigation', `#${navWrapperId}`, context)

3. +++ b/core/themes/olivero/js/tabs.es6.js
   @@ -32,9 +32,11 @@
   +        context.querySelectorAll('[data-drupal-nav-tabs]'),
   

   same

4. +++ b/core/themes/olivero/js/tabs.es6.js
   @@ -32,9 +32,11 @@
   +      tabs.forEach((el) => init(el));
   

   tabs.forEach(init) is sufficient

1. +++ b/core/themes/olivero/js/messages.es6.js
   @@ -114,13 +112,11 @@
   +      const messages = once('olivero-messages', '.messages', context);
    
          messages.forEach((message) => {
            closeMessage(message);
          });
   

   Here you can simplify as well since the messages const is not used later:

   once('olivero-messages', '.messages', context).forEach(closeMessage);
   

2. +++ b/core/themes/olivero/js/navigation.es6.js
   @@ -103,13 +103,14 @@
   +      const navWrapper = once(
   +        'olivero-navigation',
   +        `#${navWrapperId}`,
   +        context,
          );
   

   This one doesn't work right? Once always returns an array and because later you use navWrapper as a DOM element you can do either:

   // It's 'pretty' but babel adds a lot of code when using that.
   const [navWrapper] = once();
   // This works just as well and no extra babel code.
   const navWrapper = once().shift();
   

3. +++ b/core/themes/olivero/js/tabs.es6.js
   @@ -32,9 +32,8 @@
   +      const tabs = once('olivero-tabs', '[data-drupal-nav-tabs]', context);
   +      tabs.forEach(init);
   

   Here as well the tabs const is not used later so it can be a little shorter:

   once('olivero-tabs', '[data-drupal-nav-tabs]', context).forEach(init);
   

It seems like a bug to me

In what case the `Drupal.theme.message` is called but no `closeMessage`?
https://git.drupalcode.org/project/drupal/-/blob/9.2.x/core/themes/olive...

If we have a special custom function without the `closeMessage`, should we inject it?

#10 when a message is displayed by the backend (when saving a node for example) the message is on the page but Drupal.theme.message was not called to display it so we need the utility function (the naming is confusing though). Or did I miss something else?

Patch is good for me.

#10,

OK. What blocks us to do it on client-side only? (and must be client-side only?)
https://git.drupalcode.org/project/drupal/-/blob/9.2.x/core/themes/olive...

OK. So my best guess is the `click handler` and we don't want to repeat the code again on serverside.

EDIT:
and if I read the code correctly, #11 won't prevent an extra call on `closeMessage` (if we called it from clientside)

1. +++ b/core/themes/olivero/js/messages.es6.js
   @@ -3,7 +3,7 @@
   -((Drupal) => {
   
   @@ -28,8 +28,6 @@
        closeBtn.addEventListener('click', () => {
   
   +++ b/core/themes/olivero/js/messages.js
   @@ -63,10 +62,7 @@
   \ No newline at end of file
   ...
   \ No newline at end of file
   
   +++ b/core/themes/olivero/js/navigation.js
   @@ -98,4 +96,4 @@
   \ No newline at end of file
   ...
   \ No newline at end of file
   
   +++ b/core/themes/olivero/js/tabs.js
   @@ -37,9 +37,7 @@
   \ No newline at end of file
   ...
   \ No newline at end of file
   

   Uber nits

2. +++ b/core/themes/olivero/js/messages.js
   @@ -31,7 +30,7 @@
        var messageWrapper = document.createElement('div');
   -    messageWrapper.setAttribute('class', "messages-list__item messages messages--".concat(type, " messages-processed"));
   +    messageWrapper.setAttribute('class', "messages-list__item messages messages--".concat(type));
   

   EDIT: I didn't realize this was the generated code. Review item is moot.

   The diff doesn't give much for clarity, but is there a reason to have concat here anymore, if it's just `type`? Would

   `messages-list__item messages messages--${type}` be more appropriate?

for 2. the patch is using that in the es6.js file, it's the compiled code that uses concat, so it's all good on that front :)

d'oh! Thanks, @nod_. I tried to make sure I avoided reviewing the compiled code.

Yeah, sorry. My review is moot. The compiled JS doesn't have a new line and my second thing was also compiled CSS. Lookin' great to me!

Please do not ask the testbot to try again until #3207086: [HEAD BROKEN] Consistent failure in MonthDatePluginTest is fixed.

Not quite sure what's going on with patch #21.

As @benjifisher explained in #20 the RTBC patch #17 was failing due to an error in HEAD.

#3207086: [HEAD BROKEN] Consistent failure in MonthDatePluginTest was committed. Re-uploaded the original patch #17 so it will be retested on a 2 daily base, ordered retest and put this issue back to RTBC per #18.

@kuldeep_mehra27 Please provide an interdiff with updated patches.

Committed 4814a98 and pushed to 9.2.x. Thanks!