Problem/Motivation

Currently ajax.js places the following after the element that has triggered a loading action:
<div class="ajax-progress ajax-progress-throbber"><div class="throbber">&nbsp;</div></div>

This is terribly hard to modify or replace with your own loading animation, it's a UI pattern that is very dated and causes many layout issues as it changes the flow of the document.

Proposed resolution

Replace the element inserted with a class that is added to the element that triggers the loading action. The current behaviour can be replicated perfectly using a CSS pseudo element. Using a class instead increases the flexibility of the action in CSS without having to wade through any Drupal specific code. A button could change text or animate. I would also like to add a class to to the body of the page during loading, this would make full screen loading animations easier, a very common UI pattern.

Files: 
CommentFileSizeAuthor
#44 ajax-throbber-class-1490402-44.patch11.27 KBamen
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 77,738 pass(es). View
#37 ajax-throbber-class-1490402-37.patch11.24 KBandrejsmuzikovs
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch ajax-throbber-class-1490402-37.patch. Unable to apply patch. See the log in the details link for more information. View
#30 ajax-throbber-class-1490402-30.patch11.16 KBlauriii
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch ajax-throbber-class-1490402-30.patch. Unable to apply patch. See the log in the details link for more information. View
#22 throbber-views.jpg57.09 KBOuti
#20 Screen Shot 2014-03-01 at 15.29.01.jpg167.76 KBLewisNyman
#20 Screen Shot 2014-03-01 at 15.27.54.jpg159.77 KBLewisNyman
#20 ajax-throbber-class-1490402-20.patch11.04 KBLewisNyman
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 64,835 pass(es). View

Comments

LewisNyman’s picture

FileSize
1.92 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-ajax_throbber_class-1847916-1.patch. Unable to apply patch. See the log in the details link for more information. View

I've attached a patch which outlines my intentions

jwilson3’s picture

Totally agree with this. Theming / overriding the throbber style and location should be trivially easy to do.

Sometimes you want the throbber to reside inside the button itself that is clicked... while other times you may need to position the throbber elsewhere on the page, or display a full-screen overlay to block further input. The patch in #1 makes the first part possible; a follow-up patch that adds a body class (and possibly a companion empty top-level dom element?) would make the later stupid simple. Not to blow the scope completely here, but having an encapsulated way to simply override the throbber javascript in your theme without touching the entire ajax.js file would be clever as well.

jwilson3’s picture

Encapsulating the throbber js and css code to allow it to be overridable by a theme is a good candidate for the theme component library.

amen’s picture

Status: Active » Needs work
FileSize
1.55 KB
PASSED: [[SimpleTest]]: [MySQL] 58,448 pass(es). View

Here's a first shot at tackling this problem and come up with an usable, extensible way to deal with throbbers.

It replaces the html chunk that got appended to "throbbing" elements with a class named throbbing. I chose this name to make it consistent with what the Autocomplete widget uses.

There's no styling on it yet, but I guess it's a matter of coding css now if this approach gets pushed.

LewisNyman’s picture

Status: Needs work » Needs review

Go testbot

Status: Needs review » Needs work
Issue tags: -Needs themer review, -TX (Themer Experience), -Theme Component Library

The last submitted patch, ajax-throbber-class-1490402-4.patch, failed testing.

amen’s picture

Status: Needs work » Needs review
Issue tags: +Needs themer review, +TX (Themer Experience), +Theme Component Library

#4: ajax-throbber-class-1490402-4.patch queued for re-testing.

amen’s picture

FileSize
3.06 KB
PASSED: [[SimpleTest]]: [MySQL] 59,060 pass(es). View

Added CSS for the following elements:

  • a
  • .button
  • input[submit]

Also, untested RTL styling is available.

jwilson3’s picture

Issue summary: View changes

Added links to related issues.

LewisNyman’s picture

I've noticed that the edit module uses a class instead, we might be better off conforming everything to the same design for now.

LewisNyman’s picture

Issue summary: View changes
Issue tags: +JavaScript, +CSS
LewisNyman’s picture

Issue summary: View changes
Parent issue: » #1986434: New visual style for Seven
Related issues: +#2091939: Add the loading throbbers to the Seven style guide , +#2032773: Use Libricons (icon font) in Seven, consider using it more broadly in core

I did some more work on this patch and included the gif from the edit module, which is more inline with the ajax loader that ctools uses anyway.

There are some clear situations where this technique is more fragile and get's overridden by some special styling, like on the manage fields page. I think this disadvantage is part and parcel of the technique. The idea is to give themer more control over their loading state UI elements. It does mean we will have to go in and write specific styling against special UI elements that are now broken.

