WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 18 months ago

#16020 reviewing feature request

Upload custom avatar for user in Dashboard

Reported by: computerwiz908 Owned by: nacin
Milestone: Future Release Priority: normal
Severity: minor Version:
Component: Users Keywords: ux-feedback has-patch 2nd-opinion
Focuses: Cc:

Description

It would be nice to be able to upload a custom avatar for a user in the WordPress Dashboard rather than making each user sign up for a Gravatar account and upload the image to that account.

Attachments (11)

16020-options-discussion-avatars.jpg (55.1 KB) - added by cdog 22 months ago.
16020.diff (16.3 KB) - added by cdog 22 months ago.
16020-profile-avatar-default.jpg (9.3 KB) - added by cdog 22 months ago.
16020-profile-avatar-custom.jpg (32.5 KB) - added by cdog 22 months ago.
16020.2.diff (21.9 KB) - added by cdog 22 months ago.
16020-upload.jpg (35.5 KB) - added by cdog 22 months ago.
16020.3.diff (20.2 KB) - added by cdog 22 months ago.
16020.4.diff (21.2 KB) - added by cdog 22 months ago.
16020.5-profile-avatar-custom.jpg (24.8 KB) - added by cdog 22 months ago.
16020.5.diff (23.2 KB) - added by cdog 22 months ago.
16020.6.diff (23.2 KB) - added by cdog 19 months ago.

Download all attachments as: .zip

Change History (53)

comment:1 computerwiz9084 years ago

  • Keywords avatar custom upload removed

comment:2 jane4 years ago

  • Milestone changed from Awaiting Review to Future Release

Agreed. Requiring dependence on a private, hosted third-party service doesn't seem right.

comment:4 scribu2 years ago

  • Keywords needs-patch added

comment:5 sabreuse2 years ago

  • Cc sabreuse@… added

comment:6 cdog22 months ago

  • Cc catalin.dogaru@… added

comment:7 cdog22 months ago

  • Keywords ux-feedback added

comment:9 cdog22 months ago

  • Keywords has-patch added; needs-patch removed

comment:10 cdog22 months ago

attachment:16020.diff adds support for self-hosted custom avatars. Users are able to:

  • upload a new avatar image;
  • choose a rating for their custom avatar;
  • remove their current avatar image.

Users can switch between their custom avatar or their Gravatar by choosing back the Default option from the Profile page.

This patch works properly with JavaScript disabled too. The output can be previewed here: attachment:16020-profile-avatar-default.jpg, attachment:16020-profile-avatar-custom.jpg, attachment:16020-options-discussion-avatars.jpg.

comment:11 follow-up: scribu22 months ago

The patch also seems to allow custom avatar sizes.

I think the "Default" label is kind of confusing. Instead of this:

[ ] Default
[ ] Custom

Use Default to display your Gravatar. <a href="...">More Information</a>

we could have this:

[ ] Gravatar <a href="...">More Information</a>
[ ] Custom

Otherwise, the patch looks good. There's not much point using Thickbox instead of a simple file input here.

comment:12 in reply to: ↑ 11 cdog22 months ago

Replying to scribu:

I think the "Default" label is kind of confusing.

Agreed. I have updated the patch as you suggested.

comment:13 follow-up: scribu22 months ago

There are some inconsistencies here:

 	153	                        $user->avatar_type = 'gravatar'; 
 	154	                        $user->custom_avatar = array(); 
 	155	                        $user->custom_avatar_rating = 'G'; 

First, you allow the user to upload multiple custom avatars, but he can only set one custom_avatar_rating. I think it's best to only allow a single custom avatar.

Also, why is $user->has_custom_avatar = 'false'; set to the string 'false' rather than to the boolean false.

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

Replying to scribu:

First, you allow the user to upload multiple custom avatars, but he can only set one custom_avatar_rating. I think it's best to only allow a single custom avatar.

The user can upload only one custom avatar. Uploading another will replace the current one. The custom_avatar array contains the same avatar, but different sizes.

Also, why is $user->has_custom_avatar = 'false'; set to the string 'false' rather than to the boolean false.

Sorry for that. Fixed it.

comment:15 follow-up: scribu22 months ago

The custom_avatar array contains the same avatar, but different sizes.

Oh, I see.

Ideally, you should only need to store an attachment id, which could contain arbitrary sizes. See #15311

comment:16 in reply to: ↑ 15 cdog22 months ago

Replying to scribu:

Ideally, you should only need to store an attachment id, which could contain arbitrary sizes. See #15311

To use the suggested method an avatar should be inserted as an attachment which is not the case. Making the avatars visible in Media Library would let other users to alter not only their avatar.

cdog22 months ago

comment:17 follow-up: scribu22 months ago

Please don't overwrite patches that have been reviewed.

Making the avatars visible in Media Library would let other users to alter not only their avatar.

Non-administrators can only edit their own attachments.

comment:18 in reply to: ↑ 17 cdog22 months ago

Replying to scribu:

Non-administrators can only edit their own attachments.

And users without the capability to upload files to Media Library (like Subscribers) wouldn't be able to upload an avatar image then. Should the subscribers be limited on using Gravatar only?

comment:19 follow-up: scribu22 months ago

I think that's acceptable, as a first pass of this feature.

comment:20 DrewAPicture22 months ago

  • Cc xoodrew@… added

cdog22 months ago

comment:21 in reply to: ↑ 19 cdog22 months ago

Replying to scribu:

I think that's acceptable, as a first pass of this feature.

I've made some updates based on your feedback. Please check: attachment:16020.2.diff.

cdog22 months ago

comment:22 follow-up: scribu22 months ago

The avatar cleanup code is repeated in 3 places. Should make a helper function.

cdog22 months ago

comment:23 in reply to: ↑ 22 cdog22 months ago

Replying to scribu:

The avatar cleanup code is repeated in 3 places. Should make a helper function.

Cleaned up the previous submission. Please check: attachment:16020.3.diff.

With the current patch users have only one option: to upload a new avatar image. What do you think about adding support for choosing an existing image from Media Library?

comment:24 scribu22 months ago

I don't think it's a good idea to try something like that right now, given #21390.

16020.3.diff looks good, and works too.

Regarding UX, if a user makes some other changes to their profile, when they press the Upload button they'll loose their changes, right?

comment:25 scribu22 months ago

Nevermind, they won't.

The "Remove Image" button should be to the right of the Custom option, not below the rating and should probably be red and say "Delete".

comment:26 follow-up: scribu22 months ago

Oh, if you have a custom avatar and you go to /author/your-user/ in the front-end, using the Twentyeleven theme, you'll see the gravatar instead.

comment:27 in reply to: ↑ 26 cdog22 months ago

Replying to scribu:

The "Remove Image" button should be to the right of the Custom option, not below the rating and should probably be red and say "Delete".

Tried to keep the interface consistent (take a look at Appearance > Header / Background). If it's not the case, I'll follow your suggestion.

Oh, if you have a custom avatar and you go to /author/your-user/ in the front-end, using the Twentyeleven theme, you'll see the gravatar instead.

Sorry for that. Fixed it in: attachment:16020.4.diff.

Thank you for your help!

cdog22 months ago

comment:28 follow-up: scribu22 months ago

Tried to keep the interface consistent (take a look at Appearance > Header / Background).

It's not the same. The header takes up the whole width.

Also, that whole page is dedicated to the header, whereas on the profile page you have all sort of other things going on, so compactness is more important.

Last edited 22 months ago by scribu (previous) (diff)

comment:29 in reply to: ↑ 28 cdog22 months ago

I've updated the "Delete" link. Please check: attachment:16020.5.diff and attachment:16020.5-profile-avatar-custom.jpg for preview.

cdog22 months ago

comment:30 scribu22 months ago

Looks good to me. Let's see what some of the UX/UI people think.

comment:31 follow-up: johnbillion22 months ago

To me, this interface is confusing.

  • What is the 'Select Image' field for? Is this my avatar? Is this a fallback avatar for users without avatars? Is it a global avatar for all users?
  • If I switch between Gravatar and Custom (and vice versa), the explanatory text for Avatar Rating is no longer accurate.
  • It's not clear that the Remove Avatar Image button applies only to your custom avatar.

Okay I should have actually applied the patch instead of going by screenshots. This is the user profile screen, not the avatar section of the Discussion Settings screen.

I still think it's a little unclear that the Avatar Rating and Remove Image controls only apply to your custom avatar. This needs to be made clearer.

Last edited 22 months ago by johnbillion (previous) (diff)

comment:32 in reply to: ↑ 31 cdog22 months ago

Replying to johnbillion:

I still think it's a little unclear that the Avatar Rating and Remove Image controls only apply to your custom avatar. This needs to be made clearer.

The "Avatar Rating" controls becomes visible only when the user have a custom avatar uploaded. Also, the "Remove Image" section is not anymore there. It was replaced with the "Delete" link in the latest version of the patch (attachment:16020.5.diff) as you can see in: attachment:16020.5-profile-avatar-custom.jpg.

Thank you for your feedback and for testing the patch.

cdog19 months ago

comment:33 cdog19 months ago

attachment:16020.6.diff​ updates the previous submission making the patch ready to test against the latest revision (23184). Going to use the new wp_get_image_editor() instead of the deprecated image_resize() to get rid of the notices.

