Avatars, an extensible UI

[imath]

At the end of the description of #6278, i've talked a bit of this idea : having an extensible UI for avatars so that we can provide various ways of setting an avatar :

There can be 2 ways provided by core :

• Upload an image
• Take a picture using the device camera #5548 (if getUserMedia() is supported by the browser)

We could have tabs for each ways. When i've upgraded the BP Avatar Suggestions plugin, i would have loved to "extend" such an UI, to add a new tab :)

Of course, it's still a work in progress :) So far i use :

• wp-backbone
• plupload for uploads
• imgAreaSelect for the cropping part

I'm able to upload the avatar, crop it and set it for the user. Associated to the Attachment API we are working on, i think it would be an interesting enhancement.

What do you think ?

[hnla]

Does the Pope...

This looks amazing, just to be able to upload and crop, but to be able to use a device camera would be super exiting, this was something that was always a concern when users presented with fait acomplie in having to upload an avatar and not having one available.

[johnjamesjacoby]

Yes, please.

[johnjamesjacoby]

Tangentially related, ​#WP21195 allows us to simplify our Avatar implementation, maybe pretty significantly. While we are working on a new upload & crop UI, and BP_Attachments, maybe this is worth a look too.

[sooskriszta]

@imath Super work.

I have a question though.

Since you are developing the attachments API which does much of the same work as WordPress Media Manager, why make a huge deal about the *Profile Photo* at all?

What I mean is, of course, there should be a profile photo. But do we really need to upload and store it separately? Shouldn't the member be able to mark any of the photos in his/her library/albums as a profile photo? A bit like how in photo gallery software, you can mark which photo to make the album cover...basically while you are on a photo, if you are the owner of that photo, you see a link - "make this my profile photo"

[DJPaul]

+1

[imath]

I made some interesting progress about this. If we get #6278 in for 2.3. I think it can be interesting to have this UI in for 2.3 also.

[imath]

@sooskriszta

The Attachments API is not a gallery/album component. It's a library to help components to manage uploads.

[imath]

I've made a video to demonstrate the progress of my work on this ticket :
​https://vimeo.com/122254715

If my english is a torture to hear, here's a shorter one with no sound :)
​https://cloudup.com/crCB3Yn0cnx

Finally here's a screen capture of the 'Camera Tab'

[johnjamesjacoby]

So awesome. This is a great addition.

[imath]

​6290.01.patch is my suggestion to provide a new UI for Avatar management. In the following lines, i'm trying to explain the different choices i've made.

The client side

Javascripts

You will quickly see i'm using Plupload, but not wp-plupload.js. The problem with the WordPress Plupload JS code is that the models for the queued files or the uploaded files are based on WordPress Attachments model wp.media.model.Attachment. That's really too bad because i personally think a file shouldn't be that early typed as a WordPress attachment.

• With the BP Attachments plugin i had the opportunity to see that it's forcing us to completely override wp.media
• With #6004 we figured out using the Media Editor in front end wasn't that great (eg: imgAreaSelect is only enqueued in wp-admin)
• Enqueueing wp-plupload.js enqueues all the media models and we don't need them all ! (It might have changed in WordPress 4.2 with the use of a require() function but i'm not familiar enougth with it to be sure see ​WP28510)

So i came to the conclusion, we should build our own Plupload file : bp-plupload.js. It looks like wp-plupload.js but differs essentially when typing files. Files have a very generic model: they are just files (url, name, loaded..) not WordPress Attachments or even BuddyPress Attachments. It's also providing some minimal Backbone views so that any plugin can use it and get the Drag and drop UI, some views to show upload progress, errors or warnings. All these views can of course be easily overriden if they don't match the need.
The other difference is i've added a specific check in it to be sure that if more than one file is dropped in the UI although the multi_selection Plupload parameter is set to false a warning will inform only one file can be added. Which in the case of Avatar is useful :)

Then we have avatar.js. This script requires bp-plupload.js and imgareaselect (for the cropping part) and extends the file model received from the plupload script by adding some BuddyPress specific attributes (eg: object, item_id, nonces, cropping datas). It's in a way supercharging the existing UI. At the top of the file, you'll see i check for some selectors to eventually remove the legacy UI. This way if javascript is disabled or if for some reason there's a javascript error, the legacy UI will act as a fallback.
This file is building the tabs, the cropping and delete views and is triggering a specific event when one of the nav is clicked. This way other scripts can listen to it and display there custom views.

That's what is doing webcam.js. It listens to bp.Avatar.nav.on( 'bp-avatar-view:changed' ) to set the device camera capture views. As this UI is new i've added a bunch of warnings to help users get familiar with it.

For instance, as soon as it loads, the browser is displaying a prompt to authorize the use of the webcam, to be sure people will look at it i've included a specific warning.
This script is using getUserMedia. Not all browsers are supporting it see ​this table. In case the browser is not supporting it a warning message will inform the user of the missing support of his browser. In short, if you want to be sure to use the "camera" avatars, use the Chrome browser.

Css

avatar.css requires imgareaselect. For this first patch, i've included in it all the rules i've needed for the UI. But i think :

• css rules can be improved
• some rules might be better in buddypress.css (eg: the ones concerning the uploader for instance)

hnla's review of this part + the following one will be greatly appreciated :)

Javascript templates

For the javascript templates of the Media Editor, WordPress is using a unique file wp-includes/media-template.php. This means every template are loaded, no matter if you are using it or not. For our javascript templates, i'm suggesting to take benefit of our template hierarchy and to use a more modular approach. A template file for the Uploader and a folder for avatars specific javascript templates (index/crop/camera).
At first i thought let's put it in bp-legacy or another template pack. On the other hand, this would restrict the Uploader to the themes using the built in template pack. So i thought we should make the BuddyPress Attachments template parts available for all situations (BP Default, standalone BuddyPress themes). That's why i've created the bp-attachments subdirectory in bp-templates.
Theme back compat is important, so i've built a small trick to :

• maximize the inclusion of the javascript templates by hooking some existing actions in the legacy templates
• display a doing it wrong in case the "avatar templates" are outdated and are not using the new template tag bp_core_avatar_get_template_part()

The server side

Of course, i'm using the BP Attachments API Introduced in the #6278 ticket. There are specific Plupload localization functions to include our BuddyPress parameters in the Plupload strings or settings.
The most important function is bp_attachments_enqueue_scripts(). It requires the name of the class extending the BP_Attachment one and is in charge of setting the script data. For instance BP_Attachment_Avatar will populate these datas adding required parameters: the action and the file_data_name, specific attributes for avatars (object, item_id, nonces) and the required registered script or css. This function is acting a bit like wp_enqueue_media().

I've also added a bunch of Ajax functions to set, crop, delete the avatar. And included specific parts for the groups component (Yes: the new UI is working for groups avatar management :) ), leaving filters and actions for any other components requiring avatars (eg: blavatars).

2 filters can be used to eventually :

• disable the camera feature : bp_attachment_avatar_use_webcam
• disable completely the new UI : bp_core_avatar_is_front_edit_avatar

The name of the functions, selectors, code format can surely be improved. I think it would be great if we could include this UI in 2.3 now that we have the BP Attachments API in. 'Front' Uploading user files is from my point of view lacking in WordPress, BuddyPress could begin to bring a very interesting reply with this ticket.

Finally, let's have some fun while you're testing the patch! Let's share our "selfie" avatars thanks to the "camera" part of the patch! Here's mine :)

[slaFFik]

This is huge.

[henry.wright]

Probably worth adding this link, just for reference:

​http://caniuse.com/#feat=stream

IE and Safari seem to be the only web browsers that won't play nicely. Chrome, FireFox and Opera have full support for the feature.

[boonebgorges]

Hey imath - I have had an initial look at the patch, and it looks great. Bravo!

Regarding bp-plupload.js: This strategy looks right to me. Yes, it's a shame that wp-plupload is not easier to extend, but it's better to reproduce code in BP than to resort to fragile hacks on core code.

