Problem/Motivation

The default ajax progress throbber icon looks poor on a retina display.

Proposed resolution

  • Use throbber with a minimal design from the Ajax system. We propose to use the same throbber from jQuery UI that is used on modals.
  • Override throbber for the original gif in Stable since we don't want to affect existing themes.
  • Override throbber again in Bartik with a design that is reflected by Bartik
  • Ensure that overriding throbbers is as easy as possible and provide instructions for how themers can use libraries-override.

Before inative/active:
GIF ajax progress throbber inactive
GIF ajax progress throbber active

After:

Remaining tasks

  1. Propose a throbber style for Bartik.
  2. Create a patch. with the new SVG based on previous point and animate it, using the old throbber in Stable.
  3. Create a patch.
  4. Add comments in Bartik to be sure it is an easy example to follow.

User interface changes

API changes

CommentFileSizeAuthor
#121 throbber-inactive.png356 bytesckrina
#121 throbber-active.gif1.25 KBckrina
#113 interdiff.txt569 bytesManjit.Singh
#113 update_throber_icon-1974928-113.patch7.15 KBManjit.Singh
#103 update_throber_icon-1974928-103.patch7.16 KBckrina
#93 update_throbber_icon-1974928-93.patch12.53 KBgauravjeet
#91 Screen Shot 2016-02-27 at 12.51.15 pm.png15.39 KBsoumyajit.basu
#91 Screen Shot 2016-02-27 at 12.48.39 pm.png15.55 KBsoumyajit.basu
#79 interdiff-1974928-68-79.txt803 byteschrisfree
#79 update_throbber_icon-1974928-79.patch12.53 KBchrisfree
#68 interdiff-1974928-63-67.txt5.87 KBchrisfree
#68 update_throbber_icon-1974928-67.patch13.54 KBchrisfree
convert-to-tabs-do-not-test.patch1.59 KBBojhan
#63 svg-trobber-1974928-63.patch8.16 KBcorbacho
#62 throbber-redesign-1974928_1.png6.78 KBjwilson3
#61 throbber-test-1974928.gif436.33 KBjwilson3
#61 drupal-svg-throbber-1974928-61.patch8.16 KBjwilson3
#58 remove-throbber-1974928-57.png54.99 KBjwilson3
#58 upload-throbber-1974928-57.png36.29 KBjwilson3
#58 dropbutton-throbber-collapsed-1974928-57.png14.53 KBjwilson3
#58 dropbutton-throbber-expanded-1974928-57.png29.14 KBjwilson3
#58 interdiff-54-57.txt4.81 KBjwilson3
#58 drupal-svg-throbber-1974928-57.patch7.4 KBjwilson3
#54 drupal-svg-throbber-1974928-54.patch4.13 KBjwilson3
#54 throbber-active-1974928.gif230 bytesjwilson3
#54 throbber-redesign-1974928.png202 bytesjwilson3
#47 update_throbber_icon-1974928-45.patch844 bytesellizard
#45 loading-svg-icon.zip539 bytesellizard
#45 loading-gif-icon.gif46.67 KBellizard
#45 update_throbber_icon-1974928-45.patch1.05 KBellizard
#32 loading-small.png1.55 KBckrina
#32 retina_ready_throbber-1974928-31.png15.86 KBckrina
#32 retina_ready_throbber-1974928-31-do-not-test.patch3.84 KBckrina
#23 IE9-retina_ready_throbber-1974928-23.png108.84 KBckrina
#20 retina_ready_throbber-1974928-20.png16.92 KBckrina
#20 retina_ready_throbber-1974928-20.patch3.76 KBckrina
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Snugug’s picture

Can we replace it w/an CSS3 animated throbber? That'll look good everywhere.

Jon Pugh’s picture

+1

You don't need a "retina" display to know this is true!

rupl’s picture

Don't forget about SVG!

Jon Pugh’s picture

print theme('throbber');

Oops! That won't work... forgot we were talking about Drupal 8!

;)

LewisNyman’s picture

Cross posting #2091939: Add the loading throbbers to the Seven style guide

I'm really keen on #1847916: Replace the ajax-progress-throbber div with a class but we wouldn't be able to animate pseudo elements with CSS. It would be great to throw some ideas/inspiration around.

jwilson3’s picture

It seems to me that the reality is that the most versatile solution is to provide two animated gifs as a css background image, one twice the size of the other targeted to browsers and devices that need higher res versions using a combination of media-queries and background-size.

On #2032773: Use Libricons (icon font) in Seven, consider using it more broadly in core, someone throws out that they should add a throbber to the icon font being worked on by ry4n, but from what I can tell, getting the placement right is extremely challenging to do (but this shouldn't stop us if it is in fact possible). Has anyone really seen a perfectly placed throbber glyph in an icon font that rotates with zero noticeable axis alignment issues, even at a large font size of say 100px?

jwilson3’s picture

Issue summary: View changes

Updated issue summary.

LewisNyman’s picture

Issue summary: View changes
Issue tags: +CSS, +frontend, +retina
jwilson3’s picture

