Remove BuddyBar?

[DJPaul]

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)?

[r-a-y]

+1 from me here.

[r-a-y]

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.

[DJPaul]

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).

[DJPaul]

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]

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.

[DJPaul]

Milestone renamed

[DJPaul]

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

[r-a-y]

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.

[imath]

Thanks a lot @r-a-y for your work on these patches. Let's try to have this ticket fixed during the 6.0.0 development cycle as we were very close to have the conservative version of the patch committed.

[espellcaste]

@imath Are you still considering committing this one in this development cycle?

[imath]

Yes but the patch needs to be refreshed (eg: deprecated/6.0.0 and not 2.1.0). I'll take care of it if @r-a-y doesn't beat me to it ☺️

[r-a-y]

03.patch is a refreshed patch of 02.patch, but also deprecates bp_force_buddybar(), removes the admin option for the BuddyBar and updates the PHPDoc in a few places.

Let me know what you think.

[imath]

Hi @r-a-y

Thanks for the refresh, I just tested the patch and I'm having a doubt about line 596 of src/bp-groups/classes/class-bp-groups-component.php. In your patch you're removing it, but then we get a failing test: BP_Tests_Core_Nav_BpCoreNewNavItem::test_group_nav().

I've been tested the patch with or without this line and it doesn't seem to have an impact. But I'd probably keep it this way, just in case.

If you're sure we don't need it anymore, then remove the BP_Tests_Core_Nav_BpCoreNewNavItem::test_group_nav() test ;)

I believe we probably need to write a dev note about this removal on bpdevel 🤔

[r-a-y]

Hi @imath, do you remember why you wrote the unit test? (r9818) :)

I believe the -1 thing is a hack because the BuddyBar required it so the group nav wouldn't show up in the BuddyBar. See its introduction in r3818 by JJJ (wow that's a long time ago!). Since we're no longer using the BuddyBar, this shouldn't be required, but I can understand why you would feel safer leaving it in. If we want to be cautious, let's just leave the line in.

[imath]

Hi! Probably for the same reason I'm worried about it today :)

I think I'd leave it as I believe it's the reason of ​this code into the bp_core_register_nav_screen_function()

[imath]

@r-a-y I'd like to publish 6.0.0-beta1 early next week. Do you think we'd better move this ticket in next milestone ?

[imath]

Let's do it early during 7.0.0 dev cycle.

[imath]

Let's try to work on this during next development cycle

[imath]

I've just updated the patch and ran some complementary tests. It looks good so I believe it's about time to say "Adios" to the BuddyBar. I will commit it later today.

[imath]

In 12893:

Entirely remove the code of the deprecated BuddyBar

The BuddyBar has been deprecated in version 2.1. We are now completely removing it.

Props r-a-y, DJPaul

Fixes #7729