Opinions?

amen’s picture

In D7 and below, in order to change the styling on the throbber, you either have to hack the core or do some very weird css/jquery shennenigans. I know this because once I lost like 3 days trying to come up with the best solution for a client of mine who requested it.

So being able to be style the throbber just by overriding a single class is exactly why I would like this patch to be pushed. If you need me to work on some other patch for other elements tha don't work anymore please link the issues here and I will be happy to do it.

LewisNyman’s picture

FileSize
6.01 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch ajax-throbber-class-1490402-10.patch. Unable to apply patch. See the log in the details link for more information. View

Just realised I never posted the patch. How embarrassing! A review against existing use cases would be great.

Status: Needs review » Needs work

The last submitted patch, 13: ajax-throbber-class-1490402-10.patch, failed testing.

LewisNyman’s picture

Status: Needs work » Needs review
FileSize
5.69 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch ajax-throbber-class-1490402-15.patch. Unable to apply patch. See the log in the details link for more information. View

I should of seen that coming.

LewisNyman’s picture

Status: Needs review » Needs work

The last submitted patch, 15: ajax-throbber-class-1490402-15.patch, failed testing.

The last submitted patch, 15: ajax-throbber-class-1490402-15.patch, failed testing.

sun’s picture

The implementation was recently revised in #1069152: Throbber in textfield is misaligned when browser hardware acceleration enabled (gfx)

As a followup to that I suggested #2181399: Consider moving the animated .GIF throbber outside the autocomplete fields and using CSS animation instead. — which might be a duplicate of some of the related issues.

I'm not sure what the exact deal is here, but in general, "CSS class" sounds good ;)

LewisNyman’s picture

Status: Needs work » Needs review
FileSize
11.04 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 64,835 pass(es). View
159.77 KB
167.76 KB

I've rerolled this patch and gone over the use cases for the throbber in core. It looks pretty good!

