Skip to:
Content

BuddyPress.org

Opened 2 years ago

Last modified 4 months ago

#7729 new enhancement

Remove BuddyBar?

Reported by: DJPaul Owned by:
Milestone: 6.0.0 Priority: normal
Severity: normal Version:
Component: Navigation Keywords: has-patch dev-feedback needs-refresh
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 23 months ago.
7729.02.patch (24.6 KB) - added by r-a-y 23 months ago.

Download all attachments as: .zip

Change History (15)

#1 @r-a-y
23 months ago

+1 from me here.

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


23 months ago

@r-a-y
23 months ago

#3 @r-a-y
23 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 23 months ago by r-a-y (previous) (diff)

#4 @DJPaul
23 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
23 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
23 months ago

#6 @r-a-y
23 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 23 months ago by r-a-y (previous) (diff)

#7 @DJPaul
21 months ago

  • Milestone changed from 3.1 to 4.0

Milestone renamed

#8 @DJPaul
21 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
19 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
16 months ago

  • Keywords commit added

#11 @boonebgorges
16 months ago

  • Milestone changed from 4.0 to Up Next

#12 @m_uysl
15 months ago

  • Cc m_uysl added

#13 @imath
4 months ago

  • Keywords needs-refresh added; commit removed
  • Milestone changed from Up Next to 6.0.0

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.

Note: See TracTickets for help on using tickets.