I love your system for overridable Backbone templates - much more elegant than WP's. I'd suggest that bp-templates/bp-attachments is not the right place for them - I would expect each subdirectory of bp-templates to be a template pack, which bp-attachments is not. Something like bp-core/attachments/templates/ or bp-core/templates/attachments/ seems better to me.

The trick you're doing in bp_attachments_get_plupload_l10n() is very clever, but I think it's unwise. We can't depend on WP to maintain backward compatibility with these strings - or especially with the string handles when run through wp_localize_script(). It's a bit of extra work for translators, but these strings should just be included in BP.

The l10n tricks in the AJAX callbacks will have to be improved as well. String concatenation of the $message_part variety will not work. My initial thought here is that each BP_Attachment extension - like BP_Attachment_Avatar - should register user feedback strings, and AJAX requests should use these feedback strings to build their return values. It's OK to have generic fallbacks, but they should be very general: "Could not upload file" or whatever (with no concatenation/sprintf).

I have not looked over the Backbone stuff in detail, but in general the approach looks awesome to me. I especially like that you've made it fairly easy for new attachment plugins to inherit significant pieces of UI.

I'm not posting my selfie because the computer I'm currently on has all webcams disabled </tinfoilhat> :)

Thanks for your work on this! I agree with slaFFik that this is huge.

[imath]

Thanks a lot for your feedback boonebgorges.

I was hesitating about the best place for the javascript templates. Maybe because they are templates, i've put them there. But i agree that putting them elsewhere has a sense considering bp-templates as the specific place for template packs.

It's a bit of extra work for translators, but these strings should just be included in BP.

Fine with me and i understand your point.

should register user feedback strings, and AJAX requests should use these feedback strings to build their return values.

I get the idea, as we are using the BP_Attachment extensions just when needed, i wonder if we should simply use a function like BP_Attachment->script_data() (or even use this function) to fetch the feedback messages or fallback to generic ones into localization data during bp_attachments_enqueue_scripts().
It would look like :

feedback_messages: {
   1 : 'Profile avatar successfuly uploaded',
   2 : 'Uploading the profile photo failed'
}

Ajax would simply return a feedback code like 1 or 2.

I'm not posting my selfie because the computer I'm currently on has all webcams disabled </tinfoilhat> :)

ahaha :)

[imath]

​6290.02.patch is :

• applying the suggestion boonebgorges made about bp_attachments_get_plupload_l10n() : use our own strings instead of relying on wp_plupload_l10n ones
• applying the suggestion boonebgorges made about improving the way Ajax feedback messages are set. It's now happening in the BP_Attachment_Avatar()->script_data() function and each case (group profile photo, or user profile photo...) is no more a result of a concatenation/sprintf operation.
• fixing a bug hnla discovered testing the camera feature due to a device resolution lower than mine. Now we are checking the videoWidth and videoHeight to be sure it's upper than 150 and the capture dimensions is now dynamically built.
• making sure the canvas style will not be applied a max-width (twentyfourteen theme was messing the layout) < thanks to hnla for this one!

I haven't changed anything about the location of the javascript templates, as i understood with yesterday's dev-chat that we still need some time to decide where they should live.

[imath]

In ​6290.03.patch:

• i'm moving the attachments templates into bp-legacy
• i'm adding the Avatar UI in wp-admin / Extended profile
• I've tested in BP Default and it's also working, bp-legacy's template stack seems to act as a fallback if BP Default is not including the template ( < needs to be checked !) see ​this screen capture

About the wp-admin / Extended profile a new window will be opened on the Edit the profile photo link in the Profile photo metabox :

[DJPaul]

My Backbone isn't good enough for me to feel confident giving any code review on any of that, so I'm skipping it.

When this started, I was envisaging a system more like the group extension or widget system, where you can easily add new screens. It looks like all this Backbone templating has been added mainly because that's how Plupload has been implemented into WordPress Media Library?

If Jetpack wanted to add a BP Profile Pictures module that would let people choose/upload new Gravatars to WordPress.com/Gravatar.com, can they do that with this system? If so, would the Gravatar upload option appear alongside the traditional upload form, and the new web camera form?

---

I love your system for overridable Backbone templates - much more elegant than WP's.

@boonebgorges - what does WP do? I don't know anything about WP's Backbone.

empty()

This is not specific to this ticket, but the recent increase in mis-using empty() is annoying. ! empty() is the same as doing an isset and x != '' check. In this patch, for example, we have:

function bp_attachments_enqueue_scripts( $class = '' ) { 
	// Enqueue me just once per page, please. 
	if ( did_action( 'bp_attachments_enqueue_scripts' ) ) { 
		return; 
	} 

	if ( empty( $class ) || ! class_exists( $class ) ) { 
		return new WP_Error( 'missing_parameter' ); 
	}

An empty $class string will evaluate to false, so let's just use ! $class. :)

There's also a few type insensitive comparision functions in this patch (== instead of ===), let's not get sloppy.
And I will give you a miilion euros if you please do not add any more @uses phpDoc into BuddyPress, the most useless of all phpDoc tags. I will open a ticket to petition for removal of them, eventually. :)

bp_attachments_get_plupload_l10n()

Are the strings copied from WordPress core?

i18n

In bp_attachments_enqueue_scripts, in a user-facing string, we refer to "avatars". We decided to use "Profile Photo" (or something) instead of avatar a couple of versions ago (like you've done in other parts of the patch). :)

bp_core_avatar_is_front_edit_avatar

There are probably other funtion names in this pattern, but I don't like this one. Are you trying to "namespace" everything inside core and an avatar categories? Why not just "bp_avatar_"?

bp_core_avatar_template_check

I don't think we should do this. Throwing PHP Notices for user feedback is lousy. I recall someone saying this somewhere already, but I can't find it on the Trac ticket -- maybe it was on Slack? Not sure.

bp_is_my_admin_profile

Needs unit test. I think the "are we in wp-admin/users.php" check should be simplified by using is_admin etc and get_current_screen.

[imath]

Replying to DJPaul:
Thanks a lot for your feedback DJPaul

It looks like all this Backbone templating has been added mainly because that's how Plupload has been implemented into WordPress Media Library?

I don't think new bp_subnavs in the user's profile, or the group create/manage screens would be great. We are doing only one action: setting an avatar, why this action would need more bp_subnavs, page load...
Using widgets would require to edit the templates to add dynamic sidebars and we would be mixed into all other WordPress widgets in the widgets admin screen.

I personally think there are some areas where BuddyPress would benefit from using (more) Backbone, uploads is one, i can see the activity stream also, the group invites, improving the organization of our javascripts...
Back on avatars, the great benefit is that you can "carry" the UI very easily at various places e.g.: the wp-admin/Extended profile but it can be interesting to also carry the BP Uploader UI into the compose message screen, a plugin dealing with files can also carry this UI in its area...

1. If WordPress chose Backbone and underscore to manage media, i would trust them about the quality of their choice :)
2. BP_Attachment API brought the server part, this is the client part, plugins will be able to extend, share some enhancements.. :)

I took a lot of precautions on how this UI is added to the page:

• if for some reason there's a javascript error/ javascript is disabled, the bp-legacy markup will still be available
• i've included filters to completely remove the UI on front end.

So the risk is very limited and i strongly feel we need to go this road to avoid giving the impression we fear new territories.

If Jetpack wanted to add a BP Profile Pictures module that would let people choose/upload new Gravatars to WordPress.com/Gravatar.com, can they do that with this system? If so, would the Gravatar upload option appear alongside the traditional upload form, and the new web camera form?

First, this would be great!! And yes of course :)
It's possible to add/restrict tabs using this filter : bp_attachments_avatar_nav.
Once done they would simply need to add a js file a bit like the webcam.js one to listen to the tab changed event and load their content. I'll add a new filter in BP_Attachments_Avatar->script_data() so that it'll be easier to include new js scripts. They can also manage the order of the tabs. For instance they can be before the camera tab adding this kind of code using the above filter :

$nav[] = array( 'id' => 'jetpack', 'caption' => __( 'Jetpack', 'jetpack' ), 'order' => 5 );
return $nav;

They would finally need to add a javascript template by hooking to bp_attachments_avatar_main_template. I've coded this part having in mind my plugin BP Avatar Suggestions so that it could easily add a new tab.

