Allow theming throbber element

This feels like a super good idea and would bring the ajax throbber code inline with how the progress bar is built which also uses Drupal.theme js functionality.

This needed a re-roll because ajax.js has moved inside core/ folder and the functionality has slightly changed.

This patch also adds the new SMACSS compatibility to the throbber, and improves themability of the "fullscreen" progress indicator, as well as the "message" text that optionally accompanies the throbber in some cases.

Ugh, forgot the patch.

Test fail seems to be due to unrelated issues around missing schemas. Not sure... maybe try to rerun tests again later.

A new patch against updated 8.0.x branch. It seems dialog.css is placed inside the theme (classy, seven, stable) and this patch changed all these css files for throbber element.

Very nice improvement!

+++ b/core/modules/system/css/components/ajax-progress.module.css
--- a/core/themes/classy/css/components/dialog.css
+++ b/core/themes/classy/css/components/dialog.css

+++ b/core/themes/seven/css/components/dialog.css
--- a/core/themes/stable/css/system/components/ajax-progress.module.css
+++ b/core/themes/stable/css/system/components/ajax-progress.module.css

I'm afraid that we are not allowed to make CSS or markup changes for stable or classy and we should keep these changes only for core, Seven and Bartik. What we could do is override the theme functions in Stable and then override them again in Seven and Bartik.

reroll

reroll for 8.4.x

RTBC patch 19

To @lauriii's point in #13, we shouldn't be changing markup. While I love the changes to make the markup more SMACSS-y (such as ajax-progress-fullscreen to ajax-progress--fullscreen), they will break sites so we can't allow them.

Hi all!
Modified last patch without css changes. So out of box all will be same. Plus themeable throbbers.

Some minor changes for the latest patch. Also removed on instance where throbber was being created manually in the Field UI.

@Laurii I have reviewed your patch and it's working fine for me my site doesn't break

1. +++ b/core/misc/ajax.es6.js
   @@ -811,6 +811,39 @@
   +   * @return
   

   Needs a return type, {string} I think?

2. +++ b/core/misc/ajax.es6.js
   @@ -811,6 +811,39 @@
   +  Drupal.theme.ajaxProgressThrobber = function(message) {
   

   This should be a named function or preferably an arrow function.

3. +++ b/core/misc/ajax.es6.js
   @@ -811,6 +811,39 @@
   +    return '<div class="ajax-progress ajax-progress-throbber"><div class="throbber">&nbsp;</div>' +
   +    Drupal.theme('ajaxProgressMessage', message) + '</div>';
   

   Template string.

4. +++ b/core/misc/ajax.es6.js
   @@ -811,6 +811,39 @@
   +  Drupal.theme.ajaxProgressIndicatorFullscreen = function() {
   

   Same comment here about function/arrow.

5. +++ b/core/misc/ajax.es6.js
   @@ -811,6 +811,39 @@
   +  Drupal.theme.ajaxProgressMessage = function(message) {
   

   Same comment here about function/arrow.

- cleanup jsdoc
- cleanup coding standards issues.

✌️

Sorry, I accidentally uploaded wrong patch on my comment #25. The patch in #25 is actually patch from #23. I tried to combine my changes with #29. Interdiff from #29.

Just some minor feedback ✌️

1. +++ b/core/misc/ajax.es6.js
   @@ -811,6 +811,40 @@
   +   * @param {string|null} message
   

   Here's the docs for marking something as optional, as well as documenting default values. http://usejsdoc.org/tags-param.html#optional-parameters-and-default-values

2. +++ b/core/misc/ajax.es6.js
   @@ -811,6 +811,40 @@
   +  Drupal.theme.ajaxProgressThrobber = (message = null) => {
   +    // Build markup without adding extra white space since it affects rendering.
   +    const messageMarkup = message !== null ? Drupal.theme('ajaxProgressMessage', message) : '';
   +    const throbber = '<div class="throbber">&nbsp;</div>';
   +
   +    return `<div class="ajax-progress ajax-progress-throbber">${throbber}${messageMarkup}</div>`;
   +  };
   

   Could this be simplified to to just an || operator, and return a multiline template string?

#31.1: Not really. That will add some whitespaces to the rendered markup which will affect how the throbber is rendered when message is visible.

I think what I was trying to say was something like, `<div class="ajax-progress ajax-progress-throbber"><div class="throbber">&nbsp;</div>${(message && Drupal.theme('ajaxProgressMessage', message)) || ''}</div>`

edit..

If you're interested in creating multi-line template strings, and your output is whitespace insensitive, you can use the \ to remove the new line characters that are inserted at transpile time.

Drupal.theme.ajaxProgressThrobber = message => `\
    <div class="ajax-progress ajax-progress-throbber">\
      <div class="throbber">&nbsp;</div>\
      ${(message && typeof message === 'string' && Drupal.theme('ajaxProgressMessage', message)) || ''}\
    </div>`;

Updated the issue credits

Alright. I think this looks good. 🎉👍

NM

Thank you! This has been sorely needed for years! RTBC+++

Someone has to say it: Needs Tests.

Is there a core theme that should use this feature? Found this #2775725: Update throbber icon.
or a test theme? Also, should this be documented somewhere?

I don't think it needs a CR because it isn't breaking anything, but was thinking the Theming Guide for the "
Drupal theming mechanism for JavaScript code." : www.drupal.org/docs/8/theming.

@lokapujya We do change records for API additions and new features as well! It is great way to educate developers of this types of features 🚀

The latest patch looks good for me. Thank you so much @vaplas for the amazing work on adding the test coverage!

I also created a change record for this.

This seems to have all the requisite sign-offs, and I pinged @lauriii for confirmation of this fact.

Committed and pushed to 8.x. Thanks!

Super excited to be able to swap this out. Now it will be much easier to make my client required dancing dog wait icon! Don't ask.

D'oh. git push is a whole thing. ;) Thanks, @vaplas!

@heddn: Pics or it didn't happen. :D

#57: amazing! 😂🐾