Skip to:
Content

BuddyPress.org

Opened 11 years ago

Closed 7 years ago

#5026 closed enhancement (wontfix)

bp_get_loggedin_user_nav() - marked as @deprecated, but used in themes

Reported by: tivnet's profile tivnet Owned by:
Milestone: Priority: lowest
Severity: trivial Version: 1.7
Component: Navigation Keywords:
Cc: gregory@…

Description

 * @package BuddyPress Core
 * @todo Move to a back-compat file?
 * @deprecated Does not seem to be called anywhere in the core
 */
function bp_get_loggedin_user_nav() {

But:

  • used by 3rd party themes (found at least 4 on WPMU)
  • quite convenient - easy to put in a top navigation menu, for example

A) So, it's probably should stay

B) I would re-consider the last statement there:

// Always add a log out list item to the end of the navigation
	$logout_link = ....

Not always, but if logged in. Otherwise - login/register.

Change History (6)

#1 @boonebgorges
11 years ago

  • Keywords needs-patch removed
  • Milestone changed from Awaiting Review to 1.8
  • Priority changed from normal to lowest
  • Type changed from defect (bug) to enhancement

The fact that it's used in themes is not an argument for not deprecating. Generally we deprecate functions if we have new methods for accomplishing the ends of the deprecated function, and we plan to phase out support for the deprecated function. In the case of bp_get_loggedin_user_nav(), we are definitely not supporting it anymore - I don't think any changes have been made to the function in several years.

As to whether we have a recommended alternative, it's not as clear. Since BP 1.2, BP's packaged themes have used bp_get_displayed_user_nav(), but not the logged_in version, presumably because (a) we only render the menu on user pages, ie when there's a displayed user id, and (b) we do the is-this-my-profile checks when building the nav menus, so we don't need a separate function on the way out. And since BP 1.7, we have a very cool function bp_nav_menu() that spits out the displayed user's nav in a nicely nested way.

However, these functions don't allow you to get the logged-in user's nav on a non-user page. So, if bp_get_loggedin_user_nav() does this, I guess it does something that we aren't offering an alternative for, and we shouldn't drop it. Looking at the source code, it's not obvious to me that it *does* do that, though. I have a feeling that if you called this function on a group page, you would get gobbldygook. If that's true, it should be deprecated.

You're correct about the comment for the logout link. I'll fix that.

#2 @boonebgorges
11 years ago

  • Milestone 1.8 deleted
  • Resolution set to invalid
  • Status changed from new to closed

You're correct about the comment for the logout link. I'll fix that.

Actually, I changed my mind about this. You shouldn't be calling bp_get_loggedin_user_nav() for logged out users. If anything, we should be bailing out of the function if the user is not logged in.

#3 @tivnet
11 years ago

Agreed 100%. (Correction: 99%, see below)

bp_nav_menu() can be listed there as @see, and I can add the missing "logout" using the bp_nav_menu_items hook.

Last edited 11 years ago by tivnet (previous) (diff)

#4 @tivnet
11 years ago

  • Cc gregory@… added
  • Resolution invalid deleted
  • Status changed from closed to reopened

@boonebgorges:

Just to get your attention. Probably, related also to #4624

bp_nav_menu, while very nice indeed, cannot be displayed site-wide without some fixes. Specifically, outside the /members/, all top links are missing the root URL, because when bp_displayed_user_domain() is empty, this part works quite ugly:

$link = bp_loggedin_user_domain() ? str_replace( bp_loggedin_user_domain(), bp_displayed_user_domain(), $nav['link'] ) : trailingslashit( bp_displayed_user_domain() . $nav['link'] );

In addition, the if ( ! $nav['show_for_displayed_user']... part hides "Mesages" and "Settings", when outside the /members/.

I am not back the original subject, "deprecated bp_get_loggedin_user_nav()". Just wanted to say that bp_nav_menu() is not a 100% suitable substitution.

#5 @boonebgorges
11 years ago

  • Milestone set to Future Release

OK, point taken. It's probably the case that bp_nav_menu() can't do exactly what you want. Let's be sure that bp_nav_menu() is modified so that it can actually fill in for bp_get_loggedin_user_nav() before the latter is formally deprecated.

#6 @slaFFik
7 years ago

  • Component changed from Core to Navigation
  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from reopened to closed

This function is not removed, and not used in core. Some 3rd party themes are using it, but that is because they are not updated for years. Not sure that we want to be fixed to out-dated themes.

bp_nav_menu() was rewritten, BP_Core_Nav class was introduced (and additional walkers), so most of concerns (if not all) in this thread are no more valid.
Thus, I'm closing the ticket as wontfix. We might move this deprecated function into deprecated file some day, but not now.

Note: See TracTickets for help on using tickets.