Skip to:
Content

BuddyPress.org

Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#5122 closed enhancement (fixed)

Add Login/Logout/Register links to Appearance > Menus

Reported by: ubernaut's profile ubernaut Owned by: boonebgorges's profile boonebgorges
Milestone: 1.9 Priority: normal
Severity: normal Version:
Component: Core Keywords: has-patch needs-testing 2nd-opinion
Cc: ubernaut@…, math.viet@…, hugoashmore@…, mercijavier@…

Description (last modified by johnjamesjacoby)

Since we do not automatically create pages, in theory we could have a custom BuddyPress section for the pseudo-pages.

Related to #5096.

Attachments (6)

Screen Shot 2013-08-07 at 12.48.06 PM.png (107.6 KB) - added by ubernaut 11 years ago.
5122.03.diff (12.0 KB) - added by imath 11 years ago.
5122.04.patch (14.3 KB) - added by boonebgorges 11 years ago.
5122.05.patch (18.5 KB) - added by r-a-y 11 years ago.
5122.06.patch (18.4 KB) - added by boonebgorges 11 years ago.
5122.07.patch (18.0 KB) - added by boonebgorges 11 years ago.

Download all attachments as: .zip

Change History (44)

#1 @ubernaut
11 years ago

  • Cc ubernaut@… added

#2 @johnjamesjacoby
11 years ago

  • Summary changed from Add Login/logout/register meta-links options to the wp am din appearance menu to Add Login/logout/register meta-links options to the wp-admin appearance menu

#3 @johnjamesjacoby
11 years ago

  • Description modified (diff)
  • Summary changed from Add Login/logout/register meta-links options to the wp-admin appearance menu to Add Login/Logout/Register links to Appearance > Menus

#4 @johnjamesjacoby
11 years ago

Edited the description to clarify a bit more.

#5 @karmatosed
11 years ago

Just referencing the chat we had about this in the dev chat because I think it helps with some of the possible thinking of solutions: https://irclogs.wordpress.org/chanlog.php?channel=buddypress-dev&day=2013-07-31&sort=asc.

My own feelings are adding something to the WP Menu interface that isn't a page or post isn't a good idea. However, perhaps this should be looking more at reviewing the login widget.

#6 @karmatosed
11 years ago

  • Cc karmatosed@… added

#7 @ubernaut
11 years ago

added an image to clarify what i'm thinking about here

#8 @boonebgorges
11 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 1.9

Thanks for the image, ubernaut - the helper text made it clear in a way that it wasn't before.

karmatosed - I think that something like ubernaut suggests is not mutually exclusive with improvements to the login widget. We could consider doing each.

Putting this into 1.9 as I think it can be accomplished fairly easily. I'll post back after some experiments. Patches welcome from others if you get to it first :)

#9 @karmatosed
11 years ago

Looking at the image the only way I think we could do it would be to add a tab to menu. If there was a 'BuddyPress links' tab like custom links that would work. Otherwise it doesn't fit in to any of those blocks at all. I'm not sold this is right place just a possible way to fit in.

I do like idea that we review the login widget too that's cool. I think that still has uses.

#10 @ubernaut
11 years ago

i don't think it should be a tab because then it would not be part of any existing menu it would basically take over that entire menu allowing for nothing else, unless i am misunderstanding what you mean by 'tab'. i was thinking it would have its own box like custom or pages or forums but maybe theres only one thing that would be there so maybe it doesn't really deserve an entire box of it's own.

i do think that most sites (not meaning most bp or wp site but rather all sites) generally put the login info in the menu area. Regarding the widget i think thats good but not 100% effective because not all themes put the widgets in a good spot on the home page if at all.

#11 @karmatosed
11 years ago

@ubernaut: Yes you are misunderstanding. To the left you have tab blocks like 'custom links' I am proposing we have it there. It clearly doesn't fit into any of the the existing blocks there so slotting in a BuddyPress one makes sense.

