Skip to:
Content

BuddyPress.org

Opened 19 months ago

Last modified 11 months ago

#7729 new enhancement

Remove BuddyBar?

Reported by: DJPaul Owned by:
Milestone: Up Next Priority: normal
Severity: normal Version:
Component: Navigation Keywords: has-patch dev-feedback commit
Cc: m_uysl

Description

See https://imgur.com/a/KEiM5

The Buddybar was deprecated in 2.1 (2014). You can turn it on define( 'WP_DEBUG', false ); define( 'BP_USE_WP_ADMIN_BAR', false ); but it doesn't fully work nowadays and it throws PHP notices.

I know a lot of its code base is tied to the navigation bar logic, but it does load easy-to-remove CSS assets.
What do you think of taking advantage of our major 3.0 release and removing as much of the Buddybar code as easily possible (basically removing the backwards compatibility)?

Attachments (2)

7729.01.patch (35.4 KB) - added by r-a-y 19 months ago.
7729.02.patch (24.6 KB) - added by r-a-y 18 months ago.

Download all attachments as: .zip

Change History (14)

#1 @r-a-y
19 months ago

+1 from me here.

This ticket was mentioned in Slack in #buddypress by djpaul. View the logs.


19 months ago

@r-a-y
19 months ago

#3 @r-a-y
19 months ago

  • Keywords has-patch added

01.patch is a first pass at removing BuddyBar functionality.

The removal of bp_loggedin_user_domain() to each component's setup_nav() method is due in part to the generation of the BuddyBar. Since we're no longer supporting the BuddyBar, we can remove those instances and only check if we're on a user profile page.

Last edited 19 months ago by r-a-y (previous) (diff)

#4 @DJPaul
18 months ago

  • Milestone changed from Awaiting Review to 3.1

Nice. I think this doesn't fit into 3.0 as we're short on time, and the risk is raised because of the setup_nav() changes you found (unless you can very test this next week (before Friday) maybe).

#5 @DJPaul
18 months ago

The patch looks OK so far. I need to get my brain around the setup_nav change (or we get a second opinion from someone).

Are the functions in bp-core-buddybar.php basically route parsing/registration?

@r-a-y
18 months ago

#6 @r-a-y
18 months ago

the risk is raised because of the setup_nav() changes you found (unless you can very test this next week (before Friday) maybe).

There is one issue with 01.patch and that is those that rely on the BuddyBar / older buddypress()->bp_nav for generating the logged-in navigation.

01.patch breaks this unit test:
https://buddypress.trac.wordpress.org/browser/trunk/tests/phpunit/testcases/routing/core.php

Since we are removing the BuddyBar, there isn't much of a use-case for keeping logged-in navigation. We also introduced BuddyPress integration into "Appearance > Menus" a long time ago as well (#5122), which most sites are probably using for menu generation.

If we want to keep compatibility with older, logged-in navigation, then see 02.patch.

However, if we're not concerned with the older logged-in navigation, I've already tested 01.patch for all frontend user profile navigation items and this still works.

Are the functions in bp-core-buddybar.php basically route parsing/registration?

bp-core-buddybar.php is poorly named, but that file is critical to user nav registration.

Could be renamed to bp-core-nav.php or something like that.

Last edited 18 months ago by r-a-y (previous) (diff)

#7 @DJPaul
17 months ago

  • Milestone changed from 3.1 to 4.0

Milestone renamed

#8 @DJPaul
17 months ago

  • Keywords dev-feedback added

It would be interesting to get feedback from other core contributors here about which patch we should proceed with.

#9 @r-a-y
15 months ago

Just saw someone post an example using the older $bp->bp_nav syntax in the forums, so let's go with the conservative 02.patch.

#10 @DJPaul
12 months ago

  • Keywords commit added

#11 @boonebgorges
12 months ago

  • Milestone changed from 4.0 to Up Next

#12 @m_uysl
11 months ago

  • Cc m_uysl added
Note: See TracTickets for help on using tickets.