droplet’s picture

  1. +++ b/core/modules/edit/css/edit.theme.css
    @@ -246,11 +246,24 @@
    +.edit-button.action-save.is-loading {
    

    the throbber.gif is very low quality. It looks very bad on blue background.

  2. +++ b/core/modules/system/css/system.module.css
    @@ -4,6 +4,52 @@
    +  background-image: url(../../../misc/throbber.gif);
    ...
    +  height: 18px;
    +  width: 19px;
    

    It's not a big problem but not sure if we possible to find an icon exactly square in size (18 x 18)

Outi’s picture

FileSize
57.09 KB

I tested this with the latest patch, and on the content type editing form the throbber looks good (as in the previous screenshots) but on the view editing form (and probably on the other dropbuttons) it's half outside of the button.

Concerning the blue background: it's true that the throbber.gif looks very bad on a dark background (to see it: create and save a node > click on the contextual link to quick edit it > make a modification > click Save), and for instance I haven't found a .gif throbber that would look good both on a white and on a colored background. (I don't know if it's possible. Should we use two different throbbers depending on the background?)

By the way, the edit module uses this throbber already (named icon-throbber.gif), so even without the patch the throbber looks bad in the quick edit form. Perhaps we should anyway unify the different throbbers used, as the Views_ui uses still another throbber, loading.gif and loading-small.gif, that has a dark background.

rteijeiro’s picture

Status: Needs review » Needs work

Let's try to get more feedback about what @Outi asked in previous comment.

Outi’s picture

Should the throbber question be resolved in another issue and use here the existing throbber?

LewisNyman’s picture

I think we should use the existing throbber, the edit module already looks bad, so we are not introducing anything new.

steven.grimaldo’s picture

LewisNyman, what admin theme is that in your screenshots?

LewisNyman’s picture

@steven.grimaldo That's Seven :D

LewisNyman’s picture

Bojhan’s picture

Lets change the icon in a separate issue, because it is a little bit tricky. The style guide does define one, so I think its best to follow that as a guidance.

lauriii’s picture

Status: Needs work » Needs review
FileSize
11.16 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch ajax-throbber-class-1490402-30.patch. Unable to apply patch. See the log in the details link for more information. View

Rerolled patch #20. Ready for more testing.

jwilson3’s picture

Issue for changing the icon here: #1974928: Update throbber icon

Status: Needs review » Needs work

The last submitted patch, 30: ajax-throbber-class-1490402-30.patch, failed testing.

andrejsmuzikovs’s picture

Issue tags: +frontendunited

Rerolling this soon!

andrejsmuzikovs’s picture

Assigned: Unassigned » andrejsmuzikovs
rteijeiro’s picture

Issue tags: -frontendunited +FUDK

It seems official tag is FUDK (kinda weird, BTW)

andrejsmuzikovs’s picture

FileSize
11.24 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch ajax-throbber-class-1490402-37.patch. Unable to apply patch. See the log in the details link for more information. View

Rerolled. Was only one conflict in core/modules/quickedit/css/quickedit.icons.css

andrejsmuzikovs’s picture

Status: Needs work » Needs review
joelpittet’s picture

+++ b/core/themes/seven/css/style.css
@@ -918,6 +918,19 @@ textarea.form-textarea {
+input.form-autocomplete.is-loading,
+input.form-text.is-loading,
+input.form-tel.is-loading,
+input.form-email.is-loading,
+input.form-url.is-loading,
+input.form-search.is-loading,
+input.form-number.is-loading,
+input.form-color.is-loading,
+input.form-file.is-loading,
+textarea.form-textarea.is-loading,
+select.form-select.is-loading {

Is there any input that this will not work on? Maybe a input:not() would be easier to maintain?

andrejsmuzikovs’s picture

Assigned: andrejsmuzikovs » Unassigned
droplet’s picture

+++ b/core/misc/ajax.js
@@ -461,11 +461,10 @@
     else if (this.progress.type === 'throbber') {

@@ -478,9 +477,7 @@
-    if (this.progress.element) {
-      $(this.progress.element).remove();
-    }
+    $(this.element).removeClass('is-loading');

Shouldn't add a condition to avoid the code execution on all ajax calls, eg:

if (this.progress.type === 'throbber')
...
..

Status: Needs review » Needs work

The last submitted patch, 37: ajax-throbber-class-1490402-37.patch, failed testing.

amen’s picture

FileSize
11.27 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 77,738 pass(es). View

Rerolled the Patch manually.

I tried to mimic the way the progress bar is set up, by adding a Throbber Class in progress.js and creating the instances in ajax.js.

I also consolidated the css from this patch with other css that has been added since this issue started.

I think the next step would be nice to replace throbber-active.gif with a css3 animated .svg, similar to what autocomplete has.

amen’s picture

Status: Needs work » Needs review
droplet’s picture

good idea but i think we should rewritten the function into Mixin pattern. (more re-usable & less duplicated code)

amen’s picture

Assigned: Unassigned » amen
Status: Needs review » Needs work
LewisNyman’s picture

@droplet A mixin? We don't have mixins available to us

+++ b/core/themes/seven/css/components/buttons.theme.css
@@ -54,6 +54,30 @@
+  background: url(../../../../misc/throbber-active.gif) no-repeat 95% center,
+  -webkit-linear-gradient(top, #f6f6f3, #e7e7df);
+  background: url(../../../../misc/throbber-active.gif) no-repeat 95% center,
+  -moz-linear-gradient(top, #f6f6f3, #e7e7df);
+  background: url(../../../../misc/throbber-active.gif) no-repeat 95% center,
+  -o-linear-gradient(top, #f6f6f3, #e7e7df);
+  background: url(../../../../misc/throbber-active.gif) no-repeat 95% center,
+  linear-gradient(to bottom, #f6f6f3, #e7e7df);

Looks like we don't need to provide gradient prefixes any more, so we can remove a lot of code here - http://caniuse.com/#feat=css-gradients

droplet’s picture

@LewisNyman,

Sorry, i'm not speaking clearly. It's programming side designs pattern. Not the CSS Preprocessor function. :)

jwilson3’s picture

So now there is a movement to introduce HiDPI-compliant SVG files across core, which presents problems with the purely class-based background image rotation technique.
Simply put, the only way to animate/rotate SVG elements is using CSS transitions (like how progress bar currently works). But with a purely class-based solution, we cannot get a rotating throbber when used as a background image inside autocomplete form elements. We really need a standard DOM based solution (much like progress bar) that adds the appropriate DOM elements after the item in question.

I think we should re-open #2181399: Consider moving the animated .GIF throbber outside the autocomplete fields and using CSS animation instead. and focus on getting one single standardized way to add this throbber to the page that fully supports the hi-res/scalable SVG image animation.

Finally, the ideas proposed in #2407859: Allow theming throbber element could solve a lot of the original issues with the throbber being completely inaccessible to themers. I think there might be some useful code on the last patch here that could combine forces with that issue to provide themable throbbers.

jwilson3’s picture

Issue tags: +HiDPI

Tagging because this is semi-related and affected by HiDPI (see prev comment).

jwilson3’s picture

Edit: remove double comment posted.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.