search

WordPress / wp-autoupdates

Feature plugin building a UI for opting-in to plugin, theme, and core auto-updates.
https://wordpress.org/plugins/wp-autoupdates/
GNU General Public License v2.0
56 stars 18 forks source link

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

Closed azaozz closed 4 years ago

azaozz commented 4 years ago

This is somewhat related to https://github.com/WordPress/wp-autoupdates/issues/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. Screen Shot 2020-04-30 at 14 15 13

  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...". Screen Shot 2020-04-30 at 14 16 34

pbiron commented 4 years ago

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.

  1. 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 commented 4 years ago

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):

pbiron commented 4 years ago

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 commented 4 years ago

Another UI thing that may need considering/fixing:

Screen Shot 2020-05-01 at 13 28 31

Seems a lot of repetition, also some inconsistency?

azaozz commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

ajax

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 commented 4 years ago

@pbiron this approach looks good to me 👍

audrasjb commented 4 years ago

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

azaozz commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

With the exception of https://github.com/WordPress/wp-autoupdates/issues/94#issuecomment-624342035, 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).