Option to disable Group avatars

[sooskriszta]

In Admin, it would be nice to have an option to disable avatars for Groups.

I'm assigning component Groups as opposed to Theme, as it is not just a question of showing the avatar. It is a wholesale question of uploading too - so group creation workflow, group edit options, etc.

[boonebgorges]

This may not warrant a UI toggle, but it's a good idea to make it possible with a constant or a filter.

[sooskriszta]

As a very non-tech-literate user, I always advocate for a bigger admin UI footprint for BuddyPress. The more easily folks can find and use features, the better it is...

[Prometheus Fire]

I agree that a UI toggle seems like a lot of extra work for this. The use cases for this need are limited and not worth having a UI switch, but a filter or define statement in wp-config.php would be great way to turn this off.

Not all BP instances (corporate intranet, for ex.) would need avatars for groups and being able to disable would be a good flexibility.

[sooskriszta]

A wp-config.php statement would be relatively more usable than just a filter.

[johnjamesjacoby]

No patch. Punting to 1.8.

[boonebgorges]

4677.patch is a bare-bones pass at implementing a toggle. To toggle, put the following in bp-custom.php:

add_filter( 'bp_enable_group_avatars', '__return_false' );

As you see, it requires some template-level changes. Not ideal, but it's the only way to do it.

Styling probably also needs some tweaks when avatars are absent, especially in the groups directory. Could put in a no-avatar class or something along those lines, or could try reworking the way that the markup and styling works so that the spacing naturally readjusts.

Anyone have thoughts?

[boonebgorges]

Punting this one. If anyone has feedback or suggestions, we can do it for 1.9.

[sooskriszta]

This ought to do it.

[xgz]

Hello.
I have done what was needed here, how to post updates for approval? What is the procedure?
Thanks.

[boonebgorges]

Replying to xgz:

Hello.
I have done what was needed here, how to post updates for approval? What is the procedure?
Thanks.

I'm not sure I understand your question - can you clarify?

I don't mind going forward with this change, though the template-level changes mean that for themes overriding the templates in question, the filter won't fully work. (Yet another argument for not adding a UI toggle.) @hnla, do you have any thoughts about styling adjustments that could be made so that the directory and header don't totally break when the avatar is missing?

[sooskriszta]

Replying to xgz:

I have done what was needed here, how to post updates for approval? What is the procedure?

Please do not change Priority and Severity of tickets without prior consent of core team.

You can read the process of submitting patches at ​https://bitbucket.org/prepzone3/buddypress-patches/wiki/Submit%20a%20patch%20to%20BuddyPress

[sooskriszta]

Replying to boonebgorges:

I don't mind going forward with this change, though the template-level changes mean that for themes overriding the templates in question, the filter won't fully work. (Yet another argument for not adding a UI toggle.) @hnla, do you have any thoughts about styling adjustments that could be made so that the directory and header don't totally break when the avatar is missing?

What he has done so far is create a toggle to enable/disable group avatars. The avatars already uploaded, and avatar placeholders do still appear in the theme, which I think defeats the purpose.

[hnla]

Replying to boonebgorges:

Replying to xgz:

Hello.
I have done what was needed here, how to post updates for approval? What is the procedure?
Thanks.

I'm not sure I understand your question - can you clarify?

@hnla, do you have any thoughts about styling adjustments that could be made so that the directory and header don't totally break when the avatar is missing?

I can certainly have a look, I'll perhaps wait to see if there is a viable patch though, as not 100% convinced by this as a core feature toggling group avatars, but if we do doubtless we can find a means of ensuring layouts hold up.

[xgz]

Adding options on / off loading avatar in the group and on / off the display on the pages.

[xgz]

Replying to sooskriszta:
Are you still testing our patch (4677.2.patch)?
Or do you need something from us?

[boonebgorges]

xgz - Sorry, this is a volunteer project - the maintainers can't always address incoming patches immediately.

As noted earlier, we will need to address the question of templates. Turning off avatar *uploads* means that default avatars will be displayed, with no way of changing them (aside from Gravatar). This does not seem like a good user experience. IMO, if you turn off group avatars, you should remove the avatars from the templates altogether.

[sooskriszta]

Replying to boonebgorges:

if you turn off group avatars, you should remove the avatars from the templates altogether.

I think that's what the patch does, though the UI language is sketchy and unclear.