I love your system for overridable Backbone templates - much more elegant than WP's.

@boonebgorges - what does WP do? I don't know anything about WP's Backbone.

in short: WP is using a ​single file for all templates.

empty() && === && @uses

I'll work on this asap, thanks for your recommandations.

bp_attachments_get_plupload_l10n(). Are the strings copied from WordPress core?

In the first patch i was dynamically getting the _wpPlupload strings, but @boonebgorges recommended, even if it was giving some extra work to translators, it was safe to use our own strings. So i've copied the strings, edited some, added some and got rid of one.

i18n. In bp_attachments_enqueue_scripts, in a user-facing string, we refer to "avatars". We decided to use "Profile Photo" (or something) instead of avatar a couple of versions ago (like you've done in other parts of the patch). :)

Good point, thanks a lot, i think it's mainly occurring when i'm setting the feedback messages for the camera, i'll correct these.

bp_core_avatar_is_front_edit_avatar. There are probably other funtion names in this pattern, but I don't like this one. Are you trying to "namespace" everything inside core and an avatar categories? Why not just "bp_avatar_"?

That's surely because many functions in bp-core-avatars.php are starting this way. I really don't mind using "bp_avatar".

bp_core_avatar_template_check. I don't think we should do this. Throwing PHP Notices for user feedback is lousy. I recall someone saying this somewhere already, but I can't find it on the Trac ticket -- maybe it was on Slack? Not sure.

Ok. I was simply trying to help theme authors to update their templates. I really don't mind removing it.

bp_is_my_admin_profile. Needs unit test. I think the "are we in wp-admin/users.php" check should be simplified by using is_admin etc and get_current_screen.

Really too bad Ajax is not setting the current screen, but i see what you mean. I'll work on this too.

[hnla]

bp_core_avatar_template_check. I don't think we should do this. Throwing PHP Notices for user feedback is lousy. I recall someone saying this somewhere already, but I can't find it on the Trac ticket -- maybe it was on Slack? Not sure.

Ok. I was simply trying to help theme authors to update their templates. I really don't mind removing it.

@DJPaul You may be thinking of a slack mini discussion a while back where I posed the notion that we could perhaps start doing something interesting with version stamping templates as a means to being able to check for outdated templates, where there had been template updates. I recall vaguely throwing up notices not being favoured, but in general the kernel of the idea had some merit to be perhaps explored. I also discussed at WCLDN with imath.

Not sure that this is what bp_core_avatar_template_check refers to but I wouldn't chuck the idea out completely just yet and we do have precedence for posting notices as we do so if BP pages become disassociated.

[imath]

In ​6290.04.patch, i'm first applying DJPaul's recommandations :

1. ! $class instead of ! empty( $class ) in bp_attachments_enqueue_scripts()
2. use === instead of ==, get rid all @uses phpDoc tags.
3. use 'profile photo' instead of 'avatar' in the camera strings
4. use 'bp_avatar_' instead of 'bp_core_avatar_' in all added avatar functions
5. remove the notice in bp_avatar_template_check()

"Deuxio" i'm removing the wp-admin / Extended profile edit avatar feature, because i'm not comfortable with a workaround i've used. Loading templates in the Administration seems to be "forbidden" for now. In bp_locate_template() there's a check on this constant WP_USE_THEMES, and in Administration screens, this constant is not defined or set to true. So i was forcing it to be true. I'm not sure we should do that... Anyway, this feature can be implemented later, once we get the front part in, as it's very easy...
i'm also removing bp_is_my_admin_profile(), because i removed the wp-admin / Extended profile edit avatar feature :) and i don't think we should use this kind of functions. I think we should introduce some capability check instead.

That's what i've done in bp_attachments_current_user_can(). On a side note, it would be great if we could progress on #5121. But in the meantime, this function will help us to check an 'edit_avatar' capability on front-end and in back-end.

"Tertio" DJPaul's Jetpack point made me realize we had a touchy part to deal with: at the top of bp_core_avatar_handle_upload() we're allowing a complete bypass of the function and we simply stop the upload process returning true if:

false === apply_filters( 'bp_core_pre_avatar_handle_upload', true, $file, $upload_dir_filter)

It appears Jetpack is using this filter to disable Photon for BuddyPress Avatars see ​https://github.com/Automattic/jetpack/blob/master/3rd-party/buddypress.php. So:

1. we need to make sure this filter is also in the new bp_avatar_ajax_upload() function so that Photon won't play whith an ajax uploaded avatar.
2. If false is returned to this filter, we need to send an error if the plugin is not giving the required infos about the uploaded avatar.

Jetpack is using this filter to know if an avatar is uploaded, but is returning the bool unchanged. Do we know about plugins/themes playing there and returning false (/me is praying it's not the case!) ?

[georgestephanis]

Yup, our changeset tracks back to ​https://github.com/Automattic/jetpack/commit/e2800d0e6 -- happy to change how we're doing it to accommodate a new pattern in BP.

Issue History:
​https://github.com/Automattic/jetpack/pull/57
​https://wordpress.org/support/topic/jetpack-tiled-mosaic-gallery-not-loading-all-images-1?replies=41

[imath]

thanks a lot @georgestephanis i'll keep you updated if we need to change anything in this area. But i think it won't be the case.

[imath]

Tested the patch filtering the avatar dimensions and realized:

• i forgot to make the cropping preview and the camera preview dynamic.
• when deleting an avatar the gravatar fetched was not a full size one.

6290.05.patch is fixing these 2 points.

[imath]

I'm sorry. Maybe i need to take a break with this ticket!

My fear about the filter 'bp_core_pre_avatar_handle_upload' is silly. As the function bp_avatar_ajax_upload() is anyway using bp_core_avatar_handle_upload(), the filter will be fired, so :

• no potential problem with Jetpack at all :)
• no need to add a new filter like it was the case since 04.patch.

Adding 6290.06.patch to remove the unnecessary code added since 04.patch about my silly fear :)

[DJPaul]

I would still like clarification to these questions I asked previously.

When this started, I was envisaging a system more like the group extension or widget system, where you can easily add new screens. It looks like all this Backbone templating has been added mainly because that's how Plupload has been implemented into WordPress Media Library?
.
If Jetpack wanted to add a BP Profile Pictures module that would let people choose/upload new Gravatars to WordPress.com/Gravatar.com, can they do that with this system? If so, would the Gravatar upload option appear alongside the traditional upload form, and the new web camera form?

[imath]

DJPaul: of course, i'm sorry my comment ​here wasn't clear enough.

When this started, I was envisaging a system more like the group extension or widget system, where you can easily add new screens. It looks like all this Backbone templating has been added mainly because that's how Plupload has been implemented into WordPress Media Library?

When i've started this ticket, as i was mistaken on #6004, i've carefully re-read what was the ​plans and ideas for the future of BuddyPress :

Attachments

API improvements around handling media (specifically avatars) are long overdue.
No appetite for a “photo album” component in core at the moment, but our new APIs should be built to let other plugin developers build one “easily”.
We want to overhaul/rebuild the user avatar upload process/cropper/management etc as a first step for a UI change directly related to media. Clean-Cole has begin to do some refinement based on prior work by Mathieu Viet.

I clearly understood: the work around Avatars in 2.3 was the beginning of a BuddyPress API for Attachments other plugin developers could use to build their great tools. #6278 is the server part, this is my suggestion for the client part. Not only plugins playing around avatars can add their tab to the Avatar UI, but plugins can use the BP Uploader UI part to deal with file uploads.

I'm surprised you were envisaging a different system, as you commented +1 ​here and the description of the ticket was clearly informing i was planning to use :

• wp-backbone
• plupload for uploads
• imgAreaSelect for the cropping part

---

If Jetpack wanted to add a BP Profile Pictures module that would let people choose/upload new Gravatars to WordPress.com/Gravatar.com, can they do that with this system? If so, would the Gravatar upload option appear alongside the traditional upload form, and the new web camera form?

