Skip to:
Content

BuddyPress.org

Opened 11 years ago

Closed 11 years ago

#5322 closed defect (bug) (fixed)

Some menu items don't work correctly

Reported by: zachary-dubois's profile Zachary DuBois Owned by: imath's profile imath
Milestone: 1.9.2 Priority: normal
Severity: normal Version: 1.9.1
Component: Core Keywords: has-patch commit
Cc: zacharydubois@…

Description

Several items that were added to the menu in BuddyPress are incorrectly shown when the user is signed out and several are incorrectly shown when the user is signed in.

The signup menu item is shown only when logged in and it doesn't even link anywhere.


Information about my site:
I am running v1.9.1 of BuddyPress (on WordPress v3.8) with all BuddyPress modules activated and configured.
It is a WordPress multisite with subdomains. BuddyPress is network activated.

Attachments (3)

exclude-activate-register-from-pages-nav-menu-admin.diff (953 bytes) - added by imath 11 years ago.
5322.01.diff (493 bytes) - added by imath 11 years ago.
Screen Shot 2014-01-05 at 3.42.51 PM.png (60.5 KB) - added by Zachary DuBois 11 years ago.
Menu (Sign Up is the BP menu item)

Download all attachments as: .zip

Change History (28)

#1 @boonebgorges
11 years ago

  • Keywords reporter-feedback added

Can you share a screenshot, or more details about the "several items" that are shown incorrectly? In each case: (a) what did you do, (b) what did you expect to see, and (c) what did you actually see? Thanks!

#2 @Zachary DuBois
11 years ago

Problem 1)
Activity, Members, and Groups menu items are not displayed when the user is logged out. Those features of BuddyPress are always accessible to the public so why hide them. I am currently using the actual page for the menu item.


Problem 2)
Signup menu item is not shown when logged out and is shown when logged in. If I am logged in, why would I need to signup? When signed in, there is no actual link to the signup page either.


How to reproduce:
Install BuddyPress v1.9.1 on WordPress v3.8 and just add menu items in various ways.


My menu setup currently is

Activity
--| Members
--| Groups
--| Profile
--| --| Notifications
--| --| Messages
--| --| Friends
--| --| Settings
--| --| Logout
--| Login

#3 @Zachary DuBois
11 years ago

  • Cc zacharydubois@… added

#4 @boonebgorges
11 years ago

Zachary - Thanks for the additional details.

It's still not totally clear from the description you gave (not sure what "add menu items in various ways" means in particular), but it's likely that Problem 1 is by design. When you go to Dashboard > Appearance > Menus, and then open the BuddyPress panel, the Activity and Groups links appear under the "Logged-In" header. They are *specific to the logged in user*, and do not appear for logged-out users. There's a note there to this effect in the interface.

If you want to include persistent links to the Activity, Members, and Groups directories, select those items from the "Pages" panel, not the "BuddyPress" panel.

As for problem 2: It's not clear to me where and how you are adding the link. Are you selecting "Register" from the Logged-Out section in the BuddyPress panel? If so, I'm afraid I'm unable to reproduce the issue. Or are you selecting the link from somewhere else?

#5 @Zachary DuBois
11 years ago

But as for problem 2, I did add the registration menu from the signed out section of the BuddyPress panel.

If that was intended purpose of problem 1, then I am fine with that :)

#6 @imath
11 years ago

Hi Boone, Zachary,

Someone reported something approaching on the the bp-ft.net forum, i've made some tests and was unable to reproduce. But problem number 2 is worrying me, so i will check it again today and will be back on this ticket as soon as i understand what's going wrong. Because, signup (register) item menu should only be shown to non logged in users.

#7 @imath
11 years ago

Zachary,

The only thing i did manage to reproduce was having registration link for a logged in user. In his first comment Boone asked you to attach a screenshot of your Menu Administration screen. I think this would be very useful to help me reproduce the bug if any.
Could you also share a link to your website so that i can see the menu for non logged in user and see the problem live ?

On bp-fr.net i've attached a type of screenshot i need : http://bp-fr.net/buddydrive/file/buddypress-user-nav-menu/ , as show in the red box, could you detail each menu items you are using ?
Please make sure the trouble is still there when sharing your screenshot.

Once done, if you want you can delete the registration item from your menu and then apply the exclude-activate-register-from-pages-nav-menu-admin.diff file, i've attached to this ticket and test again to see if the registration link is still showing to logged in user.
Thanks for your help.

So, to have the registration link shown to the logged-in user, the only way i've found is to use the item in the "Pages" meta box.
I've tried various ways, like the one you're describing in the "menu setup" you've shared in comment 2, and it always behave the best way. For instance, if non logged in the register link was showing even if it was a sub-item of a logged-in user item.

Boone, it may not be the best place, but i think we should filter the list of links displayed in the "Pages" meta box to avoid displaying at least the Activation page, but i think now that the BuddyPress user nav menu exists we should also avoid displaying the Registration page (see diff attached).

#8 @imath
11 years ago

  • Keywords has-patch added

I was able to figure out what is going wrong with the 'Registration' BuddyPress user nav link thanks to the chat i had with the member of bp-fr.net website.

If the post_name (in other words the slug) of the registration page is not 'register', then we have a problem. The display of the registration link is managed in bp_setup_nav_menu_item() (bp-core-filters.php)
The check looks for 'register' into the class name bp-(.*)-nav, so if the slug of the registration page is 'signup' the class is set to bp-signup-nav in bp_nav_menu_get_loggedout_pages() and the condition will never matched causing some mess (register link displayed to the logged in user instead of the logged out one).

As in this part, the slug is only used to actually build the bp-(.*)-nav, i think the best is to do as for the login link : just use 'register' instead of the post_name (see 5322.01.diff attached to this ticket)

This will fix the trouble for the registration link.

@Zachary DuBois can you confirm 5322.01.diff is fixing your trouble with the registration link (PS : you'll need to first remove the registration link from the menu, to add it back)

@imath
11 years ago

#9 follow-up: @Zachary DuBois
11 years ago

That fixed the fact that is wasn't shown to logged out users and the fact that it did not link. However, it is still shown to logged in users.

#10 @boonebgorges
11 years ago

  • Milestone changed from Awaiting Review to 1.9.2

Since the 'register' issue is related to BP 1.9, let's fix it in a maintenance release. Moving this ticket to 1.9.2.

imath - Let's discuss your proposal for filtering out the pages in a separate enhancement ticket. I've gotta think through the implications here ("where did my pages go?" confusion; legitimate use cases where someone might want to put Register in a menu for all users), and I don't want it to hold up the more immediate issue.

#11 in reply to: ↑ 9 @imath
11 years ago

Replying to Zachary DuBois:

That fixed the fact that is wasn't shown to logged out users and the fact that it did not link. However, it is still shown to logged in users.

Once again, please provide what i've asked you in comment number 7 : at least a screenshot of your Administration Menu screen.

Boone, i'm creating another ticket for my proposal.

@Zachary DuBois
11 years ago

Menu (Sign Up is the BP menu item)

#12 follow-up: @Zachary DuBois
11 years ago

Attached. My menu I have it on is fairly large. So this is only a portion of it.

#13 in reply to: ↑ 12 @imath
11 years ago

Replying to Zachary DuBois:

Attached. My menu I have it on is fairly large. So this is only a portion of it.

Thanks, too bad we can't see all the items.
1/ In your comment number 2 you described your menu where Activity was the main item and others sub items. What is the type of this Activity item (Custom or Page). Are the links of Problem 1) displayed for a non logged in user ?

2/ In the top right of the Menu Administration screen, there's a tab named Screen options, if you click on it you can activate the 'CSS classes' checkbox, then you can click on the arrow at the right of your signup item.
For the Registration BuddyPress user nav menu item, values for the 'CSS Classes (optional)' field should be (if you applied the patch) :
bp-menu bp-register-nav