You're actually not right about the login info being in the menu area. Most sites prefer to have a separate menu/section for login/logout menus. It's better UX that way. However, if we have this separate box someone could also do that so it works. This is also why the widget is a good idea to work with along with this. We should be encouraging people do not tack it onto existing menus though - that's not great for users.

#12 @ubernaut
11 years ago

i thought i was misunderstanding you because the only place with existing tabs on that page didn't seem to make much sense. So yeah glad we are on the same page there. :)

regarding the menu yes you are correct that the login/out/register button is usually in a separate menu (usually above and to the right at the very top of pages) and i do think having a widget option is good but the problem is that widgets are typically displayed far away from this area we are discussing. we cannot force a theme to have two menu areas it will have have as many as it does and i would agree as part of the regular menu is not ideal from a ui/ux perspective but in the vast majority of cases it will be far closer to where it belongs and more logically placed with the existing menu structure(s) then it would be by way of widget. i would also add that more and more themes are building multiple menu areas these days and in those cases we are very close to "the way its should be" for most sites.

#13 @ubernaut
11 years ago

just a little note, saw a forum request for some other dynamic links that might make sense to also put in that same menu items box:

My Profile
My Messages
My Groups
Edit Profile

http://buddypress.org/support/topic/dynamic-url-profile-links-for-wp-nav-menu/

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

#14 follow-up: @imath
11 years ago

  • Cc math.viet@… added
  • Keywords has-patch needs-testing added; needs-patch removed

Hi,

I think @ubernaut's idea is very interesting because, it's very easy to build a widget with a wp_nav_menu thanks to the Custom Menu Widget. So, using a wp_nav_menu can give 2 outputs one in the sidebar, and one in another theme's location - User has the choice. And this last feature is interesting in the case a theme is not using a sidebar.

So i worked on a patch, i had an interesting night.

I actually build 3 patches :

  • the first one was using a dropdown box to list the BuddyPress items and a javascript was listening to its change event to fill the 2 fields that a link wp_nav_menu uses except that i made them hidden fields. Problem in that case is that we have links and name but it's then quite difficult to identify the item is a BuddyPress one. And we need to as the items must be "dynamics". For instance if a user is logged in : the link is no more a login one but a log out and the register link should in this case not be displayed.
  • then, i built a huge one that was creating a custom post type that was available in wp_nav_menu but not in other part of the UI. This step was interesting as i've found very handy to use the buddypress()->bp_nav (i was also using the bp_options_nav in this patch) to populate the BuddyPress menu items. Using a custom post type makes it very easy to identify a menu item is a BuddyPress one, but i thought this was too heavy to use for a menu (the patch is creating pages and children pages).
  • Finally, i built this third one, it uses a custom walker in order to fill the classes attribute of a nav item by inserting some css classes that help me in identifying the slug of a BuddyPress item, then a filter helps me to dynamically get the current user BuddyPress nav links and to alternatively display a logout / login - register link(s).

If you're interested in the first 2 patches, i've made them available on my dropbox
https://dl.dropboxusercontent.com/u/2322874/5122.zip

In this ticket, i only attach the last one (5122.03.diff) because it's "the lightest" way i've found to make available a BuddyPress nav menu that seems to behave the right way.

These are links to 2 screen captures :

@imath
11 years ago

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

that is super cool. I'm gonna do some testing on that asap, might be a few days though.

#16 follow-up: @ubernaut
11 years ago

quick newb question sorry does this patch require trunk or would it also work with 1.8.1?

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

Replying to ubernaut:

quick newb question sorry does this patch require trunk or would it also work with 1.8.1?

It should work in 1.8.1 as the only new functions/classes i use is the ones i've added in the patch

#18 @ubernaut
11 years ago

works like a charm for me thanks and for the hand holding as well! let me know if theres anything in particular i should be testing aside from the fact that it appears to work without any issue i have noticed so far.

#19 follow-up: @boonebgorges
11 years ago

  • Keywords 2nd-opinion added

imath - Bravo! This is a really outstanding piece of reverse-engineering. The general technique is a little bit fragile (it depends on some specifics of the WordPress implementation of nav menus), but it is an extremely ingenious way of skirting the normal nav menu requirements. Really great work.

I've cleaned up your patch, in the form of 5122.04.diff. This patch mostly improves the documentation and brings the code in line with WP/BP coding standards. I also broke bp_get_nav_item_pages() into two functions (instead of passing an awkward $selection parameter) and added some caching so that the pages wouldn't be built more than once on a page load.

To the other devs: What imath has done here is, in a nutshell:

  • added a BuddyPress sidebar to Dashboard > Appearance > Menus
  • populated it with checkboxes for Log In/Log Out, Register, and all the top-level bp_nav links (like Messages, Friends, Groups, etc)
  • written a filter that will dynamically generate the appropriate links for the logged-in user and add them in the appropriate place in the nav menu on the front end. So, 'Messages' will not appear for logged-out users, while for 'boone' it will point to http://example.com/members/boone/messages; likewise, logged-in users will see 'Log Out' while logged-out ones will see 'Log In'.

I think it's overall really clever and could be really useful to lots of site owners.

One small interface question, which has a tricky underlying technical underpinning. I think it would be much clearer for the component-specific checkboxes to read "My Messages", "My Groups", etc, so as to distinguish them from the component directories like "Groups". However, while it's easy to tack on "My " in English, this won't work well in other languages. Play around with the interface a little to see if you agree.

#20 @ubernaut
11 years ago

wondering why this is harder in other languages.

#21 @boonebgorges
11 years ago

wondering why this is harder in other languages.

In English we can just tack the word "My" on the front and it works.

In other languages, this is not true. The syntax may be different. Words like "My" may be gendered. It may go before or after the noun (or both). It may require the noun to be declined in some sort of way. There's no cross-language way to concatenate strings like this. What we'd really need to do is to register new strings when setting up the component, so that translators could handle them manually. I didn't patch it this way because I first wanted to get a second opinion about whether the "My" thing was worth worrying about in the first place.

#22 @ubernaut
11 years ago

gotcha thanks for the clarification.

#23 @r-a-y
11 years ago

I haven't had a chance to play around with this patch yet, but looks great at first glance!

My only small critique is in bp_admin_wp_nav_menu_meta_box(), we should probably check for BP multiblog as well.

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

Replying to boonebgorges:

imath - Bravo!

Thanks Boone :)

One small interface question, which has a tricky underlying technical underpinning. I think it would be much clearer for the component-specific checkboxes to read "My Messages", "My Groups", etc, so as to distinguish them from the component directories like "Groups". However, while it's easy to tack on "My " in English, this won't work well in other languages. Play around with the interface a little to see if you agree.

I think we can simply add "user nav" to the BuddyPress metabox title to inform the menu concerns user navigation. Then, people can add the prefix of their choice to the navigation using the input for the nav item title as shown in this image : http://flic.kr/p/fLtqsr

Finally, i've tested 5122.04.patch​ and it's working great ;) Like Boone or ubernaut, I really think this feature is very interesting as admins will have the choice to use a nav or a menu widget to choose the place where they want to add it. So it's like the shampoo and the after shampoo : 2 in 1 ;)

#25 @karmatosed
11 years ago

Does 'my' mean anything to the person adding the links? I'd say it doesn't as it's the user messages/ group and 'my' is just relating to yours which this isn't the case. What about just saying that?

#26 @boonebgorges
11 years ago

karmatosed - Check out http://www.flickr.com/photos/im4th/9578344048/. IMO, it's not clear that, eg, the Activity checkbox at the right refers to _the current user's activity_ rather than _the global activity directory_. While you're correct that "my" would not be technically true (since it wouldn't be the super admin's Activity that the link pointed to), I think that "My Activity" would be clear enough in context. We could/should also probably change the helper and URL text (see "addresse web" in http://www.flickr.com/photos/im4th/9692270869/) to explain this a bit more clearly, something like "This navigation item will point to the logged-in user's Activity stream".

I'll see what I can do about whipping up another patch that shows what I mean. Maybe it'll be more (or less :) ) convincing if people can actually play with it.

#27 @ubernaut
11 years ago

So this already happens in the wp admin menu bar not sure if we can adapt the same solution. In multisite installs the term "My sites" is used twice. How is that term handled in other languages currently?

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

#28 @hnla
11 years ago

  • Cc hugoashmore@… added

#29 @mercime
11 years ago

  • Cc mercijavier@… added

@r-a-y
11 years ago

#30 @r-a-y
11 years ago

Nice work by everyone so far.

05.patch adds a few UX enhancements in the "Menus" admin page.

  • Adds tabs in the "BuddyPress" metabox - "Logged-in" and "Logged-out".
    • I've separated the links according to login state and also added a blurb above these tabs.
    • Hopefully, this addresses the concerns of the nav menu names. Feel free to tweak the verbiage.
    • Here's a screenie - http://i.imgur.com/oW3CjMC.png
  • Tweaks the JS by removing the "URL" field under each BuddyPress menu item.
  • I've also renamed some functions and prefixed "bp_nav_menu_" instead of using "bp_" as that was too general.

Let me know what you think.

#31 @karmatosed
11 years ago

Perhaps have 'user logged in' 'logged out' as that puts into context a bit?

#32 @karmatosed
11 years ago

  • Cc karmatosed@… removed

#33 @boonebgorges
11 years ago

r-a-y, I like the tabs. I think that goes a long way toward clearing it up.

With the tabs in place, though, I feel like the single "Log In/Log Out" button is kinda confusing (as evidenced by the fact that it needed its own paragraph of description). In 5122.06.patch, I broke Log In and Log Out into two separate links, appearing on separate tabs. This not only makes the tab situation less confusing, but it also makes the interface more consistent, because now the text "Log In" and "Log Out" can be edited in the same way as all the other nav menu titles. It may create a *little* bit of extra work for most people who are setting up menus, but on the other hand, there may be legitimate use cases where someone would want one of the links but not the other (or would want them in different places), so splitting them up is a bit more functional.

#34 @boonebgorges
11 years ago

I was showing this patch to ericlewis, and he suggested that maybe tabs aren't the best interface for this, because it sorta hides the logged-out items. 5122.07.patch creates two separate lists - have a look and see if it makes more sense.

#35 @r-a-y
11 years ago

I think 07.patch works.

Trivial thing: Should there be a <h4> before the paragraph blurb for each section?

#36 @boonebgorges
11 years ago

In 7426:

Introduce bp_alpha_sort_by_key()

This new utility function allows an array of complex items (objects or arrays)
to be sorted alphabetically, according to a specified value in each item. For
example, sort an array of user objects alphabetically by display_name:

$sorted_users = bp_alpha_sort_by_key( $users, 'display_name' );

See #5122

#37 @boonebgorges
11 years ago

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

In 7427:

Introduce a BuddyPress links box to nav-menus.php

The new BuddyPress metabox on Dashboard > Appearance > Menus allows site
administrators to build navigation menus that include dynamic, user-specific
BuddyPress links. Features:

  • Separate sets of links for logged-in and logged-out users.
  • Logged-in user links are automatically generated based on the logged-in user. For example, adding a 'Settings' item from the BuddyPress menu will create a menu item that points to the logged-in user's Settings page, and will not be shown to logged-out visitors.
  • The list of available Logged-In links is automatically populated with all components that are registered in the BuddyPress navigation (ie, those items that are visible as top-level nav links when viewing a user's profile).
  • Log In and Register links are visible only to logged-out users.

Fixes #5122

Props imath, r-a-y

#38 @ubernaut
11 years ago

rad thanks everyone!

Note: See TracTickets for help on using tickets.