[buddypress-trac] [BuddyPress Trac] #6290: Avatars, an extensible UI

[buddypress-trac]

#6290: Avatars, an extensible UI
------------------------------------+-----------------------
 Reporter:  imath                   |       Owner:  imath
     Type:  idea                    |      Status:  assigned
 Priority:  normal                  |   Milestone:  2.3
Component:  API - Avatars           |     Version:
 Severity:  normal                  |  Resolution:
 Keywords:  has-patch dev-feedback  |
------------------------------------+-----------------------

Comment (by imath):

 Replying to [comment:22 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 [https://core.trac.wordpress.org/browser/trunk/src
 /wp-includes/media-template.php 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.

--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/6290#comment:23>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac