WordPress.org

Make WordPress Core

#21456 closed task (blessed) (fixed)

Replace animated spinner gifs

Reported by: nacin Owned by: lessbloat
Milestone: 3.5 Priority: normal
Severity: normal Version:
Component: UI Keywords: has-patch
Focuses: Cc:

Description

As discussed in the IRC dev chat on Aug 1, we should replace our existing animated spinners with spin.js: http://fgnass.github.com/spin.js/

This has some advantages, including HiDPI support, a fresher design, color scheme support, and an all JS/CSS implementation.

Attachments (15)

21456.diff (22.6 KB) - added by lessbloat 20 months ago.
wpspin_light.gif (17.0 KB) - added by lessbloat 20 months ago.
wpspin_light.psd.zip (142.5 KB) - added by lessbloat 20 months ago.
IMG_0034.PNG (1.5 KB) - added by azaozz 20 months ago.
wpspin_light.png (789 bytes) - added by alternatekev 20 months ago.
png replacement for spinner gif, low-res
wpspin_light-2x.png (1.7 KB) - added by alternatekev 20 months ago.
png replacement for spinner gif, hi-res
hidpi-spinner.diff (19.2 KB) - added by alternatekev 20 months ago.
patch for gif spinner replacement
21456.2.diff (22.7 KB) - added by lessbloat 19 months ago.
wpspin_light.2.gif (8.9 KB) - added by lessbloat 19 months ago.
Goes with 21456.2.diff
21456.3.diff (35.4 KB) - added by lessbloat 19 months ago.
21456.4.diff (39.8 KB) - added by lessbloat 19 months ago.
21456.5.diff (42.1 KB) - added by lessbloat 19 months ago.
21456.6.diff (42.8 KB) - added by lessbloat 19 months ago.
wpspin_light-2x.gif (8.9 KB) - added by lessbloat 19 months ago.
21456.7.diff (2.5 KB) - added by lessbloat 19 months ago.

Download all attachments as: .zip

Change History (46)

comment:1 johnbillion21 months ago

Interesting to note that wp.com uses spin.js already (note sure where exactly). When spin.js came up on Hacker News there were a few posts noting that its CPU usage was pretty high.

comment:2 azaozz21 months ago

I'm having some second thoughts on it too. Quite CPU intensive, on slower PCs (tablets, smart phones) interferes with typing in TinyMCE (also a little bit with typing in any text field).

The fact that it uses css only is very attractive. Perhaps we can look into using some simpler shapes, 3 pulsating squares, or circles, or... Something that can be animated by using css transitions directly and doesn't need the trails.

The "old way" of doing something similar is to have separate transparent/semi-transparent png frames. It scales down well and looks really good, but needs 10-12 small images.

comment:3 rfair40421 months ago

  • Cc rfair404 added

comment:4 lessbloat20 months ago

Brought this up in the UI chat. We all agreed that we'd rather stick with a no-js solution for the time being. @joen already came up with a 2x version of the spinner:

http://core.trac.wordpress.org/attachment/ticket/21019/wpspin_light-2x.gif

Last edited 20 months ago by SergeyBiryukov (previous) (diff)

comment:5 follow-up: lessbloat20 months ago

I thought I'd check out the CPU usage myself (to see if perhaps it had been improved). Turns out, it has not.

http://neteye.github.com/activity-indicator.html appears to be somewhat less CPU intensive. I tested them both (on FF 14.0.1 - Mac 10.7.4):

Spin.js - Single stance consumes between 40-48% CPU, constantly while it's on screen

activity-indicator - 12-15% CPU constantly

comment:6 in reply to: ↑ 5 Viper007Bond20 months ago

Replying to lessbloat:

Spin.js - Single stance consumes between 40-48% CPU, constantly while it's on screen

activity-indicator - 12-15% CPU constantly

I don't get such drastic results (faster CPU probably) but I do see activity-indicator being better. 6-7% rather than 24-25%, using Firefox.

Last edited 20 months ago by Viper007Bond (previous) (diff)

lessbloat20 months ago

lessbloat20 months ago

