WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 2 years ago

#25122 reopened enhancement

First nav menu automatically adds all pages (including sub pages) but doesn't preserve page hierarchy

Reported by: jamescollins Owned by: nacin
Milestone: Future Release Priority: normal
Severity: normal Version: 3.6
Component: Menus Keywords: needs-patch needs-testing
Focuses: Cc:

Description

When you first visit /wp-admin/nav-menus.php in WordPress 3.6, if you don't already have a menu defined it automatically creates a new menu with all published pages in the menu.

On sites with many published pages and particularly sites with multiple levels of pages (child/parent pages), all of those pages are added as top level nav menu items.

(After the first menu is created, subsequent new menu requests create empty menus with no menu items in them).

The functionality was introduced in [23441].

Previous versions (such as 3.5) didn't automatically create a new menu.

Instead, when the first nav menu is automatically created I think we should either:

  1. only add all top level pages by default, or
  2. add all pages (including sub pages) but preserve the page hierarchy into the nav menu.

My preference would be option 1.

Attachments (4)

25122.diff (928 bytes) - added by jamescollins 4 years ago.
25122.2.diff (928 bytes) - added by jamescollins 4 years ago.
25122.3.diff (740 bytes) - added by lessbloat 4 years ago.
25122.4.diff (740 bytes) - added by lessbloat 4 years ago.

Download all attachments as: .zip

Change History (22)

@jamescollins
4 years ago

#1 @jamescollins
4 years ago

  • Keywords has-patch added

25122.diff implements suggestion 1 - only top level published pages are automatically added when creating the first nav menu.

Last edited 4 years ago by jamescollins (previous) (diff)

#2 @sennza
4 years ago

  • Cc bronson@… added

#3 @SergeyBiryukov
4 years ago

  • Milestone changed from Awaiting Review to 3.7

#4 @SergeyBiryukov
4 years ago

  • Keywords commit added

#5 @nacin
4 years ago

#page-all > ul > li > label > input[type="checkbox"] looks pretty fragile.

@jamescollins
4 years ago

#6 @jamescollins
4 years ago

25122.2.diff​ now uses a slightly less fragile selector:

#pagechecklist > li > label > input[type="checkbox"]

The child selector seems to be the easiest way to only select the top level pages, however I'm open to suggestions if anyone can think of a better way of achieving the same thing.

#7 @nacin
4 years ago

Fine with this. Asked lessbloat to look.

@lessbloat
4 years ago

#8 @lessbloat
4 years ago

Replying to jamescollins:

#pagechecklist > li > label > input[type="checkbox"]

Yep, this will work.

25122.3.diff​ is another approach that's perhaps slightly less fragile.

#9 @SergeyBiryukov
4 years ago

In 25122.3.diff, parents() can probably be replaced with closest(), since it turns out to be faster (per the IRC discussion for #25112).

Last edited 4 years ago by SergeyBiryukov (previous) (diff)

@lessbloat
4 years ago

#10 @lessbloat
4 years ago

25122.4.diff​ uses closest() instead of parents().

#11 @nacin
4 years ago

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

In 25622:

Only show top-level items when adding pages to a User's First Nav Menu. (This is also the title of Dave's first children's book.)

props lessbloat.
fixes #25122.

#12 @ocean90
4 years ago

Related: #25445

#13 @ocean90
4 years ago

In 25687:

Revert [25622]. fixes #25445. see #25122.

#14 @ocean90
4 years ago

  • Keywords needs-patch added; has-patch commit removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

This needs a new patch.

#15 follow-up: @lessbloat
4 years ago

Hey @ocean90,

Can you provide some additional context behind the revert? What about this patch didn't work?

#16 in reply to: ↑ 15 @helen
4 years ago

Replying to lessbloat:

Can you provide some additional context behind the revert? What about this patch didn't work?

The Ajax issue as described in #25445.

#17 @nacin
4 years ago

  • Milestone changed from 3.7 to Future Release

#18 @chriscct7
2 years ago

  • Keywords needs-testing added
Note: See TracTickets for help on using tickets.