[buddypress-trac] [BuddyPress] #3652: bp_core_new_nav_default() doesn't really work

[buddypress-trac at lists.automattic.com]

#3652: bp_core_new_nav_default() doesn't really work
------------------------------------+------------------
 Reporter:  boonebgorges            |       Owner:
     Type:  defect (bug)            |      Status:  new
 Priority:  normal                  |   Milestone:  1.6
Component:  Core                    |     Version:
 Severity:  normal                  |  Resolution:
 Keywords:  has-patch dev-feedback  |
------------------------------------+------------------
Changes (by boonebgorges):

 * keywords:  needs-patch => has-patch dev-feedback
 * severity:  minor => normal


Comment:

 This is indeed tricky. I can't figure out a way to fix this while
 maintaining any semblance of backpat with previous uses of
 bp_core_new_nav_default(). In a nutshell, here's how the current system
 works:
 - you add a nav item, including info about the default subnav, at
 bp_setup_nav, using bp_core_new_nav_item()
 - That function looks to see whether you need to be redirected to the
 default subnav; if so, it redirects you, and if not, it hooks the proper
 screen function.

 So, in short, the redirection, etc happens *immediately after* the nav
 item is added, which leaves no obvious place to change the behavior. You
 need to have access to the nav item in order to change the default subnav
 (so you need to hop in after it's been added), but you need to alter it
 before the redirect happens, or you'll be too late to modify the nav
 default.

 3652.01.patch is a proposed solution. Here's how it works:
 - The plugin developer sets up a function that calls
 bp_core_new_nav_default() inside of a function hooked *before* the nav
 item is added. Eg,

 {{{
 function bbg_set_settings_subnav(){
         bp_core_new_nav_default( array(
                 'parent_slug'     => bp_get_settings_slug(),
                 'screen_function' =>
 'bp_core_screen_notification_settings',
                 'subnav_slug'     => 'notifications'
         ) );
 }
 add_action( 'bp_setup_nav', 'bbg_set_settings_subnav', 5 );
 }}}

 - bp_core_new_nav_default() adds this array to a nav_default_resets stack
 for later use.
 - A new hook has been added between the time when the nav item is added
 and the time when the redirect occurs (it's called bp_core_nav_default,
 and it appears in the middle of bp_core_new_nav_item())
 - A new function bp_core_reset_nav_default() is hooked to
 bp_core_nav_default. It looks to see whether, for that slug, any plugins
 have attempted to change the default subnav, and if so, it swaps out the
 information accordingly.

 So, it's not totally backward compatible - those who are currently using
 bp_core_new_nav_default() will have to change their priority so that it
 happens *before*, rather than *after*, the nav item is added. And it's not
 particularly elegant. But it works. The only other alternative I see is to
 take the redirecting stuff and to put it much later in the BP loading
 process - which is fine, except that it will make the redirects happen
 more slowly by introducing more processing before they occur, which is IMO
 a bad thing.

 Would be happy to get some feedback on this issue. Thanks.

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