As @rupl mentioned in #3, SVG might be the better solution. and according to http://caniuse.com/svg, everything that Drupal core supports is also supporting SVG.

Something like this SVG throbber used by Google Chrome:

http://plnkr.co/8J9s84ZH24OodIx75L5k

jwilson3’s picture

Title: The default throbber icon looks really bad on a retina » Update throbber icon to use SVG
Category: Bug report » Task
Issue summary: View changes

I've updated issue summary. At this point what needs to happen next is some cross browser testing of the <animateTransform> tag in an SVG, to ensure we don't have any breakage across an array of the supported os-platforms/browser-versions/devices.

joelpittet’s picture

Status: Active » Needs work

@jwilson3 I did a BrowserStack test on that link above and it doesn't animate on IE9-11 on Windows 7 unfortunately.

Also here's reference saying it's not supported from the source:
@see http://msdn.microsoft.com/en-us/library/ie/gg193979%28v=vs.85%29.aspx#ex...
And
@see http://caniuse.com/#feat=svg-smil

Maybe there is a nice CSS3 rotate that can work well or in the link above they have a decent requestAnimation frame javascript to rotate the svg, is that an option? You may need a IE9 polyfill for these:
@see http://caniuse.com/#search=animation

jwilson3’s picture

I ran the requestAnimation frame javascript example (http://samples.msdn.microsoft.com/workshop/samples/svg/svgAnimation/basi...) through BrowserStack, and it failed to animate on IE9.

On the other hand, as long as we're talking polyfills, there is http://leunen.me/fakesmile/ which did work on IE9 -- but was seriously slow to load.

I feel like we should be courteous to the non-compliant browsers and give them a gif fallback, since CSS animation itself doesn't work in IE9 or earlier.

joelpittet’s picture

@jwilson3 I'm not sure who to talk to on getting another polyfill in there but I'm game for that. Since SMIL is in the SVG maybe we can do a gif fallback instead of the polyfill, do we have any .lt-ie10 style html classes to target that animated gif?

Here is a quick little snippet for SASS and svg I put together for a D7 site if anybody want's to play:)
https://gist.github.com/joelpittet/c75ff7c2f86282ff9499

jwilson3’s picture

Yeah, I'm sticking with my feeling that a polyfill is the wrong solution in this case.

As for an lt-ie10 class, we don't have that, but do have the conditional functionality built into drupal_add_css. SMIL isnt supported in IE 11 either, so this would have to be lte-ie11.

drupal_add_css('/core/misc/throbber.fallback.css',
  array(
    'browsers' => array(
      'IE' => 'lte IE 11',
      '!IE' => FALSE
    )
  )
);

The only other browser that has problems with SMIL is Opera Mini. :'( I haven't thought of a proper solution that will work in that case yet.

Tangentially, I took your gist and ran with it to make a couple small improvements: https://gist.github.com/jameswilson/d40d303c9cf24051d332

jwilson3’s picture

I think the only solution really is to use Modernizr tests for smil and svg, which would address both opera mini and IE support.

http://modernizr.com/download/#-smil-svg

.is-throbbing {
  background-image: image-url('svg/throbber.gif');
}

html.svg.smil .is-throbbing,
html.svg.smil .is-throbbing {
   background-image: image-url('svg/throbber.svg');
}

^ this is a progressive enhancement to SVG, instead of fallback to GIF, we could do it the other way around too. Not sure which is better and the final solution depends on minimizing the effect of modern browsers downloading *both* assets as opposed to just the one that it needs (ie, the tax on double asset download should, in this case go to non-modern browsers).

Currently, the version of modernizer in Drupal core does not test for smil or svg, so we'd need to add that, however I don't think this is a bad idea. Given the direction svg is going right now as the defacto solution to high-quality web graphics for retina display and zoom, many themers will want to have that test around anyway, and having it there by default could be great.

./assets/vendor/modernizr/modernizr.js:2: * Build: http://modernizr.com/download/#-inputtypes-touch-cssclasses-addtest-test...

EDIT: updated classnames to reflect comment below.

LewisNyman’s picture

But it's not SVG support that's the problem is it? It's a specific feature within SVG? We can't just use CSS animations here?

joelpittet’s picture

The class name I think would be html.svg.smil .is-throbbing or just html.smil .is-throbbing would likely do the trick, no?

http://modernizr.com/docs/#smil

jwilson3’s picture

Yes, sorry for the confusion... i meant "smil" instead of "animate" for the classname on the html element. I edited the example code in comment #14 to reflect that.

html.smil .is-throbbing would likely do the trick, no?

Yes it would, I thought html.svg.smil is clearer.

jwilson3’s picture

But it's not SVG support that's the problem is it? It's a specific feature within SVG?

Correct.

We can't just use CSS animations here?

Yes, good point this could be possible. Here is a little investigation on CSS animation vs SIML:

  • CSS animations aren't supported in IE9 and Opera Mini: http://caniuse.com/#feat=css-animation, and so we'd still need a modernizr-based GIF fallback for older browsers.
  • iOS 6.1 and below do not support CSS-animation on pseudo-elements. Core doesn't use pseudo-elements for animations anyway, so this is just anecdotal information.
  • IE has sided with CSS-animations, but all the other browsers support both SIML and CSS animations.
  • All webkit browsers (Chrome, Safari, Opera, Android) require -webkit-animation vendor prefixes for CSS animations, meaning duplicate code (which could get ugly for non-trivial animations with multiple keyframes). OTOH, a simple rotation throbber in CSS-animation wouldn't need *too much* extra code.
  • In general, SIML animation is a more encapsulated and contained solution. Like a GIF, the animation logic is contained inside the image asset itself. So, to override the core throbber to do something other than the default clockwise rotation you'll only be required to change the background-image in CSS. The consistency and loose coupling promised here is really the primary selling point from a maintenance perspective. This also means we could still use css a single image file both the not-throbbing version AND the throbbing version and use background position to pull the animation into view, like how core's GIF does right now on auto-complete fields, thus incurring no extra HTTP requests through this issue.
  • Finally, there is the point about familiarity. SVG animation syntax is newer and thus more foreign to most devs, which is not to say that it is any more *complex* than CSS animations -- just different. However, in 2014, CSS animation syntax is simply more common; there are plenty of examples online to learn from. This could mean lost time coming up to speed on *how* to animate things in SIML, but on the same note, themers will be free to override a would-be core SVG throbber using their own CSS-based animation, and give up on SVG animation if they want to.

There are some good write-ups comparing the two solutions, from a more technical perspective here and here for anyone interested.

ckrina’s picture

Assigned: Unassigned » ckrina

Working on that.

ckrina’s picture

Here is a first approach to that.
We were talking about adding the svg files as background images and just rotate them with css, instead of using SIML animations. The problem on that is that I think there is no way to rotate only the background image and not the element without using pseudo elements.
So I’ve uploaded a patch with a SIML animation in the active throbber, and at least we have the svg files here so can discuss that.
That is how it looks:

Another thing that needs work is IE9 back port.
Also, just to point that the inactive throbber is using the #949494 because of accessibility reasons.

ckrina’s picture

Assigned: ckrina » Unassigned
Status: Needs work » Needs review

Unassigning.

ckrina’s picture

I've also tried to use :before or :after elements to move the throbber image with css animations, but inputs don’t work with them. I didn’t know, but those pseudo elements are only allowed to be used on a container element. You can read about that here. (But there’s anything on the W3C recommendations).

So, maybe the best option is using animation inside the svg files. And for those browsers that don’t support it we’ll still have the color change on the active state. Here is an screenshot for IE9, that doesn’t support it.

ckrina’s picture

Here is the screenshot.

droplet’s picture

Remember that CSS animations doesn't working in IE9 (keyframe..etc). The only way I think is apply JS to rotate the SVG BG image.

And the SVG could be sprite image. (in the past, we splited GIF into two files for performance problem)

LewisNyman’s picture

Title: Update throbber icon to use SVG » Update throbber icon and use SVG
Status: Needs review » Needs work
Issue tags: +Usability, +styleguide

I had a look over this with Bojhan and it looks good, but the size of the icon is a little too big for the text field. I think we need to add this new loading icon to other areas of the site now.

LewisNyman’s picture

It looks like loading-small.gif is referenced in:

  • core/misc/dialog.theme.css
  • core/modules/system/css/system.module.css
  • core/themes/seven/css/components/dialog.theme.css

We should be able to remove loading.gif and loading-small.gif in this issue

ckrina’s picture

Assigned: Unassigned » ckrina

Ok, then I'll work on that.

joelpittet’s picture

Won't give up yet but SMIL has no support in any version of IE:( There has got to be a work around here.

  • SVG background on the input field seems to be the cleanest approach. Input elements don't take :after (AFAIK) so rotating pseudo elements could get messy if they have to be moved to the container for positioning.
  • SMIL would be nice but support is lacking a bit too much on all IE versions and the polyfill is slow as mentioned earlier.

If we find a solution where IE9 just gets a color change without the animation but IE10 and up work, I think most of us would be happy with that solution it seems when talking to others at the conference. Speak up though if that's unacceptable for a good reason.

Maybe we can embed CSS animations in the SVG to do the animation which seems to be what IE was betting on in their short sighted decision to not support SMIL.

Going to play around, just thought I'd share my thoughts. Vectors for the scalable win!

joelpittet’s picture

Haven't cross browser tested this yet, but that idea:
http://jsfiddle.net/0ms01h7b/

Maybe @ckrina if you have access to IE browsers or know someone who does you could give it a try?

ckrina’s picture

Bad news again... There are some troubles with Firefox. If you intert the svg in the HTML it works great, but adding it as a backround image with this styles inside doesn't seem to work: is doesn't rotate.
And it's the same in IE9 and IE10: the throbber doesn't rotate as a backround image.

joelpittet’s picture

Dang, well thanks for testing that @ckrina.

Hmm may need to come back to this.

Anybody feel free to share some code, something is bound to surface:)

ckrina’s picture

Here is a patch with smaller icon, as @LewisNyman suggested on #25. Maybe it's not the final solution but at least we have the svg files uploaded. Here is the screenshot:

@LewisNyman: When you said that we should be able to remove loading.gif and loading-small.gif in this issue, did you mean changing the design that we are using now by this svg or moving to svg the current design? The current design is this one:

brunodbo’s picture

I played around with animating the throbber in JavaScript, based on http://msdn.microsoft.com/en-us/library/ie/gg193979%28v=vs.85%29.aspx. Code needs work, but I thought I'd post it in case other people want to further explore this route.

http://jsfiddle.net/1qgp8xzu/

LewisNyman’s picture

Sorry I haven't been too active recently, I think we can get away with changing the design because it's almost required to achieve what we want.

axe312’s picture

Issue tags: -retina +HiDPI

HiDPI is the official name for screens with a higher resolution per inch. Retina is just a word created by Apple for this. Thats why I changed the tag.

jwilson3’s picture

.

jwilson3’s picture

Wim Leers’s picture

Issue tags: +Novice, +CSS novice
joelpittet’s picture

So rotating the SVG as a background is a show stopper for IE as they don't support SVG animation via SMIL. See #33.

We have one viable direction:
CSS3/JS rotation of an element outside of the input (which could get tricky for positioning correctly).

