[buddypress-trac] [BuddyPress Trac] #6503: Separate functions for creating a new nav link and registering a screen function.

[buddypress-trac]

#6503: Separate functions for creating a new nav link and registering a screen
function.
------------------------------+-----------------------------
 Reporter:  dcavins           |       Owner:
     Type:  enhancement       |      Status:  new
 Priority:  normal            |   Milestone:  Future Release
Component:  Component - Core  |     Version:  2.3.1
 Severity:  normal            |  Resolution:
 Keywords:  has-patch         |
------------------------------+-----------------------------

Comment (by boonebgorges):

 >  Can anyone think of a reason for this order of operations? I moved it
 before things happen, so if the old order of operations is important,
 please let me know.

 See #1263 [2048]. I'm assuming it just happened to be added there - I
 don't see evidence that it was on purpose. I can't think of any reason why
 your change would break anything, so let's go with it. (I only see the
 green in 6503.split-new-nav-item.patch. Make sure you remove the existing
 check too.)

 > There's a do_action( 'bp_core_new_nav_item' ) at the end of
 bp_core_new_nav_item(). I had to choose where that should go, and left it
 after the screen function is registered. Is that appropriate?

 I'm not seeing this in the patch, but I'd leave it where it is. Leave it
 in `bp_core_new_nav_item()`, after the screen function is hooked, to
 ensure compatibility with the current behavior.

 > bp_core_new_nav_item () is only called in BP_Component::setup_nav()
 which doesn't currently take advantage of the split, so I've left that
 call as is for now.

 +1. This seems fine to me. We're not deprecating the existing function,
 just converting it to a wrapper.

--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/6503#comment:13>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac