Skip to:
Content

BuddyPress.org

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#5994 closed enhancement (fixed)

Use bp_core_new_subnav_item() to generate group's manage subnav

Reported by: imath Owned by: imath
Milestone: 2.2 Priority: normal
Severity: normal Version:
Component: Groups Keywords: has-patch commit
Cc:

Description

I'm currently working on moving avatar management into the BP Attachments component and i think it would be nice to make it easier to add group's manage tabs at a very precise place.

And i'm not the first who thought of it ;) In BP_Group_Extension::setup_edit_hooks() there's a @todo comment suggesting to use bp_core_new_subnav_item() to generate the manage tabs.

Here's my suggestion about it in attached patch.

Attachments (2)

5994.patch (9.9 KB) - added by imath 5 years ago.
5994.02.patch (10.4 KB) - added by imath 5 years ago.

Download all attachments as: .zip

Change History (9)

@imath
5 years ago

#1 follow-up: @boonebgorges
5 years ago

  • Keywords 2nd-opinion removed

Thanks for doing this, imath. I'm pretty sure I wrote that comment you refer to, and the inconsistency here has long been a point of annoyance for me. What's held me back from fixing it is the backward compatibility issue, but it looks like you've solved that in a clever way.

I might even suggest that if you detect a plugin adding content on the groups_admin_tabs hook, you throw a _doing_it_wrong() notice.

Aside from that, patch looks good to me.

@imath
5 years ago

#2 in reply to: ↑ 1 @imath
5 years ago

Replying to boonebgorges:

I might even suggest that if you detect a plugin adding content on the groups_admin_tabs hook, you throw a _doing_it_wrong() notice.

Aside from that, patch looks good to me.

Thanks a lot for your feedback boonebgorges. In 5994.02.patch, i've added the _doing_it_wrong() notice in case a plugin is directly hooking groups_admin_tabs.

If no objections i'd like to commit this patch as it will be useful for the first pass i'm working on about moving avatar management in attachments component.

Once committed, if you think it's necessary, i can write a post on bpdevel to inform plugin authors about it.

Last edited 5 years ago by imath (previous) (diff)

#3 follow-up: @boonebgorges
5 years ago

  • Keywords commit added

Let's do it. I suggest the following modification to the error message: 'This action should not be used directly. Please use the BuddyPress Group Extension API to generate Manage tabs.'

Once committed, if you think it's necessary, i can write a post on bpdevel to inform plugin authors about it.

Sure, it wouldn't hurt.

#4 in reply to: ↑ 3 @imath
5 years ago

Replying to boonebgorges:

Let's do it. I suggest the following modification to the error message: 'This action should not be used directly. Please use the BuddyPress Group Extension API to generate Manage tabs.'

Ok i'll edit the error message, commit and write a post on bpdevel tonight :) Thanks a lot for your help.

#5 @imath
5 years ago

  • Owner set to imath
  • Resolution set to fixed
  • Status changed from new to closed

In 9127:

Improve the way Group's Manage tabs are generated

In BP_Group_Extension->setup_edit_hooks() method, we are now using the bp_core_new_subnav_item() function to generate the Group's manage tab sub navigation.
The action 'groups_admin_tabs' is no more used. However for back compatibility reasons, we will keep on catching this hook inviting the user using a _doing_it_wrong message to now use the BuddyPress Group Extension API instead.

props boonebgorges

Fixes #5994

#6 follow-up: @DJPaul
5 years ago

This looks like a nice change, good job. :)

#7 in reply to: ↑ 6 @imath
5 years ago

Replying to DJPaul:

This looks like a nice change, good job. :)

Thanks DJPaul :)

Note: See TracTickets for help on using tickets.