lessbloat20 months ago

comment:7 lessbloat20 months ago

21456.diff adds one single retina-friendly animated gif spinner to rule them all (1x and 2x). Comes in at 17kb, so I didn't see any point in creating two of them.

Version 0, edited 20 months ago by lessbloat (next)

comment:8 lessbloat20 months ago

Something like this also works allowing us to use a png instead of a gif (to avoid the gif artifacts on different colored backgrounds):

<html>
	<head>
		<title>Spin me</title>
		<style type="text/css">
			.spin {
				background: url('spinner.png') no-repeat;
				width: 20px;
				height: 20px;
				-webkit-animation: spin 0.7s infinite linear;
				-moz-animation: spin 0.7s infinite linear;
				-o-animation: spin 0.7s infinite linear;
				-ms-animation: spin 0.7s infinite linear;
			}
			@media only screen and (-moz-min-device-pixel-ratio: 1.5), only screen and (-o-min-device-pixel-ratio: 3/2), only screen and (-webkit-min-device-pixel-ratio: 1.5), only screen and (min-device-pixel-ratio: 1.5) {
				.spin {
					background-image: url(spinner-2x.png);
					background-size: 20px 20px;
				}
			}
			@-webkit-keyframes spin {
				0% { -webkit-transform: rotate(0deg);}
				100% { -webkit-transform: rotate(360deg);}
			}
			@-moz-keyframes spin {
				0% { -moz-transform: rotate(0deg);}
				100% { -moz-transform: rotate(360deg);}
			}
			@-o-keyframes spin {
				0% { -o-transform: rotate(0deg);}
				100% { -o-transform: rotate(360deg);}
			}
			@-ms-keyframes spin {
				0% { -ms-transform: rotate(0deg);}
				100% { -ms-transform: rotate(360deg);}
			}
			@keyframes spin {
				0% { transform: rotate(0deg);}
				100% { transform: rotate(360deg);}
			}
		</style>
	</head>
	<body>
		<div class="spin"></div>
	</body>
</html>

However, when I compare the performance of this block of code against http://neteye.github.com/activity-indicator.html, they are about the same. At this point, I'm leaning towards switching the current gif to use http://neteye.github.com/activity-indicator.html.

Unless of course, we come up with anything better.

comment:9 goto1020 months ago

  • Cc dromsey@… added

At this point, I'm leaning towards switching the current gif to use http://neteye.github.com/activity-indicator.html.

Just wanted to mention that there is an issue with Net Eye Activity Indicator on webkit browsers when used with stylesheets that have @rules.

This is kind of a big deal, since child themes will inherently be using the @import rule to grab their parent theme's stylesheet.

Good news, there is a solution (see comment by becw): https://github.com/neteye/jquery-plugins/pull/47

It's kind of a bummer that this fix hasn't been integrated officially though.

comment:10 lessbloat20 months ago

Here's the trimmed down base code I was going to propose using (if we go with the neteye solution):

<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<script type="text/javascript" src="http://ajax.googleapis.com/ajax/libs/jquery/1.4.2/jquery.js"></script>
<script src="https://raw.github.com/gist/3061879/8e9d91998f796d452c7b3ef0c122cff8f4392124/jquery.activity-indicator-1.0.0.min.js" ></script>
</head>
<body>
<div id="busy3"></div>
<script>
	$(function() {
		$('#busy3').activity({segments: 7, width:2, space: 0, length: 3, speed: 1.5});
	});
</script>
</body>
</html>

Which runs at between 6% and 11% CPU on my laptop.

Last edited 20 months ago by lessbloat (previous) (diff)

azaozz20 months ago

comment:11 azaozz20 months ago

Tested this too. On the desktop PC it takes 2-3% CPU in both Chrome and Firefox. However on iPad 1 with iOS 5.1.1 it seems to be much higher. Also when zooming in noticed some irregularities with the bars.


comment:12 koopersmith20 months ago

As discussed in IRC, I think the best option for moving forward is using a PNG rotated using CSS3 animations, as outlined in lessbloat's earlier comment. We can override the background-image to use a GIF in browsers that don't support CSS3 animations.