Anybody want to take a stab at prototyping this on an input field?

joelpittet’s picture

Here's a Fiddle to start a prototype for people to play!

Needs to work cross-browser. Maybe IE9 can get a fallback?

http://jsfiddle.net/65wpyowp/

Wim Leers’s picture

#40: that doesn't rotate in Chrome?

joelpittet’s picture

It's un-prefixed, just a quick prototype. Auto-prefixer for anybody that wants to run with it.

Wim Leers’s picture

Ok, cool :) Sorry for the distraction.

jwilson3’s picture

The fiddle in #40 is pretty good, but the SVG can be optimized a bit further to shave some needless bytes.

The decoded svg in #40 is:

<svg class="throbber-svg" xmlns="http://www.w3.org/2000/svg" x="0" y="0" width="20px" height="20px" viewBox="0 0 20 20" enable-background="new 0 0 20 20" xml:space="preserve"><g class="throbber-group"><path class="arrow-1" fill="#40B6FF" d="M17.793,4.108L14.24,3.219c-0.647-0.162-1.288,0.24-1.424,0.893l-0.748,3.585c-0.136,0.653,0.215,0.897,0.78,0.543l1.656-1.038L14.6,7.171c0.704,1.161,0.983,2.576,0.641,3.997c-0.607,2.504-2.897,4.154-5.372,4.049c-0.569-0.024-0.62,0.463-0.62,0.463l-0.3,1.241c0,0-0.198,0.471,0.47,0.527c3.676,0.307,7.132-2.052,8.024-5.748c0.489-2.024,0.102-4.054-0.907-5.704l0.028-0.086l1.379-0.864C18.507,4.692,18.44,4.27,17.793,4.108z"/><path class="arrow-1" fill="#40B6FF" d="M7.228,11.636l-1.644,1.031L5.544,12.68c-0.704-1.161-0.983-2.576-0.641-3.997c0.607-2.504,2.897-4.154,5.372-4.049c0.569,0.024,0.62-0.463,0.62-0.463l0.3-1.241c0,0,0.198-0.471-0.47-0.527C7.05,2.095,3.594,4.454,2.702,8.15c-0.489,2.024-0.102,4.054,0.907,5.704l-0.021,0.065L2.134,14.83c-0.565,0.354-0.498,0.777,0.149,0.939l3.553,0.889c0.647,0.162,1.288-0.24,1.424-0.893l0.748-3.585C8.144,11.526,7.793,11.282,7.228,11.636z"/></g></svg>

Which can be reduced to:

<svg xmlns="http://www.w3.org/2000/svg" width="20" height="20" viewBox="0 0 20 20"><path fill="#40B6FF" d="M17.943,5.046L16.564,5.91l-0.028,0.086c1.009,1.65,1.396,3.68,0.907,5.704 c-0.893,3.696-4.348,6.055-8.024,5.748c-0.668-0.056-0.47-0.527-0.47-0.527l0.3-1.241c0,0,0.051-0.486,0.62-0.463 c2.475,0.105,4.765-1.545,5.372-4.049c0.342-1.421,0.063-2.836-0.642-3.997l-0.096,0.031L12.848,8.24 c-0.564,0.354-0.916,0.11-0.779-0.543l0.748-3.585c0.136-0.653,0.776-1.055,1.424-0.893l3.553,0.889 C18.439,4.27,18.507,4.692,17.943,5.046z M7.228,11.636l-1.644,1.031l-0.04,0.013c-0.704-1.16-0.983-2.576-0.641-3.997 C5.51,6.179,7.8,4.529,10.275,4.634c0.569,0.024,0.62-0.463,0.62-0.463l0.301-1.241c0,0,0.197-0.471-0.471-0.527 C7.05,2.095,3.594,4.454,2.702,8.15c-0.489,2.024-0.102,4.054,0.907,5.704l-0.021,0.065L2.134,14.83 c-0.565,0.354-0.498,0.777,0.149,0.939l3.553,0.889c0.647,0.162,1.288-0.24,1.424-0.893l0.748-3.586 C8.144,11.525,7.793,11.282,7.228,11.636z"/></svg>
ellizard’s picture

