Opened 5 months ago

Last modified 11 days ago

#23120 new enhancement

There should be indication that widget settings have been saved

Reported by: jacopo.vip Owned by:
Priority: normal Milestone: Future Release
Component: Widgets Version: 3.5
Severity: normal Keywords: ui-focus has-patch needs-testing
Cc: jacopo.tarantino@…, ADAMSILVERSTEIN@…, mdhansen@…, catalin.dogaru@…

Description

There's currently no confirmation that adding a widget or changes to a widget have been saved. In some cases(fast hosting and a solid internet connection) the changes are saved so quickly the spinner is barely on the screen at all. I think a best course of action would be to have the word "Saved" appear briefly after the spinner goes away. Or perhaps a check mark as that doesn't need to be translated.
Related post on make.wordpress.org: http://make.wordpress.org/ui/2013/01/02/this-morning-i-ran-2-users-through-some/

Attachments (15)

23120.search-widget-settings-ru_RU.png (5.3 KB) - added by SergeyBiryukov 5 months ago.
widgetpatch.diff (1.2 KB) - added by adamsilverstein 5 months ago.
adds a checkmark after spinner on widget save & reorder
23120.patch (1.7 KB) - added by SergeyBiryukov 5 months ago.
23120.diff (2.2 KB) - added by cdog 4 months ago.
no-2x.png (2.1 KB) - added by cdog 4 months ago.
yes-2x.png (1.5 KB) - added by cdog 4 months ago.
23120.2.diff (2.2 KB) - added by adamsilverstein 2 months ago.
refresh
23120.3.diff (3.3 KB) - added by adamsilverstein 2 months ago.
uses uploader-icons for images
23120.4.diff (2.4 KB) - added by cdog 2 months ago.
spinner-icons.png (3.7 KB) - added by cdog 2 months ago.
spinner-icons-2x.png (4.5 KB) - added by cdog 2 months ago.
spinner-icons-alt.png (3.7 KB) - added by cdog 2 months ago.
spinner-icons-alt-2x.png (4.3 KB) - added by cdog 2 months ago.
23120.5.diff (2.4 KB) - added by adamsilverstein 3 weeks ago.
update against current
23120.6.diff (2.3 KB) - added by adamsilverstein 2 weeks ago.
update against trunk

Download all attachments as: .zip

Change History (37)

I think a best course of action would be to have the word "Saved" appear briefly after the spinner goes away. Or perhaps a check mark as that doesn't need to be translated.

A check mark seems better to me.

In ru_RU, there's no place for the string: 23120.search-widget-settings-ru_RU.png, so it would have the same problem as in #21890.

  • Keywords ui-focus added
  • Cc ADAMSILVERSTEIN@… added

comment:4 follow-up: ↓ 6   adamsilverstein5 months ago

i have attached a patch that i believe addresses this issue

i added a class to wp-admin.css to show the yes.png checkmark bundled in /wp-admin/images

i changed the hide() action in widgets.js when the save is complete, instead i add the new class and initiate a 1 second delay() with a half second fade at the end bringing opacity to 0. finally, a callback resets the opacity to 1 and completes the hide() in the original code.

note! you can't just call delay(1500).hide() or the element gets hidden immediately (hide is not added to jquery's fx queue). the callback is required to have the hide actually happen after the delay. although the fade is not required i kinda think it looks nice.

also, i changed the spinner for dragging and resorting the widgets - it now also shows the checkmark briefly.

tested on mac chrome and firefox

Last edited 5 months ago by adamsilverstein (previous) (diff)

adds a checkmark after spinner on widget save & reorder

  • Keywords has-patch added

comment:6 in reply to: ↑ 4   jacopo.vip5 months ago

Replying to adamsilverstein:

i have attached a patch that i believe addresses this issue

i added a class to wp-admin.css to show the yes.png checkmark bundled in /wp-admin/images

i changed the hide() action in widgets.js when the save is complete, instead i add the new class and initiate a 1 second delay() with a half second fade at the end bringing opacity to 0. finally, a callback resets the opacity to 1 and completes the hide() in the original code.

note! you can't just call delay(1500).hide() or the element gets hidden immediately (hide is not added to jquery's fx queue). the callback is required to have the hide actually happen after the delay. although the fade is not required i kinda think it looks nice.

also, i changed the spinner for dragging and resorting the widgets - it now also shows the checkmark briefly.

tested on mac chrome and firefox

I think that's a perfect solution. Good work :)

great, glad you like what i came up with! i'm new to contributing patches here, so thanks...

seems like there might be other places it could be used in the interface, but i haven't hunted. running locally sure makes the issue easy to spot :)

  • Cc mdhansen@… added

Tested and works as described. I think it is a good solution, no text or translation needed.

comment:9 follow-up: ↓ 11   SergeyBiryukov5 months ago

23120.patch:

  • Removes the class after hiding the element, otherwise the spinner image no longer appears on next click.
  • When saving the order after dragging widgets, only displays the spinner in sidebar header (as it is now), not in each widget.
  • Adds the widget object as a context to the selector, in order to only animate the spinner in the current widget.
  • Milestone changed from Awaiting Review to 3.6

comment:11 in reply to: ↑ 9   adamsilverstein5 months ago

Replying to SergeyBiryukov:

23120.patch:

  • Removes the class after hiding the element, otherwise the spinner image no longer appears on next click.
  • When saving the order after dragging widgets, only displays the spinner in sidebar header (as it is now), not in each widget.
  • Adds the widget object as a context to the selector, in order to only animate the spinner in the current widget.