I understand your concern about JetPack, maybe you know a lot more than i do about their work on the subject. I've tried to search into their github repository something about it, without success :(
JetPack is a very important plugin, i guess many people are activating it right away once their WordPress is installed. So i think it (would / ) will be totally awsome (if / ) when they will make this feature available.

I hardly see how to be more clear about what i've already explained into the 5th paragraph of this ​comment. I doubt explaining it in french would help :) So i thought, let's demonstrate it.

If you apply ​6290.06.patch, clone ​JetPack and then apply ​this patch on JetPack, then you'll see that :

• JetPack will be able to let people "choose/upload new Gravatars to WordPress.com/Gravatar.com"
• It will also appear "alongside the traditional upload form, and the new web camera form".
• we can eventually build a router if we need to arrive directly to their tab using a link.

1. JetPack will be great for users avatars, but BuddyPress is also dealing with other objects, so to me it's not 'THE' solution, it's another great way to set a user's avatar just like local uploads, camera....
2. JetPack seems to be very familiar with underscore or BackBone, i saw they are enqueueing media-views for instance for their "site-logo-control" module.
3. I'm very confident on each "JetPacker" skills which must be much greater than mines :)

---

I think we have an opportunity to build something greater than an Avatar UI using Backbone, Plupload... :

• Edit User, Group, (Blog) Avatars within the front-end or the back-end,
• Provide a front-end tool plugin developers can use and extend to deal with user uploads.

If you think i had a bad idea investing a lot of my time into this, of course i might be disappointed and frustrated, but i'll understand and move on to another ticket, because BuddyPress is awesome.

For now, i will take a break from this ticket and will wait for a "stop or go on" :)

[boonebgorges]

Whoa whoa whoa whoa whoa.

We have to use JavaScript to do things like native camera capture. This means using some kind of modal dialog, or else a Backbone-ish dialog embedded in a page. I think imath went with the modal option because it closely aligns with what WP does for the Media Library. I think this is a very good option, but if we don't like it, we can always change it in a future version. Let's not stop work on this ticket for this reason.

Regarding extensibility: In the long run, we want plugins to be able to add their own versions of the uploader, ideally with a minimal amount of work. imath has already demonstrated that it's not too difficult, and in the future I imagine that we'll discover ways to make it even easier. However, let's remember that we are first building something very specific: a replacement for our avatar upload system. Let's build something *really great* for that purpose for 2.3. In my mind, extensibility is a secondary concern for the moment - it goes without saying that we don't want to preclude the possibility of extending the UI, but we should first worry about getting it right for our particular use case. We can always improve the API in future versions.

[DJPaul]

I think there's been some misunderstanding. I'm sorry if my questions contributed to that. This new feature is super exciting, because it's the first time in about three or four years that anyone has significantly change the avatar upload feature. :)

I was asking the question because it's functionality that I have wanted inside a BP for a long time (uploads to Gravatar), so I wanted to check that we were building these new screens in such a way that it would be possible to build that kind of thing. I find asking questions like this often helps to make sure we are building things in a modular, extendable way, rather than a bespoke, inflexible, one-off solution.

@imath, I appreciate the time you spent building an example for how other developers could add their own avatar management panels. Thank you. You've allayed my concerns, and proved the merits of this new approach you have built --- and also written most of an example for a future codex page. :)

I asked about JS/Backbone specifically because this ticket came fully-formed, with a patch. I was probing to see if "we" considered the implementation strong by asking questions about the technical approach. Sometimes tickets start with technical discussion before much code is written, and sometimes vice-versa -- I was trying to get my head to understand your perspective on this matter in order to hopefully provide useful feedback; not start an argument. :)

[imath]

@DJPaul,

Thanks a lot for your comment. I guess i may have overreacted a bit, must be a french thing :)

I was very much into this ticket and i understood the first part as "we're doing it wrong". (Added 2 patches yesterday - the second one to correct an unjustified fear - was shopping with wifey when received the mail : this last element was very stressfull!)

I'm not sure it's the right translation, but there were no bad feelings and is no hard feelings :)

The JetPack workaround made me realized we can improve some filters by adding some contextual data like are we setting an avatar for a user or a group or something else..

We will surely discuss about it during tonight's dev-chat, these are the parts i think we need to find the best approach on :

• localization of the javascript templates : bp-legacy is ok as it acts as a fallback if no template were found in active theme and we are avoiding some hooks on theme compat to register new template stack(s).
• Organization in bp-legacy (if definitive destination), in a discussion with @hnla he was wondering if having /assets/attachments would be an interesting one if in the future we need to do things for other areas. I think it's an important choice to make.
• Templates : name of selectors/classes, available do_actions (and names of them...)
• Styles : avatar.css is including all the needed rules for the UI, should we keep it this way? Keeping it this way is interesting considering next point, but it can also be interesting to split it to have uploader rules in a file and specific avatar ones (crop / camera) in another one.
• Administration screens : 03.patch was bringing the UI into the wp-admin / extended profile. I've removed it because bp_locate_template() is doing a check on WP_USE_THEMES and i'm not sure defining this constant to true in Administration is the right move. Maybe include a new filter/constant in bp_locate_template() is better. A filter could be toggled on/off before and after our need to get template parts.
• Displayed user in Administration screen and AJAX : the Displayed user Is dynamically get each time we need it so far (if wp-admin/profile.php or user-edit.php?user_id=ID) and in AJAX no current screen is set, reason why i'm using since 04.patch a minimal capability check ( 'edit_avatar', array( 'object' => '', 'item_id' => 0 ) ). I think the capability check is better for the future as attachments will possibly be attached to post types or any object actually!
• Other possible improvements : name of filters/actions/functions and things i might have missed :)

Finally @boonebgorges, thanks a lot for your comment :)

[imath]

I would like to thank all the core team members for their contributions, and also apologize (again) for the time i requested during the last two dev-chats. Thanks to your help, i'm now strongly convinced and very confident about the best approach about where to put the attachments templates:

• @johnjamesjacoby was right since the beginning! All BuddyPress templates are in bp-legacy. We must keep it this way, even if in the case of attachments these are Backbone templates, because Backbone templates are templates :)
• @hnla is right. In bp-legacy we should put the templates in a new directory assets. Because one day or another, we'll need assets about other BuddyPress features.
• @r-a-y is right. We should prefix the attachments directory with an underscore to visualy inform that Core wants to keep the control over this part and as a result, there's no need to try to override it from the theme or a custom template dir.

So the best approach is bp-legacy/buddypress/assets/_attachments/

Directly in _attachments/ we have the shared templates between various BuddyPress features. For instance, uploader.php is used for the Avatars and need to also be available for any component (eg: messages).
In _attachments/avatars we have all the templates relative to the Avatars.

For the introduction of the new Avatar UI, our need to keep a "core control" over the Backbone templates must be resolved in BuddyPress Theme Compat, but not by adding new template stacks. Adding these in bp-core for example will simply add three extra overridable locations. Meaning that in the case of a "not existing" template part: three extra locations will be checked, which is not the end of the world but clearly unuseful.

I've opened this new ticket #6348 to suggest another strategy to keep temporarly this control over the needed features or permanently in the case of BuddyPress Administration screens.

​6290.07.patch is requiring ​6348.01.patch and:

• applies the described approach,
• brings back the Avatar UI in wp-admin/extended profile
• improve the UI navigation: there's no more the need to refresh the page to make the "Delete" nav item appear.

[DJPaul]

I am trying very hard to not start trivial discussion over names of things, but one I do want to talk about -- because I think it is important -- is bp-legacy/buddypress/assets/_attachments/.

I know most of this code grew from your original "Attachments API" ticket but this is different from that, I don't see why this directory should be called attachments. I believe it will confuse people used to dealing with template files for attachment post types in WordPress. It also doesn't describe the purpose of the templates contained within it at all. Let's just call it /bp-legacy/buddypress/assets/_avatars/.

[imath]

Replying to DJPaul:

Let's just call it /bp-legacy/buddypress/assets/_avatars/.

it wouldn't be "juste" actually :)

I don't see why this directory should be called attachments

Because we are "visionnaires", we can predict the future :)