Status: Active » Needs review
FileSize
1.05 KB
46.67 KB
539 bytes

add svg/gif icon for loading. svg icon - in zip archive

Status: Needs review » Needs work

The last submitted patch, 45: update_throbber_icon-1974928-45.patch, failed testing.

ellizard’s picture

add gif/svg images

Manjit.Singh’s picture

Status: Needs work » Needs review

triggering test bot.

Status: Needs review » Needs work

The last submitted patch, 47: update_throbber_icon-1974928-45.patch, failed testing.

jwilson3’s picture

The patch in #47 is a patch on top of the patch in #45, and will never apply cleanly.

Besides that, I don't understand the point of the patch in #45 which uses an animated SVG since we've already established in #39 that animating SVGs are not supported in IE.

Even if there was a way to test for no-smil and fallback to a gif, this would still be less desirable than using CSS3 rotations. Modern IE supports CSS3 rotations and SVG, so we should try to leverage that, instead of forcing the gif fallback because of no SMIL support.

Let's shoot for a CSS Rotation solution first, and then work out appropriate fallbacks.

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 47: update_throbber_icon-1974928-45.patch, failed testing.

jwilson3’s picture

This issue needs to refocus around the D8 ajax progress throbber icon (throbber-active.gif) as stated in the issue title. The patches in #45 and #47 were for the "loading" icon, for which I've created #2575253: Update loading icon and use SVG. I'd be happy to properly review the patches above if @ellizard would like to move them to that issue so that he gets credit for the work. You may want to address my points mentioned above first though, including the fact that we cannot use SMIL (ie the <animate> functionality because it is now deprecated from Chrome, and not supported in IE).

jwilson3’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
202 bytes
230 bytes
4.13 KB

There are 2 ways the ajax progress throbber image is currently used in core:
* As part of additional DOM elements that are dynamically added during ajax requests. These DOM elements can be rotated with CSS.
* A background image added to autocomplete fields' <input> tag. These cannot be rotated as-is with SVG and CSS rotation/animation. More importantly, the current throbber-active.gif image committed to Drupal 8 two years ago to replace throbber.gif is surprisingly no longer animated. See #1069152: Throbber in textfield is misaligned when browser hardware acceleration enabled (gfx).

I recommend the following path to move forward:

1) avoid loss of current functionality, add HiDPI support and new shiny redesigned throbber. Replace the old static GIF with the newly designed SVG SVG ajax progress throbber redesign screenshot as a static (non-rotating) background image for autopopulating form fields. Add CSS to animate/rotate the SVG when shown using the .ajax-progress-throbber .throbber DOM elements.
2) file a follow-up to add back support for rotation functionality to the form autocomplete field. We could actually reopen and leverage #2181399: Consider moving the animated .GIF throbber outside the autocomplete fields and using CSS animation instead. (proposal by sun) for that purpose. (That issue was closed by Lewis opting for the simple class based state change #1847916: Replace the ajax-progress-throbber div with a class but I think it makes sense to reopen and revisit).

I'm going for the simple patch to move this forward: I've removed the throbber GIFs and optimized the second SVG from Comment #44 above to optimize it and round to 2 decimal places of accuracy instead of 3 (there is no perceived loss of quality). This also required slightly changing the throbber-inactive.png (and converting it to svg).

Before:
GIF ajax progress throbber redesign screenshot

After:
SVG ajax progress throbber redesign screenshot

New SVG code:

<svg xmlns="http://www.w3.org/2000/svg" width="20" height="20" viewBox="0 0 20 20">
  <path fill="#40B6ff" d="M17.94,5.05l-1.38,0.86L16.54,6c1.01,1.65,1.4,3.68,0.91,5.7c-0.89,3.7-4.35,6.05-8.02,5.75 c-0.67-0.06-0.47-0.53-0.47-0.53l0.3-1.24c0,0,0.05-0.49,0.62-0.46c2.47,0.11,4.76-1.54,5.37-4.05c0.34-1.42,0.06-2.84-0.64-4 L14.5,7.2l-1.66,1.04c-0.56,0.35-0.92,0.11-0.78-0.54l0.75-3.58c0.14-0.65,0.78-1.05,1.42-0.89l3.55,0.89 C18.44,4.27,18.51,4.69,17.94,5.05z M7.23,11.64l-1.64,1.03l-0.04,0.01c-0.7-1.16-0.98-2.58-0.64-4c0.61-2.5,2.9-4.15,5.37-4.05 c0.57,0.02,0.62-0.46,0.62-0.46l0.3-1.24c0,0,0.2-0.47-0.47-0.53C7.05,2.1,3.59,4.45,2.7,8.15c-0.49,2.02-0.1,4.05,0.91,5.7 l-0.02,0.07l-1.45,0.91c-0.56,0.35-0.5,0.78,0.15,0.94l3.55,0.89c0.65,0.16,1.29-0.24,1.42-0.89l0.75-3.59 C8.14,11.53,7.79,11.28,7.23,11.64z"/>
