Skip to:
Content

BuddyPress.org

Opened 7 years ago

Closed 20 months ago

#4209 closed enhancement (maybelater)

bp_get_displayed_user_nav() should have exclusion argument. (code included)

Reported by: DennisSmolek Owned by:
Milestone: Priority: normal
Severity: normal Version: 1.5.5
Component: Toolbar & Notifications Keywords: dev-feedback, trac-tidy-2018
Cc: DennisSmolek

Description

I am doing a Nav where I would like to keep the profile and settings pages, and in fact call them later, but I am trying to keep them off the "Main" Nav.

Now as a hack I am changing the $bp->bp_nav array and then resetting it after my call as in my case I need it later but that means 2 functions to exclude items from this UL list and a lot of janking with the $bp global.

A simpler approach would be to have the bp_get_user_nav() accept arguments including a exclusion array. This way $bp is never effected in any subsequent calls (like mine for my "accounts" page)

This would also have the benefit of allowing things like groups without the friends component showing up on the main nav if the owner didn't want it or in my case where admins have the ability to edit profiles, but the users do not. (a simple conditional)

I dont have a whole patch file but I do have a working example:

from bp-members-template.php line 746

function bp_get_displayed_user_nav( $args='' ) {
	global $bp;
	if ($args['exclude']) {
		foreach ($args['exclude'] as $key => $value) {
			$exclude[$value] = true;
		}
	}
	foreach ( (array)$bp->bp_nav as $user_nav_item ) {
		if ( !$user_nav_item['show_for_displayed_user'] && !bp_is_my_profile() || $exclude[$user_nav_item['slug']] )
			continue;

		if ( $bp->current_component == $user_nav_item['slug'] )
			$selected = ' class="current selected"';
		else
			$selected = '';

		if ( $bp->loggedin_user->domain )
			$link = str_replace( $bp->loggedin_user->domain, $bp->displayed_user->domain, $user_nav_item['link'] );
		else
			$link = $bp->displayed_user->domain . $user_nav_item['link'];

		echo apply_filters_ref_array( 'bp_get_displayed_user_nav_' . $user_nav_item['css_id'], array( '<li id="' . $user_nav_item['css_id'] . '-personal-li" ' . $selected . '><a id="user-' . $user_nav_item['css_id'] . '" href="' . $link . '">' . $user_nav_item['name'] . '</a></li>', &$user_nav_item ) );
	}
}

And to Call it:

<?php bp_get_displayed_user_nav(array('exclude' => array('profile' , 'settings'))); ?>

This example works as expected. Maintains the URL's and Slugs but removes the items from the nav menu without effecting $bp.

Attachments (1)

shop_tracker_dev.jpg (144.3 KB) - added by DennisSmolek 7 years ago.
Screenshot of a project

Download all attachments as: .zip

Change History (15)

#1 @boonebgorges
7 years ago

  • Keywords 1.7-early added
  • Milestone changed from Awaiting Review to Future Release

See also #2644. You're right that this should be possible without messing with the global.

An alternative strategy is to provide a function that does what it sounds like bp_core_remove_nav_item() and bp_core_remove_subnav_item() *should* do - namely, remove *just* the nav item, not access to the screen. Maybe adding a 'nav_only' parameter for these functions, so that your goal can be accomplished like this:

function bp_4209_remove_nav_items() {
    bp_core_remove_nav_item( 'profile', 'nav_only' );
    bp_core_remove_nav_item( 'settings', 'nav_only' );
}
add_action( 'bp_setup_nav', 'bp_4209_remove_nav_items', 100 );

and then modding bp_core_remove_nav_item() like so:

