Fix/enhance the UI around the link to enable/disable auto-updates

[azaozz]

This is somewhat related to #17.

1. A bit of a nitpick but when clicked, the links to enable/disable auto-updates jump/shift under the cursor (as the dashicons-update spinning icon is shown on the left side). That jump is unexpected/quite annoying.
   

2. If the AJAX request is slow and the user clicks second time, or there is an error, the UI "locks" at "Disabling/Enabling auto-updates...".
   

[pbiron]

This is somewhat related to #17.

1. A bit of a nitpick but when clicked, the links to enable/disable auto-updates jump/shift under the cursor (as the dashicons-update spinning icon is shown on the left side). That jump is unexpected/quite annoying.

I know what you're saying there.

Speaking just for myself (and not the "team"), I don't find it "annoying" but I can see that others might. It is probably "unexpected" the first (few) time a user clicks on the links, but will likely (I hope) quickly become "second nature". But having to say that means that it might not be a good UX, I'll concede.

At one point, we were displaying the "recycle" dashicon all the time...in which case the shift wouldn't happen. I don't remember exactly why we removed it and have it only displaying while the AJAX processing is happening. I'll have to dig back through the history to refresh my memory.

2. If the AJAX request is slow and the user clicks second time, or there is an error, the UI "locks" at "Disabling/Enabling auto-updates...".

I haven't been able to reproduce this behavior. Can you provide more details about how you can get it to happen?

[azaozz]

I haven't been able to reproduce

Seems this happens on JSON errors, i.e. when there is unexpected response from the server. Can even be PHP notices, etc. Also when more than one request/the user clicked the link multiple times.

Steps to reproduce (happens only when disabling auto-updates):

• Add sleep( 3 ); to wp-config.php to partially emulate slow connection (optional).
• Click on the link multiple times.

[pbiron]

OK, I'm now able to reproduce, without needing the sleep() by clicking Disable rapid-fire (i.e., 10-15 times before the first AJAX call returns).

I think I've got a fix, but it's late and I want to do a little more testing; but should be able to do a PR in morning.

Thanx!!!

[azaozz]

Another UI thing that may need considering/fixing:

Seems a lot of repetition, also some inconsistency?

• The table column shows the auto-updating setting status, but seems it should show it for both enabled and disabled or not show the status.
• Is all the repetition really needed?
• Is the additional repetition in the link's aria-label needed? Seems screen readers will get something like: "Automatic updates, Auto-updates enabled, Disable auto-updates, Disable automatic updates for Classic Editor".

[azaozz]

At one point, we were displaying the "recycle" dashicon all the time...in which case the shift wouldn't happen.

Yeah, having an icon there would be a good way of fixing the "jumpiness". Perhaps show dashicons-update without spin, and just add spin while waiting for the response?

Another option, perhaps, would be to show the spinning dashicons-update after some delay, perhaps 1 second? That at least won't make the jumping so bad.

[audrasjb]

Yeah, having an icon there would be a good way of fixing the "jumpiness". Perhaps show dashicons-update without spin, and just add spin while waiting for the response?

That was the UI we used to have in previous version of the plugin, but it was removed after Design team’s review.

Another option, perhaps, would be to show the spinning dashicons-update after some delay, perhaps 1 second? That at least won't make the jumping so bad.

Sounds good to me, even if most of to time, the dashicon won't shows as the action should be completed before. But that's not a big deal in my opinion, since the update happened :)

Is the additional repetition in the link's aria-label needed? Seems screen readers will get something like: "Automatic updates, Auto-updates enabled, Disable auto-updates, Disable automatic updates for Classic Editor".

I'll add a PR to fix this issue, thanks for pointing it out :)

[pbiron]

Here's my proposed solution to the first problem (the jumpiness):

1. don't have the Auto-updates enabled text above the link when auto-updates are enabled...it's redundant (since the link will be Disable auto-updates
2. display the time of the next auto-update (when one is available) below the link

The video is a little "choppy" because I reduced the frame rate to keep the file size smaller.

Note that the text while the Ajax is processing is slightly different as well: it is simply Enabling.../Disabling... (i.e., no auto-updates). I think that makes it more clear that something is happening because it is easier to see that the text has changed.

[audrasjb]

@pbiron this approach looks good to me 👍

[audrasjb]

@azaozz FYI, I removed the redundant aria-label in #109.

[azaozz]

Note that the text while the Ajax is processing is slightly different as well: it is simply Enabling.../Disabling...

Yeah, if the network connection is fast the text changes happen far too fast for the user to be able to read the text. Can be seen easily on a test install (local) where the text changes to Enabling... for about 1/4 of a second. Not a huge deal but this leaves the users wandering what they just missed.

On the other hand it's nice to change the text while waiting for the AJAX response... Perhaps can look into refining/enhancing the UX here by adding a "minimal display time" to the changed string? Slow it down just enough to be able to easily read it, perhaps 2 seconds?

Then thinking that if the string changes, and is easily readable, perhaps it wouldn't need to show the spinning icon?

[azaozz]

Another enhancement there is to block subsequent AJAX requests while waiting for the first request to complete. (Should be relatively easy to do by adding an "external" var on click and resetting it on .always().

[pbiron]

With the exception of #94 (comment), everything else mentioned in this issue is included in 0.7.0.

So, I'm closing this in favor of #115 (which covers that one remaining suggestion).