Skip to:
Content

BuddyPress.org

Opened 5 years ago

Closed 5 years ago

Last modified 4 years ago

#6859 closed enhancement (fixed)

WP Admin Bar - Sort "My Account" subnav items

Reported by: r-a-y Owned by: r-a-y
Milestone: 2.5 Priority: normal
Severity: normal Version: 1.5.1
Component: Toolbar & Notifications Keywords: has-patch commit
Cc:

Description

This is a follow-up to #3769.

The BuddyBar allows plugins to register subnav menus with a 'position' key, however the WP Admin Bar does not. Currently, plugins have to do some tomfoolery with array_splice() in order to register their subnav at a preferred position. Let's rectify this!

Attached patch allows plugins to simply use a 'position' key just like the BuddyBar when registering a WP Admin Bar subnav. For existing plugins using array_splice() to position their admin subnav item, the code in bp-core-component.php does some 'position' key backfilling, so if a plugin isn't using the new 'position' key, their admin subnav should still display at their rightful place.

Attachments (1)

6859.01.patch (17.5 KB) - added by r-a-y 5 years ago.
Refreshed due to #6870

Download all attachments as: .zip

Change History (7)

#1 @DJPaul
5 years ago

I don't understand your position backfill code. Why all the not_set_pos stuff? Wouldn't just incrementing current position by 1 be adequate?

I'm assuming also that these positions don't have to be unique. And that this is for plugin backpat rather than BP backpat?

#2 @r-a-y
5 years ago

And that this is for plugin backpat rather than BP backpat?

Yes, that's correct.

I don't understand your position backfill code. Why all the not_set_pos stuff? Wouldn't just incrementing current position by 1 be adequate?

Let's say plugins have already injected their subnav item between the Activity's "Personal" and "Mentions" tab without adding a 'position' key.

"Personal" has a position of 10; "Mentions" has a position of 20. We want the iterator to only go up to 9, so the last subnav item will have a 'position' of 19, not 20 and up.

The if ( $pos % 10 === 0 ) portion of the code is to reset the iterator back to 1 when we are on a new subnav range. So if a plugin injected their subnav between "Mentions" (at position 20) and "Favorites" (at position 30), the subnav 'position' key will be 21 instead of whatever $not_set_pos was last at in the previous subnav range.

I could be forgetting other use cases here. Let me know if the code can be improved.

@r-a-y
5 years ago

Refreshed due to #6870

This ticket was mentioned in Slack in #buddypress by r-a-y. View the logs.


5 years ago

#4 @boonebgorges
5 years ago

  • Keywords commit added

This looks good to me, @r-a-y. Regarding the $pos % 10 === 0 trick: The logic here seems right, though I guess there could be a problem if a plugin tried jamming more than 9 unordered admin bar items within a given range - it looks like every one after the eighth one would get 9, which would make their sort order indeterminate? That said, this will probably never happen, and it's probably not a bigger bug than whatever happens now when you do this (they appear in the order in which they're added?)

#5 @r-a-y
5 years ago

  • Owner set to r-a-y
  • Resolution set to fixed
  • Status changed from new to closed

In 10555:

BP Component: Allow plugins to set a custom position when registering an admin bar subnav menu.

This commit allows plugins to set a 'position' key when registering a WP
Admin Bar subnav menu item in the setup_admin_bar() method.

Previously, plugins had to do all sorts of array shuffling in order to
register their subnav at a preferred position. For backward compatibility,
we do some 'position' key backfilling, so if a plugin isn't using the new
'position' key, their subnav should still display at the same location.

Fixes #6859.

#6 @DJPaul
4 years ago

  • Component changed from General - Toolbar/BuddyBar to Toolbar & Notifications
Note: See TracTickets for help on using tickets.