More seriously, here's how it's organized now :
assets/_attachments
assets/_attachments/uploader.php < this template can be used by any component or feature requiring a file upload (not just Avatars).
assets/_attachments/avatars/index.php < the main avatar template file that is using the above file and the following ones
assets/_attachments/avatars/crop.php < the cropping template
assets/_attachments/avatars/camera.php < the camera template

Here's how it could be after a few releases :
assets/attachments
assets/attachments/uploader.php
assets/attachments/avatars/index.php
assets/attachments/avatars/crop.php
assets/attachments/avatars/camera.php
assets/attachments/_messages/index.php
assets/emails
assets/emails/signups/activation.php

What you suggest is:
assets/_avatars/uploader.php
assets/_avatars/index.php
assets/_avatars/crop.php
assets/_avatars/camera.php

imho it's not the best organization. It means the uploader will only be used for avatars forever. But I understand your concern. If the name _attachments is annoying, let's use something else.. What about _files ?

I guess we can do assets/_avatars/ for the introduction of the feature of course, but we'll need to change it as soon as we will use uploads in another component.

[DJPaul]

Ok, whatever.

[imath]

Since #6348 is invalid. I'll need to update the patch.

[imath]

Updated the patch to version 08.patch

• This version creates a new function bp_attachments_get_template_part() to load the Backbone templates only from bp-legacy/buddypress/assets/_attachments if into an Administration Screen, else use bp_get_template_part() to let theme override the templates.
• Fixes a bug with Thickbox by resetting all views if the TB_Window has been closed during the setting an avatar process.

Haven't changed anything in the template organization since 07.patch.

[imath]

Apart from concerns related to where to put the templates, how to name the folder, how to load the templates...

These last 2 days i had a fight with Internet Explorer 8, testing the patch in it. In the end, i don't know if i should kill him or kiss him!

So ​6290.09.patch is bringing compatibility with IE8, improving our BuddyPress plupload file and revealing a problem in the bp_core_check_avatar_type().

How works plupload ? You set the available runtimes in your script, that's what we're doing in bp_attachments_get_plupload_default_settings() and the library will use the first runtime available. So far as i've tested in Chrome/Safari and Firefox, i was always getting the html5 runtime. But with my tests in IE8, once i've resolved some javascript issues (that was the most painful 2 days of my coding life!), i've found that, as we are falling back on the flash runtime, the bp_core_check_avatar_type() keeps on returning a file extension error despite the file is a jpg, png or gif image. I then checked in Chrome by disabling html5 and the same issue showed up.

Here's a little summary of my tests with supported image files :

Runtime	bp_core_check_avatar_type()
html5	success
silverlight	success
flash	error

Reason: in flash the mime type is 'application/octet-stream' and not preg_match('/(jpe?g|gif|png)$/i', $file_type ) ). This is not happening when you rely on WordPress to upload your file because WordPress uses wp_check_filetype_and_ext(). Hopefully, thanks to DJPaul there's a ticket (#6336) about this, so i just added a new version of ​the attached patch and it's solving the issue in IE8 (actually with flash).

• I still need to improve the javascript strategy in case the plupload runtime happens to be html4.
• I still need to check IE versions that are supporting html5 uploads eg: 10 & 11. Thanks to hnla we confirmed yesterday that 6290.08.patch was failing in these two browsers. I hope 6290.09.patch will fix the trouble in these, else i guess i'll need to virtualize WIN/IE !

[imath]

Excellent new!

6290.09.patch is working fine in IE10, just virtualized WIN8/IE10 on my computer :

Upload View

Crop View

[imath]

Just a quick update to inform @hnla is confirming .09.patch is behaving the right way on IE10 & IE11. Many Many Thanks to him for testing it :)

[imath]

With patch 6290.10.patch, i think i'm close to have a patch ready to candidate for a first commit.

I've fixed a trouble with the html4 plupload runtime using a new function (bp_attachments_json_response()) to build the json responses.

• Chrome is ok (Uploads/Camera)
• Firefox is ok (Uploads/Camera)
• Safari is ok (Uploads)
• Internet Explorer 11, 10, 8 are ok (Uploads)

I've just quickly build a live testing environment and checked what was happening using my iPhone.

• Safari iOS : html5 Upload is failing, html4 is ok, but the cropping part is failing
• Chrome iOS : html5 Upload is ok, html4 is ok, but the cropping part is failing

jCropper is not great either on iPhone, but let's say is half working.

I'm going to send to each member of the team a login and a password to access to this testing environment (i may ping you on slack if i don't have your email). You'll see you'll be able to test the legacy UI / this suggested UI (customizing the plupload runtimes) from the settings component of your profile. I've added shortcuts to easily toggle between changing the avatar and setting the UI prefs so that it will be easier for you to test.

Watch your mails, you should receive your login/password from wordpress[at]imath-buddypress.wpserveur.net

i thank you in advance for your help, and don't hesitate to test on any device (iPad, Android Phones/tablets...)

[imath]

​6290.11.patch is:

• fixing html5 Uploads in iPhone
• replacing the imgAreaSelect library by Jcrop. We can't use imgAreaSelect because it's not supporting touch devices. Jcrop is, so Jcrop it will be!
• improving the javascripts (many thanks to grunt jstest)

I've updated the test drive with this version of the patch. I personally think, this patch is ready to be committed!

[imath]

Following latest dev-chat, this is 6290.12.patch, changes are :

• Disable the camera tab for mobile devices to avoid confusions,
• Make sure the option 'bp-disable-avatar-uploads' is not set to true before loading any Avatar UI, in front or back end.

I've updated the test site. I will now work on splitting the patch in smaller parts and will commit these parts in the coming days if no objections.

[imath]

In 9754:

Avatar UI: Add specific stylesheet and javascript files and register WordPress styles and scripts

• Edit bp_core_register_common_scripts() in order to be able to enqueue scripts into the footer.
• bp-plupload.js (handle: bp-plupload) is depending on plupload and wp-backbone and is the BuddyPress adaptation of wp-plupload.js. When a file is uploaded it builds some BackBone collections and views to manage the user feedbacks, the back and front end Upload UI.
• avatar.js (handle: bp-avatar) is depending on jcrop and is responsible of building the extensible Avatar UI navigation tabs and the Crop view.
• webcam.js (handle: bp-webcam) is depending on bp-avatar and is responsible of building the Camera view after checking the bowser supports getUserMedia to capture the video stream.
• avatar.css (handle: bp-avatar) is depending on jcrop and is building the styles for the complete Avatar UI.

Props hnla.

See #6290

[imath]

In 9755:

Avatar UI: Add Conditional functions and new BP Attachment methods

• bp_avatar_is_front_edit() checks if the user is editing his or a group avatar on front-end
• bp_avatar_use_webcam() checks if the Camera feature should be enabled. By default, it is not the case for touch devices as it can introduce some confustions with the possibility of using the camera of the device to take a selfie, save it as an image an upload it using the Browse button of the Avatar UI. Chrome and Firefox are the only browsers supporting getUserMedia.

NB: these two functions can be filtered to disallow the Camera feature or to completely disallow the new Avatar UI if you prefer to carry on using the legacy UI.

BP_Attachment::script_data() and BP_Attachment_Avatar::script_data() are two new methods to build the javascript localization data. The bp_attachment_avatar_params filter is fired if the current object is not a group or a user and can be used to build the params of another object such as a blog. You can also use the filter bp_attachment_avatar_script_data if you need to override data for any object.

Props boonebgorges, DJPaul.

See #6290
See #6278

[imath]

In 9756:

Avatar UI: Add BuddyPress Attachments specific functions.

• bp_attachments_enqueue_scripts() requires one parameter the name of the class you are using to extend BP_Attachment, eg: BP_Attachment_Avatar. It will use the class script_data() method to finish building the Plupload settings and strings. It also makes sure the script are only loaded once per page load.
• bp_attachments_current_user_can() is used to check the current user capabilities in front-end and back-end.
• bp_attachments_json_response() is a wrapper of WordPress json response function that makes sure the json response is correctly sent for html5 or html4 uploads.
• bp_attachments_get_template_part() is used to safely load our Backbone templates into the administration screens or use bp_get_template_part() when on front-end.