Benefits of using a rotating PNG:

  • More granular control over transparency.
  • We don't have to matte the spinner based on the background, which makes the spinner far more flexible.
  • Low CPU usage.
  • Rendering bugs are less likely to occur.

comment:13 lessbloat20 months ago

  • Keywords needs-patch added

alternatekev20 months ago

png replacement for spinner gif, low-res

alternatekev20 months ago

png replacement for spinner gif, hi-res

alternatekev20 months ago

patch for gif spinner replacement

comment:14 helenyhou20 months ago

  • Keywords has-patch added; needs-patch removed

comment:15 nacin19 months ago

  • Owner set to lessbloat
  • Status changed from new to assigned
  • Summary changed from Replace animated spinner gifs with spin.js to Replace animated spinner gifs
  • Type changed from enhancement to task (blessed)

I believe this is being reviewed by lessbloat.

comment:16 lessbloat19 months ago

I tested hidpi-spinner.diff. Two things:

  • For whatever reason, when using CSS animation with the transparent PNG, the icon is somewhat lopsided, and it suffers from intermittent delays (tested in FF 14.0.1). I played with the PNG trying to get it to not look lopsided, without any luck.
  • Additionally, I tested CPU on the dashboard without the spinner showing (11%-12%) CPU, and then I dropped the slider code in via the console, and the CPU jumped to (38%-40%) which still seems really high to me for a small spinner.

For these reasons, my proposal would be to stick with 21456.diff, which isn't perfect, but overall seems to offer a better solution than using CSS animation with the PNG (at this point).

lessbloat19 months ago

lessbloat19 months ago

Goes with 21456.2.diff

comment:17 lessbloat19 months ago

Per UI chat, tried out spinner at 32px, and it appears to work:

http://f.cl.ly/items/1230093i0R1M0g0V0f1u/32px-animated.gif

Two Notes:

  • 21456.2.diff also fixes a conflict in 21456.diff
  • wpspin_light.2.gif should be named wpspin_light.gif, was renamed at upload because wpspin_light.gif was uploaded previously (at a 64px res).

comment:18 follow-up: nacin19 months ago

I suggested to lessbloat that it is probably time to move to a span with an image as a background, for both forwards and backwards compatibility reasons. It will be nice to have less insane markup to implement a spinner.

lessbloat19 months ago

comment:19 in reply to: ↑ 18 lessbloat19 months ago

Replying to nacin:

I suggested to lessbloat that it is probably time to move to a span with an image as a background, for both forwards and backwards compatibility reasons. It will be nice to have less insane markup to implement a spinner.

Replaced all <img> spinners with <span class="spinner"> in 21456.3.diff

comment:20 follow-up: nacin19 months ago

The reason why visibility: hidden is often used instead of display: none is because then the spinner takes up space. When toggling the display property, we've often seen other objects move around, which can be jarring and certainly not expected behavior. So it is definitely safer to use visibility.

I'd like to do either a 'visible' class or an 'active' class. Then show() and hide() can be add/toggle/removeClass. Better then css('visibility', value), that's for sure.

Looking at 21456.3.diff, I'm seeing a lot of brokenness. Not your fault — we basically rolled our own spinner code for quite a long time. This is unfortunately way more than just a find-and-replace operation. Also, the spinners on options-general.php aren't unused.

I suggest we introduce .spinner, then start to use it wherever we can. Each instance will need to be confirmed (which is not fun or easy) as being not broken style-wise. A decent amount of CSS classes (.waiting, .ajax-loading) can probably vanish, and some JS could probably be cleaned up.

Also, the nice thing about .spinner is it also allows us to direct a media query at wpspin_light.gif for the 2x spinner.

@lessbloat: Perhaps we can get someone else to pick this up so we don't get sucked into this much further.

lessbloat19 months ago

comment:21 in reply to: ↑ 20 ; follow-up: lessbloat19 months ago

Couple improvements in 21456.4.diff:

Replying to nacin:

