WP Admin Bar - Cannot arrange "My Account" items

[r-a-y]

If you register a new subnav item under "My Account", you can only position it above the current My Account items or below them.

This is because 'bp_setup_admin_bar' action in the core BP_Component class fires all hooks at the same priority):
​http://buddypress.trac.wordpress.org/browser/tags/1.5.1/bp-core/bp-core-component.php#L221

A proposed solution involves removing the "bp_setup_admin_bar" hook from the core BP_Component class and adding it in each component's setup_actions.

Attached is a patch showing how this could be done with the friends component. It's hooked at a priority of 30 as an example.

[boonebgorges]

I agree that we should be hooking directly to setup_admin_bar. It stinks to force all BP content to be right next to each other. Would like to get feedback from jjj on that, as I know he's big on consolidating BP hooks (which I agree with in general - it's just that in this case, it has limiting factors for UX, which should take precedence).

[johnjamesjacoby]

r-a-y: I like the idea of your patch, but don't like decoupling this from the BP_Component base class. The goal of that class is to automate all of the things a traditional component does, and this is a step away from that. I'm moving this to Future Release, but let's revisit this again in 1.7.

[r-a-y]

02.patch adds an additional parameter to BP_Component::start() so we can define an 'adminbar_position' parameter to pass onto BP_Component::setup_actions() and use it there. In the patch, I've used the Friends Component as an example.

I've opted to add a fourth parameter to BP_Component::start() instead of refactoring BP_Component::start() to use a one-parameter array like BP_Activity_Activity::get(), but we can go that route if we decide to.

Let me know what you guys think.

[DJPaul]

Moving this to 1.8 as it has a patch, and probably just needs a refresh and some TLC. Punt to future release if this misses the 1.8 boat.

[boonebgorges]

I think this technique is fine. My only quibble is with the parameter name 'adminbar_position', which (to my ear at least) suggests positioning on the top-level admin bar, while what's happening here is really positioning on the My Account menu. Maybe something like 'myaccount_menu_order'?

[r-a-y]

My only quibble is with the parameter name 'adminbar_position', which (to my ear at least) suggests positioning on the top-level admin bar, while what's happening here is really positioning on the My Account menu. Maybe something like 'myaccount_menu_order'?

I think 'myaccount_menu_order' is an improvement, but that could also refer to the BuddyBar / profile nav menu order. How about 'adminbar_myaccount_order' or 'toolbar_myaccount_order'?

[boonebgorges]

How about 'adminbar_myaccount_order'

I like this :)

[r-a-y]

Before I commit 02b.patch, here's something I just thought of.

The only bad thing about this technique is for plugins wanting to support older BP versions.

This is something I'm currently doing in my custom extended BP_Component class.

	/**
	 * Custom version of the {@link BP_Component::start()} method.
	 *
	 * Since BuddyPress 1.9, start() has four parameters.  However, for backwards-
	 * compatibility with older versions of BuddyPress, we need to pass three.
	 */
	protected function init() {
		$menu_order = 65;

		$rf = new ReflectionMethod( 'BP_Component', 'start' );

		// as of BP 1.9, start() method accepts 4 parameters
		if ( $rf->getNumberOfParameters() == 4 ) {
			parent::start(
				'blah',
				__( 'Blah', 'bp-blah' ),
				constant( 'BP_BLAH_DIR' ) . '/includes',
				array(
					'adminbar_myaccount_order' => (int) $menu_order
				)
			);		

		// older versions of BP
		// this section will probably be removed in a future version
		} else {
			parent::start(
				'blah',
				__( 'Blah', 'bp-blah' ),
				constant( 'BP_BLAH_DIR' ) . '/includes',
			);

			// set properties manually
			$this->adminbar_myaccount_order = (int) $menu_order
		}

	}

init() is used in place of parent::start() in the constructor.

[boonebgorges]

There's no particular problem with passing more arguments than needed to BP_Component::start(). You can run the following on BP 1.8:

parent::start(
    'blah',
    'Blah',
    'blah/includes',
    array( 'foo' )
);

and it'll just ignore the additional param.

I guess what you're really pointing to is the adminbar order. But I think it's fine not to support it officially for older versions. It's a new feature. Plugins that want to support it can do a workaround like the one you've suggested here. So, I don't really see an issue.

[r-a-y]

There's no particular problem with passing more arguments than needed to BP_Component::start().

For some reason, I thought that a warning would pop up because of the number of arguments being used do not match those in the start() method. Needed the sanity check. Thanks Boone!

[r-a-y]

In 7454:

Add a fourth parameter to the BP_Component::start() method.

This parameter is an array that allows extended classes to define some
properties that need to be set early one for BP_Component to be able
to work its magic.

Currently accepts 'adminbar_myaccount_order', which sets a custom
position for the component menu generated under the WP Toolbar's
"My Account" menu. If this value isn't set, we set the value to 90,
which will generate the menu before the Settings menu is added.

Fixes #3769.