Props boonebgorges, DJPaul, johnjamesjacoby.

See #6290

[imath]

In 9757:

Avatar UI: Add Backbone templates and adapt the legacy avatar templates.

The Backone templates will be in a new "assets" sub directory of bp-templates\bp-legacy\buddypress. For the introduction of the feature we are advising you *to not override* these specific templates from your theme. The underscore in each sub folder names of this "assets" directory is informing we wish to restrict the content of the sub folder to Core use only.

We are introducing a new template tag to load these Backbone templates : bp_avatar_get_templates(). We recommand that you update the avatar templates of your theme if you are overriding groups/create, groups/single/admin or members/single/profile/change-avatar once 2.3 will be released. If this new template tag is not into the template, then bp_avatar_template_check() will try to load them using template hooks.

Props johnjamesjacoby, hnla, r-a-y.

See #6290

[imath]

In 9758:

Avatar UI: Add Ajax functions to upload/crop/capture and delete avatars. Enqueue the needed Avatar scripts and css.

bp_core_avatar_scripts() will enqueue the scripts only if editing an avatar on front-end and if the user avatar uploads are not disabled. The hooks to maximize backward theme compatibility are localized in this function.

See #6290

[imath]

In 9759:

Avatar UI: use it in wp-admin/extended profile.

Clicking on the "Edit Profile Photo" link from the avatar metabox will open a thickbox window in which the Avatar UI will be displayed.

See #6290

[imath]

In 9760:

Improve bp_core_get_allowed_avatar_types(). Make sure to get the file type if the upload was not done using html5

The way we used to check the image file type is not working if the browser is Internet Explorer < 10. In this particular case, the Plupload runtime is falling back to flash and the file type is application/octet-stream. We are now using wp_check_filetype_and_ext() to fix this issue.

You can use the filter bp_core_get_allowed_avatar_types if you wish to *restrict* the avatar image types. Our supported types are: jpg, jpeg, png and gif.

Props DJPaul.

Fixes #6336
See #6290
See #6278

[hnla]

Update avatar.css: ad vertical centering for crop pane upload message/button. Minor re-factoring to ruleset formatting.

[imath]

Just tested it and i'm not convinced :)

Using chrome.

The upload view looks nice. But the crop avatar view and the camera view are not great at all. The preview seems broken on the crop view and on the camera view it's at the bottom..

See screencaps : ​https://cloudup.com/c6sv1m4uShM

PS: this UI can be loaded anywhere (backend, user's profile, group's manage/create profile photo)

[hnla]

@imath I'll check that out now.

[imath]

Great Thanks a lot for your work on it ;)

[hnla]

That is in fact simply the overflow property on avatar-crop-pane, which needed to be re-thought, but this initial pass was only meant to address formatting and the drag-drop-inside centering. I should have re-instated the overflow until the second pass re-factored the approach.

I'll correct this in .02 with hopefully something that will serve better to allow us to manage the positioning of the crop-actions buttons, also might need to add some media queries in as we'll run into problems with responsive themes that have content areas of a narrower width than the widths we have to keep in regard to things like the avatar-to-crop fixed width on video element.

I'll aim to have css.02 up later today with some updates.

[imath]

In 9785:

Avatar UI: clean up the Camera Backbone template

The template was using a "hide" class for the save button view with no corresponding css rule. As the strategy is to display the button and eventually inform the user to first click on the "capture" button if the "save" button was first clicked, we are removing this class.

Props hnla for the good catch.

See #6290

[hnla]

Second pass at avatar styles, breakpoints & jcrop correction

[hnla]

6290.css.02 works up breakpoint views for the new Camara capture feature.

Jcrop upload screen is trickier and has an initial pass at improving the views in mobile up but suffers from it's inherent flaws in original design, evident in existing use, this pass improves but isn't perfect.

There are still areas to deal with and this styling does rely on a new wrapper div for the crop functions.

[hnla]

Adding a patch for templates which adds a new wrapper element around the 'avatar-crop-pane' & 'avatar-crop-actions' this helps to manage layout positioning in only having to manage two elements with the new one containing and restraining the child elements.

[hnla]

Update avatar templates

[imath]

Thanks a lot hnla, 2 patches are almost perfect :) There's this little detail we talked about in slack (button alignment in the camera view of wp-admin/extended profile)

[hnla]

Currently updating the admin modal views.

Also re-defined the avatar constants to check how things handle with varying sizes defined by users (I tend to define a new default as a matter of course up at around 250px as it allows more flexibility in sizing ) as things stand the difficulty here is that with horizontal implementations of the two main elements we need to float both and manage their widths explicitly, if one of these elements has an unknown width or width that may vary this becomes problematical, in these circumstances one would tend to not float one of these elements so it could simply clear the unknown width, a classic floated layout is one floated the other overflowed to force a new box context to clear the floated item.

There are two choices I may need to address element order in the templates to ensure floated elements are flowed first or we could try and get tricky and pass the new constant widths as a class token for floated element or parent or pass dimension properties to the style attr inline on the element, but that's as far as I've thought it through thus far - re-ordering elements and adjusting rules is probably best option.

Edit One option I almost forgot about is that we don't actually allow the avatar-crop-pane to change in dimensions, after all it's a functional item that really just needs to represent the aspect ratio but doesn't have to show the final actual size, restricting this elements dimensions is the easiest option.

[imath]

I think changing the default avatar size is not a good idea because many avatars on websites will be 150x150 and when fetching avatars, they will all look pixelized in the member's header for instance. And this is a real problem i think.

I'm not sure it's just the aspect ratio for the avatar crop pane as when cropping an avatar we are also sending width & height values.

Honestly if you are concerned about these aspects because of the wp-admin/extend view only, i think we should first fix the button alignment in the camera view. I've tested your improvements emulating an iPhone and having the crop preview under the image is ok for me, as it's the way it is today - so no regression. Maybe for this kind of devices we can add a warning to invite users to turn it so that they are viewing the site in landscape mode.

[hnla]

Update styles for WP Admin Modal & groups uploader, enhance profile screens

[hnla]

• Patch .css.03 addresses WP admin extended user profile modal window
• Updates groups 'photo' screen for jcrop
• Improves user account profile avatar views

We are, with these styles, in a somewhat Damned if you do, damned if you don't type scenario

We are attempting to manipulate element layout on a media query basis while also attempting to style these elements for a variety of themes (twenty* at this stage). Intrinsically this is and always will be a flawed if not impossible task, media queries were never the answer to responsive layouts given they simply denote a point where the browser viewport hits a certain width but can't describe or determine how the elements within that viewport are laid out and to what widths.

We will have to accept that compromise is inevitable in trying to achieve perfection some themes/layouts will work better than others.

The Camara views by and large perform nicely across a range of themes and media breakpoints for mobile tablets etc (proof testing in real devices essential though).

Sadly the existing jcrop image cropping is not and never has been suited to responsive layouts and is extremely difficult to manipulate without breaking functionality, it's here that we will have to accept a large amount of give and take, but we never have had a particularly great layout for this screen.

There are still areas that will require fine tuning so testing and feedback is needed now. Testing in the more irksome browsers (yeah you know who you are) is still to be done.

[hnla]

Update styles for improved handling of upload screen where avatar default sizes changed by user.

[imath]

Thanks for your work on this new patch. Tested in twentytwelve/twentyfifteen so far :
1/ camera view is fine no matter the avatar dimensions (150x150 or 250x250)
2/ wp-admin/extended profile upload/crop is fine no matter the avatar dimensions (150x150 or 250x250)
3/ in groups or user's profile on front end, the crop preview is always under the avatar to crop no matter the avatar dimensions (150x150 or 250x250). Everything is centered which is not the case in wp-admin/extended profile.

