Skip to:
Content

BuddyPress.org

Opened 9 years ago

Closed 7 years ago

#6525 closed enhancement (maybelater)

Members and Groups avatars: Allow to set 'title' argument

Reported by: lenasterg's profile lenasterg Owned by:
Milestone: Priority: low
Severity: minor Version:
Component: Core Keywords: needs-patch, needs-refresh, trac-tidy-2018
Cc:

Description

Similar to ticket:6519.
The current settings of the functions related to group's and member's avatar don't allow the 'title' parameter to be set.

The attached file is a patch for the groups and members template files in order to allow the use of "title" as argument for avatars.

Attachments (3)

avatars_members_groups.patch (10.9 KB) - added by lenasterg 9 years ago.
6525_2.patch (20.5 KB) - added by lenasterg 9 years ago.
6525.3.patch (20.5 KB) - added by boonebgorges 9 years ago.

Download all attachments as: .zip

Change History (18)

#1 @hnla
9 years ago

I agree in principle with adding some of these but would ask what the use case is or the perceived necessity.

The use of the title attribute is intended to be one where it's use adds needed additional info on a link or element however it's usefulness in this sense has always been questioned due to browser/devise support.

I would prefer we didn't add title attributes simply cos they exist and we can as that's an inappropriate use of them.

In some cases in the patch we double up where there is an element that can and does carry the 'alt' attribute adding in a title attribute with same value - not necessarily a bad thing though.

If we add these it might be preferable to set the default as empty only adding the attr if the user passes one across - although that does mean it may be a struggle to find how to pass values of a more complex nature?

I'm not discouraging the use of the attr though just that we need to look at each piece of data we pass to it to ensure it's relevant and worthwhile.

http://www.w3.org/TR/html5/dom.html#the-title-attribute
http://www.w3.org/html/wg/wiki/ChangeProposals/notitlev2

#2 @hnla
9 years ago

  • Type changed from defect (bug) to enhancement

#3 @lenasterg
9 years ago

@hnla hi.
I understand what you are saying.
I proposed the changes for two reasons:

  1. To keep the consistency for the arguments that the functions using the bp_core_fetch_avatar() accept.
  2. To allow theme designers/plugins developers to use the title argument to show info (for the devices which supports title). Example: if a theme only displays members avatars in the group's members loop, or only group avatars in the groups loop.

I was about to proceed to the same changes for the activity avatars. So, please, let me know what the final decision about the title argument will be.

Thanks in advance,
Lena.

#4 @hnla
9 years ago

To keep the consistency for the arguments that the functions using the bp_core_fetch_avatar() accept.

Consistency is one good reason then.

I'm not against as stated but in adding these we need to understand that they are not some alternative or semantically useful necessarily.

I would prefer the option was there but defaulted to empty arg so it had to be set explicitly, that may not be everyone's view though and I have supported, even added in title attrs myself in the past.

I would say go ahead, it's not going to be wasted effort. The leads devs will pass final judgement on how these are added.

@lenasterg
9 years ago

#5 @lenasterg
9 years ago

@hnla, hi again.
Please, ignore the avatars_members_groups.patch

The 6525_2.patch includes changes for avatars on activity, groups, members, messages, notifications and xprofile admin.

Thanks in advance
Lena

#6 @boonebgorges
9 years ago

In 9964:

Improve formatting for docblocks of group avatar functions.

Props lenasterg.
See #6525.

#7 @boonebgorges
9 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release

Thanks for the patch, lenasterg. I committed a few of the docblock improvements; 6525.3.patch is your patch, minus these fixes.

I agree with hnla that we generally should not be encouraging or enforcing the use of title on avatars. With that in mind, I'd like to suggest the following improvements to your patch:

  • Let's go ahead and add support for passing the title (and alt, where appropriate) attribute to functions like bp_messages_thread_avatar(). As you note, bp_core_fetch_avatar() supports the use of these attributes, so there's no reason why you shouldn't be able to use them in the wrappers. If possible, please consider writing unit tests for these new parameters - see [9960] for some examples.
  • In cases where the function is not already defining a title, let's leave the default value as an empty string '' (eg bp_message_thread_avatar()). In cases where the function *is* already defining a title, make the default value the same as whatever the title currently is (eg bp_get_group_avatar()).
  • Let's not pass title attributes to bp_core_fetch_avatar() where they don't already exist. This means rolling back changes like those in BP_Groups_Invite_Template::the_invite().

@boonebgorges
9 years ago

#8 @boonebgorges
9 years ago

(also, small nitpick - if you can clean up the whitespace to conform to WP/BP standards, that'd be nice too :) )

#9 @DJPaul
9 years ago

  • Milestone changed from Future Release to 2.5

See also #6415 which is kind of related.

#10 @DJPaul
9 years ago

  • Keywords needs-refresh added
  • Milestone changed from 2.5 to 2.6

#11 @DJPaul
9 years ago

  • Milestone changed from 2.6 to Future Release

#12 @DJPaul
8 years ago

  • Component changed from Component - Any/All to Core

#13 @mercime
8 years ago

Recommend that we create a new BP tooltip which will be accessible and usable in all devices instead for avatars and links which need additional information. See #7188.

#14 @DJPaul
7 years ago

  • Keywords trac-tidy-2018 added

We're closing this ticket because it has not received any contribution or comments for at least two years. We have decided that it is better to close tickets that are good ideas, which have not gotten (or are unlikely to get) contributions, rather than keep things open indefinitely. This will help us share a more realistic roadmap for BuddyPress with you.

Everyone very much appreciates the time and effort that you spent sharing your idea with us. On behalf of the entire BuddyPress team, thank you.

If you feel strongly that this enhancement should still be added to BuddyPress, and you are able to contribute effort towards it, we encourage you to re-open the ticket, or start a discussion about it in our Slack channel. Please consider that time has proven that good ideas without contributions do not get built.

For more information, see https://bpdevel.wordpress.com/2018/01/21/our-awaiting-contributions-milestone-contains/
or find us on Slack, in the #buddypress channel: https://make.wordpress.org/chat/

#15 @DJPaul
7 years ago

  • Milestone Awaiting Contributions deleted
  • Resolution set to maybelater
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.