[imath]

I'm adding myself to the notifications about this ticket as this could have an impact on the checks i'm doing in #6290 to load/not load the next/new Avatar UI.

[boonebgorges]

Replying to sooskriszta:

Replying to boonebgorges:

if you turn off group avatars, you should remove the avatars from the templates altogether.

I think that's what the patch does, though the UI language is sketchy and unclear.

Yeah, I guess you're right - we already do this when avatars are disabled altogether. My apologies for misreading.

4677.3.patch​ makes a few revisions:

• Instead of falling back on false when no value has been saved for 'bp-disable-group-avatar-uploads', fall back on the value of the general 'bp-disable-avatar-uploads'. This is for backward compatibility: anyone who currently has avatars disabled across the site will expect them to be disabled after upgrade.
• I added a very slight change to the stylesheet to make things look decent on group directories without avatars. This involves adding the 'group-no-avatar'/'group-has-avatar' class. This is kinda ugly, but it's the only way I could think of doing it without template-level mods.
• General cleanup

It'd be nice to get a second set of eyes on this.

[imath]

@boonebgorges: i will look at it asap :)

[imath]

Replying to boonebgorges:

Great!

• Instead of falling back on false when no value has been saved for 'bp-disable-group-avatar-uploads', fall back on the value of the general 'bp-disable-avatar-uploads'. This is for backward compatibility: anyone who currently has avatars disabled across the site will expect them to be disabled after upgrade.

Absolutely

• I added a very slight change to the stylesheet to make things look decent on group directories without avatars. This involves adding the 'group-no-avatar'/'group-has-avatar' class. This is kinda ugly, but it's the only way I could think of doing it without template-level mods.

Group descriptions are best aligned, great.

It'd be nice to get a second set of eyes on this.

Suggestions :

• I think you should carry on disallowing group avatars if the 'Show avatars' WordPress setting is not available, else it could be a regression of what we've fixed in 2.1 see #5345
• I think we shouldn't display the activity secondary avatar if the avatar are not allowed for groups. Else, i find it weird to have the mystery man in 'imath posted an update in [mystery man] group'.
• I think you forgot the Group Profile Photo create step in bp-groups-loader.php.
• Even if there's no chance the Avatar UI would load, i think i'd feel better if we could use your new function. Else, i'll simply commit a change (using true) on #6290 about bp_avatar_is_front_edit() because unlike the user profile photo, the group profile photo screen is never displayed in this case as no gravatar service is available for groups.

4677.4.patch = 4677.3.patch + the above suggestions.

[hnla]

Ok I've patched and tested this:

• Style wise about the best that can be done generically for bp-default styles, adding the classes a good thing.

• UX/UI doesn't feel right, if you're going to allow to disable avatars then you'll have to remove the group creation step

• Similarly in terms of markup rendering we have a mixed bag, group header if already had an avatar will still display the parent wrapper elements, group created after disablement won't, Group dir list displays the markup including empty anchor pointing to group if old or new group.

We'll need to wrap all the avatar calls in a new check in the templates if we do this to remove all traces.

We do impose a minor overhead albeit somewhat trivial of having to factor in additional styles possibly for companion sheets.

If we do commit this to core what would be super nice is if we could have a brief write up or editing of existing guides for new UI functionality so that we can maintain up to date Codex references especially for the user guides section, with perhaps a @mention to myself or mercime so that we can record the addition and keep track.

[boonebgorges]

imath - this is all great. Thanks so much. I had forgotten/neglected most of these points.

hnla - imath's patch addresses your point about group creation.

I intentionally tried not to touch the templates. But I agree that it's crummy to leave empty markup in there. And now that I'm doing the trick with the new 'group-has-avatar' class, removing the markup itself will be a sort of progressive enhancement, which means it's not such a big deal to mod the templates. 4677.5.patch​ does what are (I think) all the necessary template checks, and it looks to me like the page flow continues to be fine in these cases.

[hnla]

Appears to be fine, no markup and no group creation steps.

Now got to puzzle out quite what to do with the additional space now for lists without avatars and with little descriptive text but hey ho :)

[boonebgorges]

In 9828:

Allow group avatars to be disabled.

Props xgz, boonebgorges, imath, hnla.
Fixes #4677.

[imath]

In 9832:

Add missing changes in buddypress-rtl.css

See #4677