[buddypress-trac] [BuddyPress] #5103: Group slug and user's subnav parent_slug trouble

[buddypress-trac]

#5103: Group slug and user's subnav parent_slug trouble
-------------------------------------+------------------------------
 Reporter:  imath                    |       Owner:
     Type:  defect (bug)             |      Status:  new
 Priority:  low                      |   Milestone:  Awaiting Review
Component:  Core                     |     Version:
 Severity:  major                    |  Resolution:
 Keywords:  needs-patch 2nd-opinion  |
-------------------------------------+------------------------------
Changes (by boonebgorges):

 * priority:  normal => low


Comment:

 I've noticed this problem before as well.

 The root issue is the way the `bp_options_nav` array is indexed. User
 profile-related nav is keyed by component: 'profile', 'activity', etc.
 Group-related nav is keyed by the group slug (since BP 1.5). So, when
 there's a match between the group slug and a component, everything gets
 jumbled. It's a little bit more complicated than this, but basically, the
 following two things happen:

 {{{
 // in the activity component
 bp_core_new_subnav_item( array(
     'slug' => 'favorites',
     'parent_slug' => 'activity', // $bp->activity->slug
 ) );

 // in the group component
 bp_core_new_subnav_item( array(
     'slug' => 'members',
     'parent_slug' => 'activity', // $bp->groups->current_group->slug
 ) );
 }}}

 Messing around with the forbidden names array seems hackish to me. It
 doesn't address the root issue, which is that `bp_options_nav` is built in
 an inconsistent way across object types. Also, it wouldn't be effective
 anyway, for reasons like what imath suggests.

 If we didn't have to worry about backward compatibility, I'd say that we
 should rethink the way that `bp_options_nav` works. There are a couple
 possibilities.

 a) Add a new parameter to `bp_core_new_nav_item()`, something like
 'context', which could be 'group', 'user', etc. That'd be stored in the
 `bp_options_nav` array. Then, when the nav is built (in
 `bp_get_options_nav()`, I think), you'd check to make sure that you're
 only loading context-appropriate items before spitting them out. This
 would solve the problem on the surface, but the global data object would
 still be pretty messed up. That said, it wouldn't be too hard to manage
 backpat for this strategy.

 b) Rethink the way that `bp_nav` and `bp_options_nav` are structured, and
 make it more consistent across contexts. We currently consider the current
 group to be the top-level nav item when creating the group nav (and,
 oddly, we call that nav item "Memberships", though it's not displayed
 anywhere), and all of the "main" nav items under it (Members, Admin, etc)
 are subnav items. In contrast, the "main" nav items (Profile, Activity,
 etc) under a user are *top-level* nav items (stored in `bp_nav`) and the
 second row of nav items (Edit Profile, Change Avatar, etc) are stored in
 `bp_options_nav`. I know why it works this way from a legacy point of
 view, and from the point of view of what counts as a "component" in
 BuddyPress, but it doesn't make a whole lot of sense when looked at with a
 fresh pair of eyes. There are various ways we might think about changing
 this arrangement, but all of them are likely to break backward
 compatibility in many ways.

 Anyway, those are my initial thoughts. I'm anxious to hear what ideas
 other people have.

 (I'm marking this ticket Low priority, because in the end, I think we
 might wait until we do a major overhaul of navigation building before
 attempting to fix it. Also, the case that imath raises is kind of an edge
 case, annoying as it may be, so it's not urgent to fix it. That said, if
 we come up with a good idea for fixing the issue, I'm glad to move it into
 1.9.)

--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/5103#comment:1>
BuddyPress <http://buddypress.org/>
BuddyPress