Too bad we can't have the preview at the right of avatar to crop although
avatar to crop + crop preview < width available :(

[hnla]

The admin modal window is a special case and responds slightly differently to the frontend screens where we have the avatar elements embedded to the layout. Modal windows have their own sets of issues, one of which being inflexible height, we have issues with larger crop-panes moving below but breaking out of the window and forcing vert scroll bars, so I attempt to keep things horizontally laid out for as long as possible until the viewport narrower width allows the window to grab sufficient height to let us move the crop-pane below.

This is the best iteration so far and best balance between issues presented, it's sad that jcrop defies manipulating it's fixed widths without collapsing the cropping ability, if we can gain control of that aspect we can lay things out horizontally at wider screen widths, but for the moment I would live with the stacked view for the upload screen,it's still an improvement on what we had )

I want to address the avatar menu styling, proposing that we copy what I did in the twentyfifteen companion styles for them - 'tabbed view style' -to either avatar.css or bp main styles, however would/does that tab style suit any theme generically, as it is somewhat a design element?

[imath]

Replying to hnla:

Thanks hnla for your great work so far. My feeling as i told you in private is : i'm afraid people won't understand if we are displaying the two elements the one under the other despite the fact there's enough width available to display them side by side.

I like how the UI is shapping for all other aspects, so i suggest we fix this litte annoying thing directly in javascript.

In avatar.js i get the width (0 to 450) of the avatar to crop (ac.w), the width of the parent (p.w), and the full width avatar constant (full.w), so i'm simply checking :
ac.w + full.w + 20 < p.w 20 is a margin i keep.

If we are in this case, i'm adding a specific class so that the layout is using all the available width, else the preview is going under.

So 6290.layout.01.patch is 6290.css.04 + 6290.tpl.01 + js workaround

I agree it's not the best way of dealing with this, but i think we need to have this to carry on and improve this in future releases.

I want to address the avatar menu styling, proposing that we copy what I did in the twentyfifteen companion styles for them - 'tabbed view style' -to either avatar.css or bp main styles, however would/does that tab style suit any theme generically, as it is somewhat a design element?

That would be terrific !! I really like the tab view you are using in twentytwelve. I think this should go in avatar.css

If you're ok, i'll update the test drive so that we can test on many browsers/OS.

[DJPaul]

If I have to click the Capture button before the Save button, why don't we only show the Save button after the Capture button has been pressed once??

Furthermore, when the nag message about using the capture button appeared, I did not notice it because there was already a big yellow block of text. I agree we need to show the initial message (user must accept Video in their browser), but for "Video stream loaded. You can use the capture button to display the profile photo preview" -- I think this is not needed because I see the video feed. What do you think about removing this message?

[imath]

Thanks for your feedback @DJPaul, my thoughts was as it's new let's drive the user to make sure he's not lost :) I need to remind why i ended up giving up the hide/show save button. But i had a good reason :) Will detail it asap.

[DJPaul]

This is the first time I've tested this code since the very early version. Something else I noticed immediately was the video was not flipped -- I moved my left arm, and Virtual Paul's "right" arm moved! Most streaming video services flip the video around to fix this.

6290.flip.01.patch is a quick effort to flip the video feed and image horizontally. My webcam.js changes don't work but I believe I have the right approach; I am pretty busy the next few days so I wanted to get an example up to show you.

[imath]

Thanks DJPaul, i agree. I'll try to understand why your patch doesn't work. The examples i've seen were dealing with images, the ones with video are not flipping the stream.

[imath]

@DJPaul i got it flipped ! :)

Thanks a lot for the suggestion, it's really better this way. I've added the flip thing with the layout ones, and will commit this patch tonight : 6290.layout-flip.02.patch

[imath]

In 9799:

Avatar UI: Improve templates markup and general layout

avatar.css has been improved to maximize a nice display of the avatar UI in latest WordPress default themes and devices. General appearence has been also improved especially the camera view.

Props hnla

See #6290

[imath]

In 9800:

Avatar UI: Improve the webcam capture

Flip the video and the video capture horizontally so that it is easier and more natural to use.

Props DJPaul

See #6290

[imath]

FYI just updated the test drive for this UI on ​http://imath-buddypress.wpserveur.net. Feel free to test :)

Tested on iPhone, Internet Explorer10 & 8 and Chrome. No problem for me :)

[johnjamesjacoby]

Thoughts on prefixing the progress class? It conflicts with Twitter bootstrap styling, and makes progress bars on the same page float all funky.

[imath]

I guess progress is widely use :) I suggest to use bp-progress & bp-bar see 6290.prefix.patch

[hnla]

It may also be wise or safer than sorry if we nest the classes under .bp-avatar in case we use .bp-progress elsewhere in the future?

[imath]

Maybe but i think .bp-avatar might not the good one progress "belongs" to the uploader.. I'll need to Check this out :)

[hnla]

Ok if 'progress' belongs to the uploader then it's rules ought to move to buddypress.css and avatar.css only manage any rules required for some specific styling requirement, sort of what we discussed vis a vis looking at styles and their location in files.

[imath]

Yes it should actually be the case :)

But for the introduction of the feature, we kind of restrict the _attachments usage, so i'm hesitating! I'd say let's use .bp-avatar and keep all styles in avatar.css and do some moves when we will clearly open the use of the UI to do something else than set avatars. What do you think ?

[hnla]

Ok, yes for the immediate interim I agree, we'll ensure nasties can't happen by using .bp-avatar to restrict the .progress properties and then as and when the element is required for other implementations we can pick the rules out and move over to buddypress.css and add whatever else required; sounds safe to me.

Good spot by JJJ though, easy thing to slip past, re-enforces the reason for as much testing as possible!

[hnla]

Move tabbed avatar nav styles out of companion styles to avatar.css (includes bp prefix for .progress/.bar)

[hnla]

css.05 patch re-works the tabbed nav styles to work from avatar.css rather than companion styles.

I have added the prefix changes manually to this patch, including the '.bp-avatar' parent selector.

The tabbed styles are removed from twentyfifteen stylesheet so there may be an overlap point where styles are missed if I commit 2015 sooner than this patch is applied.

[imath]

Thanks a lot hnla, will check this out this sunday :)

[imath]

Ok, tested it, tabs are great :) About prefixing, actually the uploader progress bar is not in .bp-avatar but in .bp-avatar-status. So i will use this parent. Will commit in a few minutes :)

[imath]

In 9805:

Avatar UI : improve the avatar nav and prefix the classes in the avatar status box

Props hnla, johnjamesjacoby

See #6290

[imath]

In 9808:

Avatar UI : avoid duplicate IDs in the delete view

Props hnla for the good catch.

See #6290

[r-a-y]

First of all, I love the new interface. Drag and drop is so much more user-friendly than trying to find an image in a file input box.

Secondly, sorry for getting to this late! Here is what I have in mind:

Users - Profile > Change Profile Photo:

Groups - Manage > Photo:

Although I love how the "Delete" functionality works when you remove a photo and dynamically switch out the current photo on the page, I think the "Delete" link is not as prominent as before.

What I've done is hide the "Delete" button from the Backbone avatar nav on the frontend and move the older "Delete" block above the uploader. The "Delete" button is still visible from the WP admin's "Extended Profile" area though.

I've tried to make this work well if JS is disabled and I think this works okay.

The .bp-uploader-window CSS in the patch is to fix this bug:

Would love to hear what everyone else thinks about this.

[johnjamesjacoby]

I think r-a-y is on the right track, though I'd like it to be even more prominent and obvious.

How about a little section to show what the actual current avatar is that's being deleted? It's helpful for comparing what you have and what you'll end up with when you're done.

[sooskriszta]

Replying to r-a-y:

Would actually be great if to change your profile photo, you dont have to navigate to Edit Profile etc. Instead, if while logged in you click on your profile photo then BuddyPress assumes you want to change your photo and takes you to that screen. If you clicked by mistake, there's always the Cancel button.

[r-a-y]

How about a little section to show what the actual current avatar is that's being deleted?

This was my first thought initially.

But like imath stated in last week's dev chat, the photo is already visible in the profile header. I understand that themes might have overriden the profile header so the photo isn't displayed on some pages, so maybe we should add a thumbnail of the current photo.

Would be happy to go with the majority here.

Instead, if while logged in you click on your profile photo then BuddyPress assumes you want to change your photo and takes you to that screen.