I want to add support for the new media manager but I think it would be better to wait until the ticket gets closed. Because the code is getting pretty big for a single patch I suggest to continue it separately. It would be easier to write (and review) and to avoid creating new bugs.

comment:34 nacin19 months ago

  • Keywords close added
  • Owner set to nacin
  • Status changed from new to reviewing

I don't think this is core material. Gravatar, despite it being "a private, hosted third-party service," is the industry standard. Suggesting close as maybelater, or wontfix.

comment:35 follow-up: scribu19 months ago

Erm, no; the industry standard is for each site to have their own user avatars. See twitter, facebook, etc. etc.

comment:36 scribu19 months ago

  • Keywords 2nd-opinion added; close removed

comment:37 cdog19 months ago

I agree with jane and scribu. Even if it's not an important feature it's requested by users (Add Local Avatar has reached more than 250K downloads and it's just one of the available plugins). Gravatar will be still the default option even if support for self-hosted avatar will be added. I think it's better to have this option rather than making users to register an alternate email address on Gravatar or install an additional plugin.

Version 1, edited 19 months ago by cdog (previous) (next) (diff)

comment:38 in reply to: ↑ 35 ; follow-up: rmccue19 months ago

Replying to scribu:

Erm, no; the industry standard is for each site to have their own user avatars. See twitter, facebook, etc. etc.

-1, I agree with nacin. I personally think this is firmly plugin territory.

The examples you noted are social networks, so they have their own avatars, whereas most other sites don't. See GitHub (which is basically a social network), StackOverflow (ditto), etc. (Note that these are technology-focussed sites, so that's slightly biased.)

comment:39 in reply to: ↑ 38 cdog18 months ago

Replying to rmccue:

The examples you noted are social networks, so they have their own avatars, whereas most other sites don't. See GitHub (which is basically a social network), StackOverflow (ditto), etc. (Note that these are technology-focussed sites, so that's slightly biased.)

The Envato marketplaces (ThemeForest, CodeCanyon etc.) are technology-focused sites too and they don't have support for Gravatar.

Thinking again the situation. Mixing both options (Gravatar and self-hosted avatars) doesn't seem right for core. Obviously, replacing Gravatar is not a good choice either. Perhaps moving it into a separate plugin would be the most appropriate solution.

Closing it as wontfix would be fine for me. But let's wait for nacin and scribu to confirm and take a decision.

comment:40 follow-up: scribu18 months ago

Maybe a better course of action would be to lobby the Gravatar developers (i.e. Automattic?) to release some sort of API which opens an iframe/lightbox/whatever for users to create/change their image, instead of having to go to Gravatar.com to do it, which is quite a confusing workflow.

comment:41 in reply to: ↑ 40 ; follow-up: beaulebens18 months ago

  • Cc beau@… added

Replying to scribu:

Maybe a better course of action would be to lobby the Gravatar developers (i.e. Automattic?) to release some sort of API which opens an iframe/lightbox/whatever for users to create/change their image, instead of having to go to Gravatar.com to do it, which is quite a confusing workflow.

You could load gravatar.com in a thickboxed iframe right now, we actually do something like that on WordPress.com. Signup would still be an issue though (and there are no plans to provide a signup API). Because we need to verify ownership of an email address before we can assign it to an account, there needs to be some involvement with Gravatar.com there.

We've talked about possibly making the signup flow easier to integrate with other (non-Gravatar) sites, but haven't come up with a good solution to that yet that still includes the verification step.

As a sidenote, you can link here to slightly improve the signup process: https://gravatar.com/site/signup/email%40domain.com

And check if a user has an existing Gravatar by requesting this, with the 0's replaced with the user's email hash: http://www.gravatar.com/avatar/00000000000000000000000000000000?d=404 (default = 404 response)

There is also an XMLRPC API for working with Gravatar, although to be honest it doesn't get much attention since I don't believe many folks use it. Obviously that would change if WP core started using it. http://en.gravatar.com/site/implement/xmlrpc/

comment:42 in reply to: ↑ 41 cdog18 months ago

Replying to beaulebens:

There is also an XMLRPC API for working with Gravatar, although to be honest it doesn't get much attention since I don't believe many folks use it. Obviously that would change if WP core started using it. http://en.gravatar.com/site/implement/xmlrpc/

Thank you for the information. I think it's a good approach to let users manage their Gravatar right from the profile screen. It would improve the workflow making it less confusing. Going to take a more in-depth look at the XMLRPC API to see what it can be done.

I've moved the current patch into a plugin, cleaned it up and documented the code. It's available on GitHub and WordPress Plugin Directory if anyone is interested in giving some help or just prefers a plugin to work with than hacking directly the core.

Note: See TracTickets for help on using tickets.