The reason why visibility: hidden is often used instead of display: none is because then the spinner takes up space. When toggling the display property, we've often seen other objects move around, which can be jarring and certainly not expected behavior. So it is definitely safer to use visibility.

With the new spinners using float, this should no longer be necessary. They should all work with display: none. Please let me know if you see some place where there is any movement, and I'll fix it.

I'd like to do either a 'visible' class or an 'active' class. Then show() and hide() can be add/toggle/removeClass. Better then css('visibility', value), that's for sure.

This still needs to be done.

Looking at 21456.3.diff, I'm seeing a lot of brokenness. Not your fault — we basically rolled our own spinner code for quite a long time. This is unfortunately way more than just a find-and-replace operation. Also, the spinners on options-general.php aren't unused.

I went through each spinner one by one and tested them in several browsers. I made a spreadsheet with the status of each: https://docs.google.com/spreadsheet/ccc?key=0Av7nMZXUOf52dExvVXBKZE8tbHFJWVRBck9kQkJxd3c#gid=0 There are still a couple that I need some help figuring out. Again, please also report any additional bugs that you spot, and I'll fix them.

I suggest we introduce .spinner, then start to use it wherever we can. Each instance will need to be confirmed (which is not fun or easy) as being not broken style-wise. A decent amount of CSS classes (.waiting, .ajax-loading) can probably vanish, and some JS could probably be cleaned up.

Done. Still need to do this for the spinners in the spreadsheet that I've been unable to test.

Last edited 19 months ago by lessbloat (previous) (diff)

comment:22 in reply to: ↑ 21 ; follow-up: SergeyBiryukov19 months ago

Replying to lessbloat:

There are still a couple that I need some help figuring out.

  • class-wp-themes-list-table.php:
    On Install Themes screen, select any feature in the filter, click "Find Themes". Scroll to the bottom. The spinner will appear in the bottom right corner while the themes are loading.
  • class-wp-plugin-install-list-table.php and class-wp-theme-install-list-table.php:
    These look like remnants of AJAX in list tables (#14579, #16262).
  • class-wp-editor.php (.river-waiting):
    Open "Insert/edit link" dialog, search for something that would return many posts. Scroll to the bottom. The spinner will appear at the bottom while the posts are loading.

lessbloat19 months ago

comment:23 in reply to: ↑ 22 lessbloat19 months ago

Replying to SergeyBiryukov: Thanks Sergey! That was super helpful.

21456.5.diff should cover the rest of the remaining spinners.

lessbloat19 months ago

lessbloat19 months ago

comment:24 lessbloat19 months ago

21456.6.diff adds media query for wpspin_light-2x.gif for backwards compatibility.

comment:25 nacin19 months ago

In [22019]:

New HiDPI spinner. Uses clean <span class="spinner"></span> markup.

Be on the lookout for weirdness.
props lessbloat. see #21456.

comment:26 mitchoyoshitaka19 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

comment:27 ocean9019 months ago

Shouldn't we provide some backwards compatibility for plugins which are using the old image with the .ajax-loading CSS class to hide the spinner by default?

comment:28 nacin19 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Yes.

comment:29 nacin19 months ago

.ajax-feedback should get visibility: hidden back, and any remaining instances of it in HTML should be removed.

#wpcontent .ajax-loading should also return as visibility: hidden.

In both cases let's mark them with some kind of basic /* deprecated */ comment. Maybe one day (not now) we spin off a deprecated.css that we can use so when people inspect CSS, it's clear their styling comes from a deprecated file.

lessbloat19 months ago

comment:30 lessbloat19 months ago

Changes in 21456.7.diff:

  • Set #ajax-loading, .ajax-loading, .ajax-feedback, .imgedit-wait-spin, .list-ajax-loading to visibility: hidden;
  • Removed all instances of .ajax-loading from HTML (all instances of .ajax-feedback were already removed)
  • Fixed margin-top for theme list page spinner

comment:31 nacin19 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In [22167]:

Retina/span spinner cleanup. Restores CSS classes no longer used by core. props lessbloat. fixes #21456.

Note: See TracTickets for help on using tickets.