If it's not the case, check the patch is active, then remove the signup item, save your menu, and add the signup item back. The field 'CSS Classes (optional)' should now be as described above. And the register link should only show to non logged in users.

#14 follow-up: @Zachary DuBois
11 years ago

That was the case, I took out the signup form because it did not work. I added it back to the bottom just to see how it worked.

As for the CSS classes, I see

bp-menu bp--nav

It is still shown for logged in and logged out users.

#15 in reply to: ↑ 14 @imath
11 years ago

Replying to Zachary DuBois:

It is still shown for logged in and logged out users.

Hi,

Just to be sure : what is the content of lines 1778 to 1785 of the file bp-core/bp-core-functions.php ?

#16 follow-up: @Zachary DuBois
11 years ago

function bp_nav_menu_get_item_url( $slug ) {
        $nav_item_url   = '';
        $nav_menu_items = bp_nav_menu_get_loggedin_pages();

        if ( isset( $nav_menu_items[ $slug ] ) ) {
                $nav_item_url = $nav_menu_items[ $slug ]->guid;
        }

#17 in reply to: ↑ 16 @imath
11 years ago

Replying to Zachary DuBois:

Really sorry Zachary, i forgot i was using trunk version. In 1.9.1 Lines i need are 1727 to 1734

#18 follow-up: @Zachary DuBois
11 years ago

It's cool

        if( ! empty( $bp_directory_page_ids['register'] ) ) {
                $register_page = get_post( $bp_directory_page_ids['register'] );
                $bp_menu_items[] = array(
                        'name' => $register_page->post_title,
                        'slug' => $register,
                        'link' => get_permalink( $register_page->ID ),
                );
        }

#19 in reply to: ↑ 18 @imath
11 years ago

Replying to Zachary DuBois:

It's cool

Thanks a lot Zachary, i realize my explanation about 5322.01.diff was not detailed enough. Sorry for this. If your look again at the patch, you'll see that the slug argument for the $bp_menu_items[] is 'register' and not $register.

As $register is unset it explains the CSS classes is bp-menu bp--nav.

The patch is setting the slug argument to 'register' in order to have the CSS classes set to bp-menu bp-register-nav.

So you simply need to replace at your line 1731 'slug' => $register, by 'slug' => 'register', and it should fix the trouble as soon as you also remove the signup link, save the menu and add the signup item back.

If so, it will confirm it's a possible way to solve the issue, thanks for your feedback.

#20 follow-up: @Zachary DuBois
11 years ago

Sorry about that. The CSS properties now applied are

bp-menu bp-register-nav

It also has fixed my issues with it appearing for signed in users. It is properly linked to the page aswell.

#21 in reply to: ↑ 20 @imath
11 years ago

Replying to Zachary DuBois:

It also has fixed my issues with it appearing for signed in users. It is properly linked to the page swell.

Nice :) Thanks for your feedback

#22 @Zachary DuBois
11 years ago

No prob! Thanks for fixing it :P

#23 follow-up: @boonebgorges
11 years ago

  • Keywords commit added; reporter-feedback removed

imath, would you like to do the honors? :)

#24 in reply to: ↑ 23 @imath
11 years ago

Replying to boonebgorges:

imath, would you like to do the honors? :)

ahah! great minds :) I was thinking it was a good candidate for my very first commit!! But before doing so, i would like to chat with you just to be sure i'm not breaking anything ;) i'll ping you tonight.

#25 @imath
11 years ago

  • Owner set to imath
  • Resolution set to fixed
  • Status changed from new to closed

In 7734:

Make sure the register item of a dynamic BuddyPress nav menu is displayed to a logged out user

In case the slug of the Register directory page has been changed, the register item was not displayed to logged out user.
Instead of the Register directory page slug, we need to use the string "register" in a similar approach to the login item.

Fixes #5322

Note: See TracTickets for help on using tickets.