#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 deprecated-css |
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)
Change History (47)
#2
@
12 years 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.
#4
@
12 years 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
#5
follow-up:
↓ 6
@
12 years 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
#6
in reply to:
↑ 5
@
12 years 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.
#7
@
12 years 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 a low-res version. Props to @joen for the PSD.
#8
@
12 years 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.
#9
@
12 years 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.
#10
@
12 years 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.
#12
@
12 years 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.
#15
@
12 years 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.
#16
@
12 years 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).
#18
follow-up:
↓ 19
@
12 years 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.
#19
in reply to:
↑ 18
@
12 years 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
#20
follow-up:
↓ 21
@
12 years 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.
#21
in reply to:
↑ 20
;
follow-up:
↓ 22
@
12 years 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.
#22
in reply to:
↑ 21
;
follow-up:
↓ 23
@
12 years 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
andclass-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.
#23
in reply to:
↑ 22
@
12 years ago
Replying to SergeyBiryukov: Thanks Sergey! That was super helpful.
21456.5.diff should cover the rest of the remaining spinners.
#24
@
12 years ago
21456.6.diff adds media query for wpspin_light-2x.gif for backwards compatibility.
#27
@
12 years 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?
#29
@
12 years 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.
#30
@
12 years 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
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.