Skip to:
Content

BuddyPress.org

Opened 5 years ago

Closed 5 years ago

Last modified 3 years ago

#5810 closed defect (bug) (fixed)

Side effect of r8821 on WP Nav Menus and WP List Pages

Reported by: imath Owned by: djpaul
Milestone: 2.1 Priority: high
Severity: normal Version:
Component: Templates Keywords: has-patch dev-feedback commit
Cc:

Description

Since r8821, i've noticed this :

https://farm4.staticflickr.com/3856/14883321666_94a7001dab_z.jpg

In WP Nav Menus or WP List pages, when viewing single member pages or single group pages, the "parent" directory is no more the current item in nav menus.

Reason is r8821 is resetting the post when not a directory page, so item menu is no more the current item.

I don't know if "rebuilding the current parent page" in menus should be a theme concern or BuddyPress one. But, just in case we should handle this, i suggest the attached patch

Attachments (2)

5810.patch (3.6 KB) - added by imath 5 years ago.
5810.02.patch (1.8 KB) - added by r-a-y 5 years ago.
Updated patch for trunk.

Download all attachments as: .zip

Change History (12)

@imath
5 years ago

#1 @boonebgorges
5 years ago

  • Keywords dev-feedback added
  • Milestone changed from Awaiting Review to 2.1
  • Priority changed from normal to high

r-a-y - Can you chime in here? The fact that this is cropping up in the case of nav menu current state makes me fear that there are going to be other odd consequences of wiping out these globals.

#2 follow-up: @r-a-y
5 years ago

Nice eye, imath. Sorry for breaking your theme :)

When root profiles is on, the "Members" page doesn't get highlighted without r8821 as well. When we move to rewrite rules, the same problem will occur.

I know that isn't an excuse :), so I'm looking to see what else we can do. I'll probably reinstate the page look up, but keep the reset post option in place.

Will do some further testing and report back.

#3 @r-a-y
5 years ago

02.patch reinstates the page look up and post globals and should restore the nav menu highlighting as before. Please test and let me know what you think.

Root profile nav menu highlighting does not work, but this is not a regression.

I still get to remove the SQL query, which is what I wanted :)

@r-a-y
5 years ago

Updated patch for trunk.

#4 in reply to: ↑ 2 @imath
5 years ago

Replying to r-a-y:

Nice eye, imath. Sorry for breaking your theme :)

No problem at all :) At the beginning, I thought i was the one who broke it while on another project!

Just tested 02.patch and it solved the issue.Thanks a lot.

#5 @DJPaul
5 years ago

  • Keywords commit added

#6 @djpaul
5 years ago

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

In 8932:

Fixes a regression in r8821 in rbp_core_load_template() which was preventing the correct parent page being highlighted in WP nav menus.

This change reinstates the page look up and post globals as before r8821. Overall, these changes are still a net benefit as an uncached SQL query has been removed. See #5241 for background.

Fixes #5810

#7 @DJPaul
5 years ago

In my haste to get this committed, I forget to credit imath and r-a-y in the commit message. I am very sorry. :(

Last edited 5 years ago by DJPaul (previous) (diff)

#8 @r-a-y
5 years ago

No biggie!

This ticket was mentioned in IRC in #buddypress-dev by jjj. View the logs.


5 years ago

#10 @DJPaul
3 years ago

  • Component changed from Appearance - Template Parts to Templates
Note: See TracTickets for help on using tickets.