#6509 closed enhancement (fixed)
Add BuddyPress Menus to Customizer
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 2.3.3 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Core | Keywords: | has-patch |
Cc: |
Description
Because menu management has been added to the Customizer https://core.trac.wordpress.org/changeset/32806
Attachments (6)
Change History (34)
#1
@
10 years ago
- Milestone changed from Awaiting Review to 2.4
- Type changed from defect (bug) to enhancement
#2
@
10 years ago
#3
@
10 years ago
@imath Thank you for taking a look at this ticket and your patch, plus two thumbs up for the ticket and patch in core, looks great! I attached a screenshot for 2014 theme :) I'm also watching another ticket in core - to allow the customizer to be made wider.
#4
@
10 years ago
@mercime you're welcome :) Thank you for the screenshot on 2014. Watching WP32708 to see how my request will evolve, and to eventually refresh the patch depending on WordPress decisions.
#5
@
10 years ago
Unfortunately, WP32708 has been delayed to a future release :(
I don't think we should absolutely try to be in the customizer by faking a post type and change how we're doing so far to have a dynamic menu based on the current user.
We should only make sure to avoid generating a notice error in the customizer for 4.3, and wait for WP32708 for the rest...
see 6509.avoid-notice.patch
#6
@
10 years ago
@imath Looks like there's still hope for your ticket in core. Let's see if it happens in 4.3 beta 3 :)
#8
@
10 years ago
6509.02.patch is updated to latest evolutions on WP32708. You'll need the patch @valendesigns shared on the WordPress core ticket to test it.
#10
@
10 years ago
- Keywords needs-refresh added
if ( 0 !== $page || 'bp_nav' !== $obj_type ) { return $items; }
Is this condition really necessary? You have the code below it checking for $obj_name
and retuning $items
when it's not a BP menu object. It just seems redundant to me.
Also, when setting up the $items
array the classes key should have a string value. Otherwise, the output string for classes will have commas in it because the Customizer does not convert the array properly.
So this line:
'classes' => array( 'bp-menu', "bp-{$bp_item->post_excerpt}-nav" )
Becomes
'classes' => "bp-menu bp-{$bp_item->post_excerpt}-nav"
#11
@
10 years ago
Thanks for your suggestions @valendesigns
I'll update the patch as soon as the WP ticket will be committed.
I'll include your suggestions except that i'll keep
if ( 0 !== $page ) { return $items; }
Else it would load the same items again and again as soon as you scroll in the available nav items of the customizer.
#12
@
10 years ago
Good point! Although, scroll should only trigger when there are more than 10 items, so I guess it is something to consider on a case by case basis. If you have only a couple menu items though, an additional page load should not be triggered when you scroll. Just an FYI for those listening to the conversation.
#13
@
10 years ago
scroll should only trigger when there are more than 10 items
Thanks for your comment, and you are completely right! I completely forgot plugins can add BuddyPress user nav items, so we can have more than 10 items.
Next patch will "paginate" BuddyPress nav items. If i understand well the "load more" in the customizer, it's triggered as soon as the reply is not an empty array. Just tested with 11 pages and it seems to be the case.
#15
@
10 years ago
Filters have been committed to WP Trunk :)
6509.04.patch is adapted to it.
@mercime could you test it to double check everything is ok ?
#16
@
10 years ago
@imath tested it on WP/BP trunk plus your patch and it's working well :) There's something wonky going on in IE 8, but more likely that it's not due to BP but the customizer.
#17
@
10 years ago
thanks for your tests @mercime, i'll check IE8 from work to be sure it's not a side effect of the patch. Then i think we'll be able to have this in. I'm just wondering if we should wait for 2.4 to have this in or to have it in for 2.3.3 (branch 2.3). I'd like to have @johnjamesjacoby 's opinion about it. My personal opinion is : if 2.3.3 will be there to include some fixes for WP 4.3 (eg: #6465), then maybe we should include it or at least a part of if (the notice thing - first part of the patch) in 2.3.3.
This ticket was mentioned in Slack in #buddypress by imath. View the logs.
10 years ago
#19
@
10 years ago
If we can safely make the 2.3 branch work with WP4.3, that would be ideal. If not, or if it's too new or the implementation needs more work, a notice in 2.3 seems okay, but not great.
I'll try and review the patch this weekend, but I'm sure it's better than what we have if it's broken currently.
#20
@
10 years ago
Thanks a lot for your feedback @johnjamesjacoby :)
It's not broken. Setting a BP User navigation using the "old" UI is working great. When using the customizer to build nav menus, we have a notice error and setting the BP User Navigation is just not available yet. Thanks to the 2 filters introduced in WP33366 and the patch it will be possible to set a BP User Navigation items directly into the customizer.
This ticket was mentioned in Slack in #buddypress by imath. View the logs.
10 years ago
#23
@
10 years ago
@imath, not seeing any trouble with IE8 now. Must be the trunk version I downloaded when I tested it. Many improvements made since then. Thank you :)
#24
@
10 years ago
- Milestone changed from 2.4 to 2.3.3
mercime: Great!
As discussed during our latest dev-chat i will commit it in the coming days in branch 2.3, making sure to add a context to strings (_x()
) and test there are no side effects on WP < 4.3.
Thanks everyone for your help.
+1 As i'll explore the customizer for sites profile photos in #6026, i will look at this one for sure!