function bp_core_remove_nav_item( $parent_id, $type = 'all' ) {
    // ...

    if ( 'nav_only' != $type && $function = $bp->bp_nav[$parent_id]['screen_function'] ) {
    // ...
}

What I like about this approach, in contrast to DennisSmolek's original suggestion (which would definitely work) is that it keeps navigation management out of the template functions. As with the rest of BP, adding and removing nav items happens in a function hooked to bp_setup_nav. We should shoot to be internally consistent if possible. Also, there's a big advantage to having nav management happen outside of templates, because it makes it easy for plugins to modify navigation without requiring that site owners make manual changes to their template files.

Also: We could consider extending the $type strategy to the bp_core_new_*_item() function too, so that you could (for instance) add an arbitrary link to the nav bar (without having to add a BP screen function, as you have to do now). See #2389.

Opinions from other devs?

#2 @DennisSmolek
7 years ago

I agree it makes sense to keep it out of template files if possible, but without defining my own template function how would I display the nav multiple times? Example,
Nav array(a,b,c,d,e)

Nav1 has no (d,e): So a, b, c

Then later in the page I have another one,

Nav2 no (a,b) So c, d, e;

The only thing that comes to mind in the remove_*_item method would be to add another function to add those items back after you remove it, or I have to define my own function to display the nav items like the one above.

The snippet I wrote allows you to say keep the main nav in tact but use a modified version later in the page if needed.

Just as an example I'm attaching a screenshot of the site I'm working on, its still early in the dev but you can see the main nav and then the "Accounts" screen, which will the Profile and Settings Tab and their subnav's as well.

@DennisSmolek
7 years ago

Screenshot of a project

#3 @boonebgorges
7 years ago

Thanks for the helpful use case, DennisSmolek. I hadn't thought of this kind of situation. It might make sense for us to implement something like both ideas.

#4 @boonebgorges
7 years ago

  • Keywords 1.7-early removed
  • Milestone changed from Future Release to 1.7

Moving to 1.7 for further discussion. This would require a fairly big change in the way we load screens, so it'd have to be addressed early in the cycle for it to happen at all - it's a candidate for punting.

#5 @DennisSmolek
7 years ago

Sorry for my ignorance, why does it affect the screens? If you dont pass exclusions it works the same, but allowing the exclusion lets Devs that use BP beyond a simple network to create more complex screens without it all showing up in the nav.

#6 @boonebgorges
7 years ago

why does it affect the screens

Because currently, nav item registration and screen function registration happens in the very same place. Ideally, the two would be somewhat separate, but it's something that will need discussion.

I would rather avoid *only* fixing this problem by adding an exclude parameter to the template function bp_get_displayed_user_nav(), but if no other good ideas come up, then it's a relatively harmless enhancement.

#7 @johnjamesjacoby
7 years ago

  • Milestone changed from 1.7 to 1.8

No patch. Punting to 1.8.

#8 @johnjamesjacoby
7 years ago

  • Component changed from Members to Toolbar/BuddyBar

#9 @boonebgorges
6 years ago

  • Keywords needs-testing removed
  • Milestone changed from 1.8 to Future Release

I'm afraid we've run out of time to do something this big for 1.8.

#10 @DJPaul
3 years ago

  • Component changed from General - Toolbar/BuddyBar to Toolbar & Notifications

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


3 years ago

#12 @imath
3 years ago

Ideally, the two would be somewhat separate, but it's something that will need discussion

This is really important i agree. I'm doing this separation in my latest patch about #6026 (the function at the top of the src/bp-blogs/site/screens.php file) or see this comment

About the exclude args, i think we can now do it thanks to the BP_Core_Nav API.

#13 @DJPaul
20 months ago

  • Keywords trac-tidy-2018 added

We're closing this ticket because it has not received any contribution or comments for at least two years. We have decided that it is better to close tickets that are good ideas, which have not gotten (or are unlikely to get) contributions, rather than keep things open indefinitely. This will help us share a more realistic roadmap for BuddyPress with you.

Everyone very much appreciates the time and effort that you spent sharing your idea with us. On behalf of the entire BuddyPress team, thank you.

If you feel strongly that this enhancement should still be added to BuddyPress, and you are able to contribute effort towards it, we encourage you to re-open the ticket, or start a discussion about it in our Slack channel. Please consider that time has proven that good ideas without contributions do not get built.

For more information, see https://bpdevel.wordpress.com/2018/01/21/our-awaiting-contributions-milestone-contains/
or find us on Slack, in the #buddypress channel: https://make.wordpress.org/chat/

#14 @DJPaul
20 months ago

  • Milestone Awaiting Contributions deleted
  • Resolution set to maybelater
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.