WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#14145 closed task (blessed) (fixed)

Allow custom post types to be a submenu

Reported by: nacin Owned by:
Milestone: 3.1 Priority: normal
Severity: normal Version:
Component: Posts, Post Types Keywords: has-patch
Focuses: Cc:

Description

Patch adds show_in_menu to register_post_type $args. Defaults to true.

If === true, then it's a top-level menu. If false, then no menu. Otherwise, it's the parent page slug, like 'users.php' or 'edit.php?post_type=post'.

Patch mostly works. I'm finding that something in the menu handling code is giving wp-has-current-submenu classes to the 'Posts' menu even though the submenu is assigned to users.php.

Attachments (2)

14145.diff (4.1 KB) - added by nacin 4 years ago.
14145.2.diff (4.7 KB) - added by duck_ 4 years ago.

Download all attachments as: .zip

Change History (19)

nacin4 years ago

comment:1 in reply to: ↑ description duck_4 years ago

Replying to nacin:

Patch mostly works. I'm finding that something in the menu handling code is giving wp-has-current-submenu classes to the 'Posts' menu even though the submenu is assigned to users.php.

The state in the _wp_menu_output foreach loop for the Posts entry in $menu, when a custom post type set under tools.php is active, is:

global $parent_file = tools.php (set by get_admin_page_parent())
global $self = edit.php (from $_SERVER['PHP_SELF'])

$item[2] = edit.php (the Posts file)

Since there is no query string in the $parent_file var (as there normally would be for a custom post type) and $item[2] == $self the Posts menu gets given current status (menu-header.php, line 50)

comment:2 kevinB4 years ago

  • Cc kevinB added

comment:3 follow-up: t31os_4 years ago

  • Cc wp-t31os@… added

Andy i don't believe you're taking into consideration that post types usually have several subitems, and setting show_in_menu only registers the main/parent item for the post type (the link to edit.php?post_type=MYPOSTTYPE), missing other items such as "Add New", "Taxonomy 1", "Taxonomy 2" and so on... which would usually appear under a post type's given menu (as with any post type, posts for example). I personally think the function would serve little purpose if it doesn't register the other items along with the main link to edit.php ...

We can already effectively move items by way of hooking onto admin_menu and re-sorting or modifying the $menu array, etc..

Could you please clarify what this new parameter aims to do?

I've been dealing with the menu code over the last couple of weeks, and i'd be happy to help once i understand the goal.. :)

comment:4 in reply to: ↑ 3 nacin4 years ago

Replying to t31os_:

Andy i don't believe you're taking into consideration that post types usually have several subitems, and setting show_in_menu only registers the main/parent item for the post type (the link to edit.php?post_type=MYPOSTTYPE), missing other items such as "Add New", "Taxonomy 1", "Taxonomy 2" and so on... which would usually appear under a post type's given menu (as with any post type, posts for example). I personally think the function would serve little purpose if it doesn't register the other items along with the main link to edit.php ...

I think it comes down to use case. If the plugin wants to register a post type as a submenu, then obviously taxonomies would not appear under (or rather, next to) it, and they would have to register taxonomy menus on their own. Or, we could make it optional as well to show taxonomies as sibling submenu items.

We can already effectively move items by way of hooking onto admin_menu and re-sorting or modifying the $menu array, etc..

Could you please clarify what this new parameter aims to do?

It aims to make it easy to have a post type submenu item start as a submenu item. Currently it's a bit ridiculous to do. You need show_ui to be true, but then you need to unset the corresponding key for $menu, either by looping through the array to find it, or by setting menu_position to some high, unique-enough number then unsetting exactly that key. #14666 will alleviate the removal pain, but regardless you still need to add the submenu item when you're done. It's dirty, and cheap.

There are plenty of good use cases for having post types a submenu, and I think it should be much easier to do. I know jane is also +1 on this from a UI perspective because it will lead to less menu bloat. There are also good use cases for wanting the UI activated (show_ui), but for it not to appear in a menu at all. Hence fine-grained controls will be excellent.

comment:5 nacin4 years ago

  • Type changed from feature request to task (blessed)

comment:6 follow-up: t31os_4 years ago

Would you not consider the "Add New" link to be essential or would you consider having to navigate to the edit page first to click the "Add New" link there to be adequate?

I've also recently noticed a page not existing as a submenu item also renders that page inaccessible also due to a check performed in the user_can_access_admin_page function (IIRC) which in part validates access to a page based on the given page having a value inside the submenu array (came across this problem when moving menu items around myself). Did you test access to admin pages that don't have submenu items?

comment:7 in reply to: ↑ 6 nacin4 years ago

Replying to t31os_:

Would you not consider the "Add New" link to be essential or would you consider having to navigate to the edit page first to click the "Add New" link there to be adequate?

I would not, at all. As I said above, it comes down to the use case. Only if it makes sense for the post type to appear as a single submenu, without Add New and without taxonomies, should the plugin do it. We owe them that flexibility and their ability to make a decision.

I've also recently noticed a page not existing as a submenu item also renders that page inaccessible also due to a check performed in the user_can_access_admin_page function (IIRC) which in part validates access to a page based on the given page having a value inside the submenu array (came across this problem when moving menu items around myself). Did you test access to admin pages that don't have submenu items?

I'm not sure what you mean there. Example code?

comment:8 t31os_4 years ago

Apologies Andy, the problem i was describing was actually in relation to unsetting the menu altogether (my mistake).

At present, if you create your own admin menu (dropdown, or whatever) and you decide to unset the existing menu (why render the original if you're not using it?), this has a knock-on effect that causes user_can_access_admin_page to return false due to checks against items in the $submenu array (maybe this is why no admin menu plugins exist).

Realistically this is a seperate and possibly unrelated issue, and it's my fault for getting confused, so simply disregard my last response about inaccessible pages.

comment:9 nacin4 years ago

I would still need to see an example unfortunately. Not sure what you're trying to do.

comment:10 t31os_4 years ago

Sorry Andy, my info is a bit off, having taken a break from the menu code i got my errors muddled.

There is an underlying issue with unsetting or trying to remove the WP admin menu though, but where would be an appropriate place to detail the issue?

A new ticket here?
Mailing list?
The dev blog?
My blog?

Just trying to avoid derailing this ticket any further... ;)

duck_4 years ago

comment:11 duck_4 years ago

  • Keywords has-patch added

Patch checks for empty $typenow instead of lack of query string in $parent_file. Contains nacin's patch.

comment:12 mikeschinkel4 years ago

  • Cc mikeschinkel@… added

comment:13 boonebgorges4 years ago

  • Cc boonebgorges added

comment:14 DJPaul4 years ago

  • Cc DJPaul added

comment:15 husobj4 years ago

  • Cc ben@… added

comment:16 nacin4 years ago

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

(In [15844]) Introduce show_in_menu for register_post_type. Allows post types to be a submenu. Accepts boolean and also a parent base. With help from duck_. fixes #14145.

comment:17 nacin3 years ago

This ended up duplicating #13727. My bad :-)

Note: See TracTickets for help on using tickets.