looks good. i had added the class removal on my end, but you beat me to it. plus your patch adds the widget context to the selection which makes sense, thanks.

cdog4 months ago

cdog4 months ago

cdog4 months ago

comment:13 follow-up: ↓ 14   cdog4 months ago

  • Cc catalin.dogaru@… added
  • Keywords needs-testing added

attachment:23120.diff adds two new classes, .success and .error, to use with spinners. Their use is to indicate if the request was completed succesfully or not. Personally I don't think the use .delay() is really necessary. Fading out slowly the success/error icons should be sufficient. Added also support for the HDPI versions of yes.png and no.png images.

Would be nice to merge with ticket:22839 for a better standardization of spinners.

comment:14 in reply to: ↑ 13   adamsilverstein4 months ago

Replying to cdog:

attachment:23120.diff adds two new classes, .success and .error, to use with spinners. Their use is to indicate if the request was completed succesfully or not. Personally I don't think the use .delay() is really necessary. Fading out slowly the success/error icons should be sufficient. Added also support for the HDPI versions of yes.png and no.png images.

Would be nice to merge with ticket:22839 for a better standardization of spinners.

great improvements. needs merge with #22839 and i think similar success/error code across all spinners.

refresh

23120.2.diff

  • refresh agains trunk
  • copy attached images into wp-admin/images

comment:16 follow-up: ↓ 17   helen2 months ago

NB: Not actually testing the patch just yet.

Those yes/no icons are ugly, don't match the admin, and make me super sad. We've got way better ones in the wp-includes/images/uploader-icons*.png sprite, although you can't go guessing at where those are located so we'd need to copy them (or put a version of them) into wp-admin/images. If the coloring doesn't seem right, JerrySarcastic should be able to help us there, as he created those icons, or anybody handy with the source, which is located in the design repo: http://design.svn.wordpress.org/production/wp-includes/images/

comment:17 in reply to: ↑ 16   adamsilverstein2 months ago

thanks for the review. my original patch referenced the existing yes.png and no.png files in wp-includes/images. the new images from @cdog are the retina versions and upon review look very similar to the original versions.

the sprite images you referenced are b&w, is that preferable to the color icons we are using now? it seems like green check and red x include color to help make the action clearer, but if they don't match the rest of the interface b&w might make more sense.

if the images are already in wp-includes/images it seems like we can reference them directly? or is there a reason to copy them over to wp-admin instead of in wp-includes? i can rework the css to use the appropriate images, just let me know what you think is best!

Replying to helen:

NB: Not actually testing the patch just yet.

Those yes/no icons are ugly, don't match the admin, and make me super sad. We've got way better ones in the wp-includes/images/uploader-icons*.png sprite, although you can't go guessing at where those are located so we'd need to copy them (or put a version of them) into wp-admin/images. If the coloring doesn't seem right, JerrySarcastic should be able to help us there, as he created those icons, or anybody handy with the source, which is located in the design repo: http://design.svn.wordpress.org/production/wp-includes/images/

Version 0, edited 2 months ago by adamsilverstein (next)

comment:18 follow-up: ↓ 19   helen2 months ago

We can't guess at where wp-includes is located from a CSS file in wp-admin. I don't know if B&W is preferable to color in this instance, but those yes/no icons are ugly, period, and are only used in one place - insert from URL in the old media Thickbox that doesn't even appear by default anymore. If a different color is appropriate (or size), then like I said, I'm sure Jerry or another person handy with Photoshop can help out with that.

uses uploader-icons for images

comment:19 in reply to: ↑ 18 ; follow-up: ↓ 20   adamsilverstein2 months ago

Replying to helen:

We can't guess at where wp-includes is located from a CSS file in wp-admin. I don't know if B&W is preferable to color in this instance, but those yes/no icons are ugly, period, and are only used in one place - insert from URL in the old media Thickbox that doesn't even appear by default anymore. If a different color is appropriate (or size), then like I said, I'm sure Jerry or another person handy with Photoshop can help out with that.

i knew there was a reason, thanks for explaining :)

the attached 23120.3.diff​ uses the suggested uploader-icons files, to get it working just copy the images from wp-includes/images to wp-admin/images. after reviewing i like the b&w fine, no need for color really - the shape says it all. i did not get a chance to test this on a HiDPI display, but it should work according to my calculations.

cdog2 months ago

cdog2 months ago

cdog2 months ago

cdog2 months ago

cdog2 months ago

comment:20 in reply to: ↑ 19   cdog2 months ago

Replying to helen:

I don't know if B&W is preferable to color in this instance, but those yes/no icons are ugly, period.

Agreed. They were used as placeholders while writing the patch.

Replying to adamsilverstein:

the attached 23120.3.diff​ uses the suggested uploader-icons files, to get it working just copy the images from wp-includes/images to wp-admin/images.

Thanks for keeping the patch up to date. :)

Please check: attachment:23120.4.diff. After applying the patch, the spinner-icons (grayscale) or spinner-icons-alt (color) must be placed in wp-admin/images. The icons are based on uploader-icons.psd. If the shapes and/or colors need correction, I can provide the PSD files. Personally, I prefer the grayscale version. It fits better with the existing UI, even if the color version is more suggestive.

update against current

23120.5.diff updates 23120.4.diff against current

update against trunk

  • Milestone changed from 3.6 to Future Release
Note: See TracTickets for help on using tickets.