Skip to:
Content

Opened 4 weeks ago

Closed 2 weeks ago

Last modified 12 days ago

#7485 closed enhancement (fixed)

Allow bp_get_group_permalink() to produce HTML links.

Reported by: dcavins Owned by: dcavins
Milestone: 2.9 Priority: normal
Severity: normal Version:
Component: Groups Keywords: has-patch
Cc: dcavins

Description

I believe there's a use-case for a convenience function that outputs a group's permalink as HTML, like:
<a href="<?php bp_group_permalink(); ?>"><?php bp_group_name(); ?></a>
sort of like bp_core_get_userlink().

We can get the convenience without adding a new function by adding a parameter to bp_get_group_permalink() as I've done in the first patch.

Thanks for your feedback!

Attachments (3)

7485.1.diff (2.7 KB) - added by dcavins 4 weeks ago.
Add an "html_link" parameter to bp_get_group_permalink().
7485.2.diff (1.6 KB) - added by dcavins 3 weeks ago.
Adds a new function bp_get_group_link() to produce an HTML-formatted link to a group.
7485.replacements.diff (3.3 KB) - added by dcavins 3 weeks ago.
Here are some places that the new function could replace "built" links.

Download all attachments as: .zip

Change History (21)

@dcavins
4 weeks ago

Add an "html_link" parameter to bp_get_group_permalink().

#1 @hnla
4 weeks ago

+1 Is it better though that we use the existing get function, is this meant to echo a complete link, would not a dedicated function be better?

#2 @dcavins
4 weeks ago

  • Owner set to dcavins
  • Status changed from new to accepted

I was thinking along the lines of bp_core_get_userlink() which can return a url or a formatted link.
https://buddypress.trac.wordpress.org/browser/branches/2.8/src/bp-members/bp-members-functions.php#L498

In this case, the default behavior of bp_get_group_permalink() is left intact, the second parameter allows the new behavior.

This ticket was mentioned in Slack in #buddypress by hnla. View the logs.


4 weeks ago

#4 @boonebgorges
4 weeks ago

  • Keywords dev-feedback removed

A separate function seems better to me. The fact that bp_core_get_userlink() can return an unformatted URL seems like a historical accident - it just becomes a wrapper for bp_core_get_user_domain().

Maybe bp_get_group_link() or something like that?

#5 @dcavins
3 weeks ago

Thanks @hnla and @boonebgorges for your feedback. I'm attaching a new patch that leaves bp_get_group_permalink() unchanged and adds a new function as suggested: bp_get_group_link().

Is this a useful template function, or is this one of those things that is no big deal to build when you need it, so there's no need for a dedicated function? I'm OK either way.

Thanks again for looking at the first patch.

#6 @hnla
3 weeks ago

was going to ask whether this name was right for an echo type function but see there is one bp_group_link() :)

I think it never hurts to have these? Yes one can build quite easily but still nice to save a little time when pressed with 1001 other things to get done.

#7 @boonebgorges
3 weeks ago

As a rule, if there are more than 2 places in BP where we could immediately put the new function to use (by swapping out what is currently concatenated), then it's probably a good move. I haven't looked, but I assume that there's many more than two places for this specific function.

@dcavins
3 weeks ago

Adds a new function bp_get_group_link() to produce an HTML-formatted link to a group.

#8 @dcavins
3 weeks ago

Those are good points. I'll attach a diff that would update a handful of existing instances in BP. Another popular form is creating a link using the group photo instead of the group name.

Thanks again for taking the time to look at this and respond.

@dcavins
3 weeks ago

Here are some places that the new function could replace "built" links.

#9 follow-up: @hnla
3 weeks ago

Just to over egg the pudding could we class the anchor:
<a class="%s-link" href="%s">%s</a> or %s-home

If we wanted to really over egg the pudding we could allow class strings to be passed through :) but that would be gilding the lilly (today is a mixed metaphor day!)

#10 @hnla
3 weeks ago

I'll be ticketing Nouveau to update those replacement lines highlighted in templates.

Last edited 3 weeks ago by hnla (previous) (diff)

#11 in reply to: ↑ 9 @dcavins
3 weeks ago

Replying to hnla:

Just to over egg the pudding could we class the anchor:
<a class="%s-link" href="%s">%s</a> or %s-home

I'm happy to add whatever class name(s) you recommend, and think it's a good idea. Since these are general as well as specific, maybe we should add a couple, like a general group-home-link as well as a specific %s-home or %s-link? (I'm assuming we'd use the group's slug as the identifier in the class string.)

Thanks!

#12 @hnla
3 weeks ago

maybe we should add a couple, like a general group-home-link as well as a specific %s-home or %s-link? (I'm assuming we'd use the group's slug as the identifier in the class string.)

Absolutely to all the above, should be a generic link too & yes assumed the group slug would provide the dynamic portion.

This ticket was mentioned in Slack in #buddypress by hnla. View the logs.


3 weeks ago

This ticket was mentioned in Slack in #buddypress by dcavins. View the logs.


3 weeks ago

#15 @dcavins
3 weeks ago

Per our discussion in dev chat, I think we've settled on bp-group-home-link for the general and %s-home-link for the specific. Please comment if you think something else would be better. :)

#16 @dcavins
2 weeks ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 11527:

Introduce new template function bp_group_link().

This new template function outputs a group’s name as a text hyperlink
to a group’s home page.

Props hnla, dcavins.

Fixes #7485.

#17 @dcavins
2 weeks ago

In 11528:

Use new template function bp_group_link().

Use the new template function where appropriate.

See #7485.

#18 @r-a-y
12 days ago

  • Milestone changed from Awaiting Review to 2.9
  • Version 2.8.2 deleted
Note: See TracTickets for help on using tickets.