Change History (52)

​.show() restores the display property to whatever it was initially. Because the spinner is a span element, that value will be inline which causes the width and height properties to be ignored conform to the ​CSS Level 1 specifications.

The spinner is displayed in .widget-title because the float property is used which establishes a new ​block formatting context, while in .sidebar-name float is set to none.

One solution would be to use ​.css() instead of ​.show() to set the display property to inline-block. Or to make the element to establish a new BFC which requires less changes in code.

Other affected ones include: .find-box-search .spinner, .options-general-php .spinner, .press-this #publishing-actions .spinner, .sidebar-name .spinner, and .update-php .spinner, in wp-admin.css alone. But I doubt all of those are broken.

.show() and .hide() are clever but we should probably pivot to adding/removing a class in the future, as that will also be helpful.

The spinner does appear for me when rearranging within a sidebar (note that those are also .sidebar-name), including Inactive Widgets. As azaozz noted in #20957:comment:4, is the only time that it appeared, at least in 3.4. I just gave it a run in 3.4.2 and can't get that particular spinner to appear except in the rearrangement scenario.

As nacin noted, I am one of the notorious Firefox holdouts :) They are working just fine for me. Testing in the console is nice and all, but watching it fly by in practice shows me that inline-block is what gets set when rearranging widgets.

As nacin noted, I am one of the notorious Firefox holdouts :) They are working just fine for me. Testing in the console is nice and all, but watching it fly by in practice shows me that inline-block is what gets set when rearranging widgets.

Got in error by myself while switching from .ajax-feedback to .spinner. It's confusing because of the two different methods used to show the spinner (depending on float or not). Who knows where my mind was.

Still, applying the patch what I've suggested in my first comment would make possible to use:

Will have to test to see what side effects the patch might have. Doesn't seem right to just suddenly make it positioned absolutely, and anything we do in core in the future really should be about the spinner in general, not just in one place. Reusable, and all that, which I think is what you're looking for here.

Yeah, adding/removing a class would be best. Yes, setting visibility to hidden reserves the place but that is usually good as it ensures the elements/text around the spinner won't "jump" when it's displayed.

Thinking more about this: there is an accessibility aspect here too. Ideally screen readers should announce that something is happening on the page, so we may need to toggle some aria attributes there. Lets look at spinners again in 3.6.

Currently, spinners are toggled via .show()/.hide() jQuery methods. But this doesn't seem to work in all cases creating some inconsistencies (mainly when floats are used).

attachment:22839.2.diff​ introduces the .active class which toggles a spinner when added/removed. The main advantage is that it adds a standard way of toggling a spinner. Another advatage of adding/romoving a class is that it allows more than one property to get changed at once (not only display) if it's needed.

In Quick Edit mode, the spinner seems to have an unnecessary offset of 10px (removed in patch).

Also found that the #major-publishing-actions .spinner never shows up when editing an attachment (with or without the patch applied) like it does when editing a post.

Steps to reproduce:

In Dashboard, go to Media > Libary.

Select an image.

Press Update.

This particular spinner (from #submitpost) is toggled only in wp-includes/js/autosave.js which is not loaded when editing an attachment.

It's time we fix this. Let's go with a class name like .is-active - we've still got a lot to figure out when it comes to naming standards, but I think that will be a better starting point when it comes to JS-facing selectors.

I did ​a lot of testing here, but I know there are going to be some places that redefine spinner CSS that are now broken. Let's open new tickets for those instances. #30725 will address the media library spinner, which is definitely broken right now.

Please note that changing the styles for the spinners from a display:none to a visibility:hidden might (and in fact does) have consecuences in third party plugins, as I just found out in one I work on.

In our particular case, we are adding the spinner span dinamically, and jQuery-show()-ing/hide()-ing it on demand, which does not have any effect over the visibility property. Not a big deal, we plan to implement out own spinner span soon, but this change might affect other too. Worth mentioning :-)

I am aware that this is a little bit of a visual breaking change - I'm not sure there's really a way around it. It is, as far as I see, a matter of two things: spinners now taking up space all the time (which they should have been in the first place, hence this fix), and spinners not showing until plugins/themes have been updated. I don't consider either of those to be critical visual bugs, and would rather have a better base moving forward.