Sounds like a good idea, but we cannot do this for 2.3, sookriszta.

Update: Sorry, maybe we can do something like this. I thought you meant that a modal window would pop up similar to imath's screenshot in comment:19.

[imath]

@r-a-y @johnjamesjacoby Sorry, but i don't like this solution. It breaks the dynamic of the UI and all the delete code in the backbone UI is not used ??? Please wait for a more detailed feedback from me.

It's more a workaround than a real solution.

[hnla]

@r-a-y what theme is that that demonstrates the float issue?

[r-a-y]

It breaks the dynamic of the UI and all the delete code in the backbone UI is not used ???

ux.02.patch uses the backbone delete avatar code that imath wrote when the "Delete Profile Photo" button is clicked. My Backbone-fu isn't the greatest, but imath, let me know what you think.

what theme is that that demonstrates the float issue?

twentytwelve, the king of all themes.

[hnla]

@r-a-y

twentytwelve, the king of all themes.

Not for much longer :)

Odd I cannot replicate that issue in twentytwelve, or are you using some peculiar browser like Chrome :0

[imath]

@r-a-y :

Thanks for your suggestions, but again, i think it's not a great idea.

1. Since the beginning i've explained i would use a navigation to list all available avatar actions : Upload / Camera / Delete, i'm a bit annoyed this patch comes the last day before we will discuss about 2.3 beta.
2. Both patches are putting the delete action as the major action of the UI, above the others as if the most important thing was to delete an avatar.
3. The first patch breaks the dynamic part of the UI when a new avatar is set as the delete tab is no more showed, and you need to refresh the page to have the delete button above the UI
4. Your second patch is adding the button + the delete tab so the same action is duplicated.
5. "Or you can upload a new photo:" What about the camera ?
6. Both patches does not change anything into the wp-admin/extended profile, so in admin it's ok to not have a button to delete an avatar ?
7. Both patches are editing the templates moving some parts, meaning the experience will be different for BuddyPress standalone themes or BP Default. Here's the 2 ux patches in BP Default :

Nothing changes for BP Default..

I think the "Delete" link is not as prominent as before

I think r-a-y is on the right track, though I'd like it to be even more prominent and obvious.

Even if I have difficulties to understand this need to have a "prominent" delete action, i think the right way would be to add a simple message to help people find the third tab of the UI. But as i've explained in slack i think we then need to use the available Backbone views to do so that we maximize its display in themes like BP Default for instance or in the administration and preserve some consistency. Something like this :

But honestly the delete tab is enough. The most important thing in a community is not to delete an avatar. it's to have one :)

[r-a-y]

Hi imath,

Just going to reply to your points:

Your second patch is adding the button + the delete tab so the same action is duplicated.

I wanted to keep what you said about the nav in mind so I left the "Delete" tab alone in the second patch. The second patch is mostly to show that it is possible to use the Backbone delete code from outside the Backbone template.

"Or you can upload a new photo:" What about the camera ?

It was mostly placeholder text. However, my rationale is that not every computer will have a webcam (my desktop doesn't have one), so the "Camera" tab shouldn't show up ideally in this scenario.

Both patches does not change anything into the wp-admin/extended profile, so in admin it's ok to not have a button to delete an avatar ?

In my opinion, there is less of a user instructional emphasis on the admin dashboard's "Extended Profile" screen, which is why I left that alone. Plus, the avatar uploader shows up in a modal window, which changes the focus of the action.

Both patches are editing the templates moving some parts, meaning the experience will be different for BuddyPress standalone themes or BP Default.

Right, but we are not really supporting bp-default any more.

Something like this:
​https://farm8.staticflickr.com/7726/17119251799_e46c752c43_b.jpg

I think this works well and is a good compromise. Let's do this instead of what I was proposing. I'd prefer that the message is placed right below the nav, but that's just me. The color scheme of the message also doesn't match the existing colors we use for #message p, but now I'm just nitpicking ;)

My remaining issues have to do with the older text (not your fault) when the new uploader is shown below it:

Your avatar will be used on your profile and throughout the site. If there is a Gravatar associated with your account email we will use that, or you can upload an image from your computer.

We shouldn't be using the word "avatar" any more. Plus the "Gravatar" portion of this text should be removed as it doesn't make sense to leave it in when a profile photo exists. I would love this text to be dynamic based on certain situations, however these changes would require a string change in the template and I don't know how we all feel about that.

Also, about compatibility, can we change the following for the group "Manage > Photo" JS:
​https://buddypress.trac.wordpress.org/browser/trunk/src/bp-core/js/avatar.js?marks=66-70#L64

So it isn't reliant on the first paragraph? If a theme has added more than one paragraph here, then the existing code will not work well. $('p input').remove() would be safer.

---

Finally, boo Ray! Sorry for bringing up these suggestions just before beta. My plan was to look at this over the weekend, but I got sick and couldn't look into this until now. I'd rather add my feedback than not add any at all.

My intention was definitely not to deemphasis all the hard work you've done! If I did, then I apologize.

[imath]

Really sorry. Boo me too! I guess i lack some sleep, not to mention my nervosity about #6278 when WP < 4.0, but it looks to not be as problematic as i thought.

No problem. I wonder where i got these colors. I'll check & correct. I'll also check the first paragraph thing and will include the message status. Is the string ok ?
"If you'd like to delete your current profile photo but not upload a new one, please use the delete tab."

[imath]

First, there's a trouble!

WordPress was using Plupload 1.7 till 3.9. Since 3.9 it's using Plupload 2.1.1. I've tried all day to make the UI behaves the right way in Plupload 1.7 and couldn't make it. So unless we decide to drop support for WordPress 3.6 to 3.8, i really think we should disable the UI for these versions and fallback to legacy. See 6290.wp39.patch.

Now the user "warnings" : See 6290.warning.patch.

• Added a new warning to inform the user he needs to use the delete tab to delete his avatar
• Found that a warning needed to be output if the uploaded image had smaller dimensions than the avatar ones.
• Had to edit the avatar.css file : to 'match the existing colors we use for #message p' and to fix a display issue that appears when i'm adding the warning about the dimensions on the crop view (@hnla: it would be fantastic if you could check this!!)

[boonebgorges]

So unless we decide to drop support for WordPress 3.6 to 3.8, i really think we should disable the UI for these versions and fallback to legacy. See 6290.wp39.patch.

Yes. Since it's straightforward to do so, let's fall back to the legacy interface. We should start a list of places where we're doing things like this, so we can remove the checks when we've bumped our supported versions.

[hnla]

I wonder where i got these colors

In respect of colors these will be a BP aspect of styles, rules for being handled by the general .messages .notice rules in buddypress.css - which may be stating the obvious. In companion styles we have the chance to and have changed colors for something a little different.

(@hnla: it would be fantastic if you could check this!!)

I'll endeavour to in a while.

[hnla]

As discussed with imath the additional property rule to force containment should be ok.

On the question of messages classes and colors, I've said we ought to ensure appropriate BP message classes are used (avatar already does mainly) and rules for visual styling of these removed from avatar.css allowing buddypress.css to manage them.

I have though identified a need for additional classes .warning & .success along with suitable colors plus the existing class .info -currently un-styled - added to with a class ? notice-info only as that class may already be styled or not so don't want to upset established sites. These additional classes better reflect common usage and avatars already uses .warning

I propose opening a ticket and adding this for 2.3 as it's relatively straightforward and non invasive for current sites.

[imath]

Sure @hnla

Many thanks to everyone. I'm going to commit the last 2 patches and close this ticket tonight.

[imath]

In 9826:

Avatar UI : improve user feedback messages

• Display a message informing the user can delete his current avatar using the "Delete" tab of the UI.
• Eventually display a message informing the user the image uploaded have smaller dimensions than the avatar full dimensions.

Props r-a-y

See #6290

[imath]

In 9827:

Avatar UI : make sure the current site is using at least WordPress 3.9

To maximize browsers compatibility, the Avatar UI requires Pluload 2.1.1. This version of the script was introduced in version 3.9 of WordPress.
For WordPress versions 3.6 to 3.8, the legacy UI will be used.

Fixes #6290