</svg>
aburrows’s picture

Status: Needs review » Needs work

Upon testing it, it does add the svg, however there is no animation. I'm guessing we want an animation of the arrows spinning?

jwilson3’s picture

Issue summary: View changes
jwilson3’s picture

Assigned: Unassigned » jwilson3
jwilson3’s picture

Here I've added the CSS rotation with an "is-throbbing" class and improved how the throbber works inside dropbuttons.

To test this, you can check that the spinner works on the following pages:

  1. Attach an image to an article node (/node/add/article).
  2. Enable or disable a view (/admin/structure/views) using the dropbutton.
    Collapsed



    Expanded

Status: Needs review » Needs work

The last submitted patch, 58: drupal-svg-throbber-1974928-57.patch, failed testing.

The last submitted patch, 58: drupal-svg-throbber-1974928-57.patch, failed testing.

jwilson3’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
8.16 KB
436.33 KB

Looks like excluding the binary details of the image files being deleted made the patch in #57 invalid; here is a reroll with all the binary data glory.

Also, here is a little screencast gif I recorded demonstrating the new throbber in action.

jwilson3’s picture

Issue summary: View changes
FileSize
6.78 KB
corbacho’s picture

Good work James
I didn't have time to go through this patch/issue. I will try to review later properly.
But I noticed file /core/modules/system/css/components/autocomplete-loading.module.css lacks CSS animation.
Patch #61 was not applying, rerolled attached.

jwilson3’s picture

@Corbacho: Thanks!

Regarding non-animation of autocomplete, I think this should be handled in a follow-up. Please read My comment #54 above, which explains it all.

hass’s picture

As this patch has not committed in-time, can someone RTBC #2642362: Animation of throbber-active.gif image is broken to fix the current icon, please?

hass’s picture

Aside, the is-throbbing class used here is currently named ui-autocomplete-loading, but I like the new name. However it may require some cleanup that is not inside this patch.

hass’s picture

Version: 8.0.x-dev » 8.1.x-dev
chrisfree’s picture

In fixing #2642362: Animation of throbber-active.gif image is broken I stumbled on this thread. The latest patch applies cleanly but since the Stable was made the default base, these changes needed to be applied there as well. The attached patch takes care of replicating the existing proposed changes over on the Stable side of things.

As a side note, in ajax-progress.module.css there is also a "Full screen throbber" section. This uses another animated .gif that might also be worth updating to SVG for the same reasons we're altering the throbber. I'd propose to do that in a separate issue, however.

jwilson3’s picture

Cant really make sense of what changed, your interdiff didnt seem to actually show the differences.

chrisfree’s picture

@jwilson3 sorry, this was my first time attempting an interdiff. I thought I had done it right, but I'll try again. Basically, I duplicated your changes within the stable theme. I'll push up a new interdiff here shortly. Stand by.

chrisfree’s picture

@jwilson3 actually, I think my interdiff is correct. I followed the steps found here and the resulting file was exactly the same as my previous. Perhaps this is confusing because the changes are identical but are now also occurring within the Stable theme? There weren't any changes to those that you and @corbacho applied but those were simply replicated in the equivalent files within Stable.

So changes you made in /core/misc were also made in /code/themes/stable. Does that clear things up? Or am I missing something?

hass’s picture

Status: Needs review » Needs work

The gif/png images need to be kept at least until D9.

We may safe one http request by adding both svg images into a svg-stripe-set as symbols.

joelpittet’s picture

We can move the improvements to Bartik/Seven though they are free to change up for 8.1.x

jwilson3’s picture

@chrisfree: ok, i see... missed that from your comment in #68. Thanks for the clarification.

The interdiff is really confusing, this line tripped me up, and i dont understand it:

Corbacho's code:

diff --git a/core/misc/ajax.js b/core/misc/ajax.js
index 4dba53e..0a7b0de 100644
--- a/core/misc/ajax.js
+++ b/core/misc/ajax.js
@@ -704,7 +704,7 @@
    * Sets the throbber progress indicator.
    */
   Drupal.Ajax.prototype.setProgressIndicatorThrobber = function () {
-    this.progress.element = $('<div class="ajax-progress ajax-progress-throbber"><div class="throbber">&nbsp;</div></div>');
+    this.progress.element = $('<div class="ajax-progress ajax-progress-throbber"><div class="throbber is-throbbing">&nbsp;</div></div>');

Your code:

diff --git a/core/misc/ajax.js b/core/misc/ajax.js
index 225b2f0..8b57afb 100644
--- a/core/misc/ajax.js
+++ b/core/misc/ajax.js
@@ -713,7 +713,7 @@
    * Sets the throbber progress indicator.
    */
   Drupal.Ajax.prototype.setProgressIndicatorThrobber = function () {
-    this.progress.element = $('<div class="ajax-progress ajax-progress-throbber"><div class="throbber">&nbsp;</div></div>');
+    this.progress.element = $('<div class="ajax-progress ajax-progress-throbber"><div class="throbber is-throbbing">&nbsp;</div></div>');

Interdiff (wat?):

diff -u b/core/misc/ajax.js b/core/misc/ajax.js
--- b/core/misc/ajax.js
+++ b/core/misc/ajax.js
@@ -704,7 +704,7 @@
    * Sets the throbber progress indicator.
    */
   Drupal.Ajax.prototype.setProgressIndicatorThrobber = function () {
-    this.progress.element = $('<div class="ajax-progress ajax-progress-throbber"><div class="throbber is-throbbing">&nbsp;</div></div>');
+    this.progress.element = $('<div class="ajax-progress ajax-progress-throbber"><div class="throbber">&nbsp;</div></div>');
chrisfree’s picture

@jwilson3right? I spent way too much time trying to sort that out too. It makes literally zero sense. Otherwise the patch looks fine, eh?

jwilson3’s picture

Yes. its good aside from that, now that i actually looked to see /stable/ in the paths that changed ;) Pardon my misunderstandings, banter and general distractions.

chrisfree’s picture

Actually, I think we need to keep the older versions of the files instead of deleting them. I think that is what @haas is saying. So a revised patch that doesn't delete them might be in order. I could see instances where other developers might be using those paths in other contexts.

chrisfree’s picture

Finally getting around to recreating this patch such that it does not delete the existing gif/png files. Patch to follow shortly.

chrisfree’s picture

Status: Needs work » Needs review
FileSize
12.53 KB
803 bytes

Updated patch and interdiff. This patch should do the same as the previous, only it does not delete the existing core imagery related to throbbers.

markdorison’s picture

Status: Needs review » Reviewed & tested by the community

Patch in #79 worked for me in testing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 79: update_throbber_icon-1974928-79.patch, failed testing.

droplet’s picture

the new icon is ugly. no IE fallback ?

The last submitted patch, 79: update_throbber_icon-1974928-79.patch, failed testing.

chrisfree’s picture

@droplet: I'm confused. Are you saying that this animated SVG is ugly?

SVG Animation

Or are you saying that you are seeing something else?

As far as an IE fallback is concerned, SVG is supported back to IE9 and CSS animation is supported back to IE 10. Without being animated, we should still see the SVG. This is a reasonable fallback in my opinion.

droplet’s picture

I'm not keen on it anyway but if we wanted to support it. We can detect it from JS side and add a class name to toggle it.

+++ b/core/themes/stable/css/system/components/ajax-progress.module.css
@@ -47,3 +54,19 @@ tr .ajax-progress-throbber .throbber {
+@-moz-keyframes rotate-throbber {
+  to {-moz-transform: rotate(-360deg);}
+}
...
+@-ms-keyframes rotate-throbber {
+  to {-ms-transform: rotate(-360deg);}
+}

I believe we didn't need these 2 prefix.

hotpizzas’s picture

@droplet: Drupal 8 doesn't support IE8, so why worry about an IE fallback when IE9 supports SVG?

droplet’s picture

@hotpizzas,
IE9 doesn't support keyframes. We can't call SVG is fallback.

No radius corners in IE is make sense, it's a basic visual change. Drop anime in a Loader icon, it's a UX problem. Is it loading ? or disconnection ?

GIF is bad, but not that bad at many case in most sites. I'm now using Macbook Retina to post. I think the GIF icon shown on this page is acceptable and better than a still image in non-retina IE9 screen.

If we all think IE9 is worthless, Let's announce it in public. It's very bad to kill IE9 silently but official said it's still supporting for marketing purpose.

jwilson3’s picture

Is modernizr feature detection for cssanimations a dependency we could add?

if(!Modernizr.cssanimations) {
    //jQuery rotation fallback 
}

OR

.no-cssanimation .ajax-throbber {
   background-image: url(path/to/fallback-throbber.gif);
}
nandasoftware’s picture

Assigned: Unassigned » nandasoftware
Issue tags: +drupalconasia2016
nandasoftware’s picture

Assigned: nandasoftware » Unassigned
soumyajit.basu’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
15.55 KB
15.39 KB

Checked. The throbber is appearing correctly with the new icon. Checked with the scenario as mentioned above. Please check the attached screenshot.

star-szr’s picture

This would be a slight BC break for Stable/Classy but I think it might be justifiable in 8.1.x. I'd like to give it some more thought and testing though.

In the meantime…

+++ b/core/modules/views_ui/css/views_ui.admin.theme.css
@@ -766,14 +766,36 @@ td.group-title {
+  border-radius:  0 50% 50% 0;

Minor: Two spaces after colon. Applies to the Stable version of this file as well. Leaving at RTBC because it's very minor :)

gauravjeet’s picture

Changes as per suggestion in #92.

gauravjeet’s picture

Status: Reviewed & tested by the community » Needs review

Triggering the test bot

The last submitted patch, 32: retina_ready_throbber-1974928-31-do-not-test.patch, failed testing.

The last submitted patch, convert-to-tabs-do-not-test.patch, failed testing.

star-szr’s picture

Status: Needs review » Needs work

Discussed this issue with @xjm and @effulgentsia.

The safest bet here would be to not make these changes in Stable and Classy because of the BC break and only change core, Bartik, and Seven.

We can then provide instructions for how themers can use libraries-override to get this shiny goodness into their themes. Seem reasonable?

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

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now 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.

jwilson3’s picture

Re #97, what precicely is unsafe about swapping a gif with a rotating svg?

Could you expand upon the BC (backward compatibility? i presume) break?

star-szr’s picture

@jwilson3 the CSS changes, for folks who have overridden the spinner CSS we are changing things from underneath them.

jwilson3’s picture

@Cottser: Ok, does that mean #2405059: Replace menu images with Libricons also needs to be bumped to 8.2.x?

tstoeckler’s picture

Issue tags: +DrupalBCDays
ckrina’s picture

Status: Needs work » Needs review
FileSize
7.16 KB

Trying another patch without Stable changes.

@Cottser when you said to provide instructions, you mean in the code itself as comments?

mgifford’s picture

I haven't looked at at the latest patch, but really like the examples in #2575253: Update loading icon and use SVG nice to revision the throbber..

ckrina’s picture

Although I also like the other icons, the idea on using the suggested one was that even if it can't be animated (old IE & friends) it can be interpreted as an "update" or "loading" action anyway.

Proposed throbber icon

Wim Leers’s picture

I like it!

aburrows’s picture

Can we get this decided and closed off at Drupalcon?

Bojhan’s picture

Status: Needs review » Reviewed & tested by the community

Going to put this back to RTBC. Looked like Coster was onboard already without the Stable break, looks like this removes that issue.

alexpott’s picture

Assigned: Unassigned » star-szr

Assigning this to @Cottser

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 103: update_throber_icon-1974928-103.patch, failed testing.

star-szr’s picture

Status: Needs work » Reviewed & tested by the community

It was a migrate test fail, putting back to RTBC :)

star-szr’s picture

Assigned: star-szr » Unassigned
Status: Reviewed & tested by the community » Needs work

Yup this is looking much better, thanks @ckrina! What I would love to see here before committing is bringing the SVG version into Seven and Bartik for all throbbers, not just the dropbutton one.

One other idea along the lines of #97 would be to include a small library and CSS file for SVG throbbers in Stable and not load that library from Stable, only load it from Bartik and Seven via libraries-extend. This way any theme can pull in this "Throbber SVG-ify" library. Any libraries we add to the Stable base theme this way would have to remain stable once they are in. I think of it as an API addition to Stable so I think these types of changes would have to happen in minor versions not patch versions.

+++ b/core/modules/views_ui/css/views_ui.admin.theme.css
@@ -766,14 +766,36 @@ td.group-title {
+  border-radius:  0 50% 50% 0;

Minor: Extra space after colon here (two instead of one) per https://www.drupal.org/node/1887862#properties and as mentioned in #92 (#93 didn't provide an interdiff, but also didn't fix it).

Manjit.Singh’s picture

Status: Needs work » Needs review
FileSize
7.15 KB
569 bytes

removed extra space and added interdiff.

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

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now 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.

Gábor Hojtsy’s picture

#2775725: Update throbber icon started independently to implement the Seven styleguide spinner. We should figure out which one to bring forward. Discussed this on the UX meeting today.

MustangGB’s picture

FWIW I think the spinner from #2775725: Update throbber icon looks a lot nicer.

marameodesign’s picture

I'll personally believe #105 has a great point.

lauriii’s picture

Title: Update throbber icon and use SVG » Update throbber icon
Status: Needs review » Needs work
Issue tags: +Needs issue summary update

I discussed today with @ckrina and @alexpott and we agreed that we should change the approach taken on this issue.

The approach that we agreed on:

  1. Use throbber with a minimal design from the Ajax system. We propose to use the same throbber from jQuery UI that is used on modals.
  2. Override throbber for the original gif in Stable since we don't want to affect existing themes
  3. Override throbber again in Bartik with a design that is reflected by Bartik
  4. Ensure that overriding throbbers is as easy as possible

The reason why we want to do so is because:

  1. Themes should be responsible for setting design for the throbber
  2. Modules should provide minimal design and therefor shouldn't reflect Bartik or Seven
  3. Overriding throbbers should be done in Core to demonstrate how its done easily
  4. We don't want to give drupally feeling if a theme doesn't override the throbber
ckrina’s picture

Assigned: Unassigned » ckrina
Issue summary: View changes

I'll work on that.

ckrina’s picture

I'll work on this.

ckrina’s picture

Issue summary: View changes
FileSize
1.25 KB
356 bytes
Gábor Hojtsy’s picture

Can we try to not work on this one and #2775725: Update throbber icon in parallel and choose one path instead of two parallel paths wasting our time? :) Please :)

ckrina’s picture

Assigned: ckrina » Unassigned
Status: Needs work » Postponed
Related issues: +#2775725: Update throbber icon

No problem! We can come back to this one when #2775725: Update throbber icon in Seven theme is finished and decide if design or anything else needs to be readjusted for Bartik.
I change the status to Postponed until the other one is finished.

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

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now 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.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.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.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.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.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.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.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.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.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

ressa’s picture

There has been great progress with updating the throbber icon in the new Claro theme #3087950: Progress throbber position is incorrect, so adding here for reference.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.