Better structure for admin $menu

[scribu]

Currently, the global $menu variable is one big linear array:

$menu = array(
    [2] => array('Dashboard', ...
    [4] => array('', 'read', 'separator1', ...),
    [5] => array('Posts', ...)
    ...
)

To allow plugins to add a menu item at the end of a group, we use a bunch of additional global variables that remember the last element in each group.

It's very low level.

Things would be a lot easier if groups were built in:

$menu = array(
    'dashboard' => array(
        [5] => array('Dashboard', ...
    ),

    'object' => array(
        [5] => array('Posts', ...)
        [10] => array('Media', ...)
        ...
    ),

    'utility' => array(
        [5] => array('Posts', ...)
        [10] => array('Media', ...)
        ...
    ),
)

With this structure, separators could be inserted automatically in the right places. Also, you wouldn't have to worry about adding 'menu-top-first' and 'menu-top-last' classes.

In terms of backwards compatibility, only the $position parameter from add_menu_page() would be affected.

Whatever we do we would need this to be fully back compat.

Plugins that specify a $position would just get their menus created at the bottom. I think that's an acceptable degree of backwards compatibility-ness.

Anyway, lots of time to think about this until the 3.1 cycle starts.

Replying to scribu:

Plugins that specify a $position would just get their menus created at the bottom. I think that's an acceptable degree of backwards compatibility-ness.

+1 Like the sound of that! :D

The current system 'allows' for overriding of items, that is to say if you specify a $position that already exists it will simply overwrite it. I'm not sure if this was intentional or not but it seems like strange behaviour to me. I can see that you might wish to allow people to override items, but it just seems to easy to do by accident. I'd personally like to see this as an option, but by default shift items down out of the way. ($position_override=FALSE for example).

Interesting approach by MikeSchinkel:  https://gist.github.com/792b7aa5b695d1092520  https://gist.github.com/65dd845e9637f621b73b

cags: It does allow override in some cases, but not in others. I think menu_position for register_post_type ensures no conflict, or maybe it ensures no conflict when it is then pushing them onto the $menu array.

Related #14666, removing menus and submenus.

Replying to nacin:

cags: It does allow override in some cases, but not in others. I think menu_position for register_post_type ensures no conflict, or maybe it ensures no conflict when it is then pushing them onto the $menu array.

cags?

Also note that I wrote those to be 100% compatible with the current structure.

They could be added to core and used for 3 or 4 versions down the road and at the same time discourage all direct access.

By about v3.5 we could potentially modify the underlying structure to be more rational, deprecate the methods I have related to refresh, and just let the really old plugins break.

Potentially good transition strategy?

Replying to mikeschinkel:

cags?

It was in reply to  http://profiles.wordpress.org/cags - see comment above.

Also note that I wrote those to be 100% compatible with the current structure.

That's good. Excellent, in fact. I don't think we'll be looking too far ahead (you reference 3.5 for example) but severing plugins from direct access is indeed the next step. That said I doubt you'll see a refactoring of the underlying storage on a roadmap in the near future, as long as an abstraction layer does everything we need it to.

Replying to nacin:

Replying to mikeschinkel:

cags?

It was in reply to  http://profiles.wordpress.org/cags - see comment above.

Arghh, those cryptic user names, always tripping me up! ;-)

Also note that I wrote those to be 100% compatible with the current structure.

That's good. Excellent, in fact. I don't think we'll be looking too far ahead (you reference 3.5 for example) but severing plugins from direct access is indeed the next step. That said I doubt you'll see a refactoring of the underlying storage on a roadmap in the near future, as long as an abstraction layer does everything we need it to.

I was just trying to think through all the angles. It's not a perfect API because, for example when you look at the object in the debugger you don't see the menus and because it has to be refreshed, but it's fully abstracted. I guess it would/should be possible to add some functions that will build an object structure for those who want to see if via print_r(), etc.

Anyway, love feedback on this or to contribute in any way. I'm itching to provide some material contribution to core finally vs. the one line patch in v3.0. :)

Mike, how would you remove a single menu item with your API?

Replying to scribu:

Mike, how would you remove a single menu item with your API?

What I presented was a proof of concept and not a fully fleshed-out example. That said, I just spent time and updated to it provide a delete_admin_menu_item() to encapsulate the object calls and thus make it easier for the average guy (and I found and fixed a few bugs. :)

Here's the gist/download link:

 https://gist.github.com/792b7aa5b695d1092520

So let's say you wanted to delete the "Editor" items for both "Appearance" and "Plugins" menu sections. The first one requires you to set a priority of 102 because the Editor item is added using a priority of 101:

add_action('admin_menu', 'my_admin_menu',102);
function my_admin_menu() {
  delete_admin_menu_item('Appearance','Editor');
}

The second one requires a regular expression because the Plugin menu section title has a <span></span> that follows it to display the number of plugins that need to be updated so I use a simple RegEx to match:

add_action('admin_menu', 'my_admin_menu');
function my_admin_menu() {
  delete_admin_menu_item('Plugins .*','Editor');
}

Another I could do these is to use the slugs, i.e.

add_action('admin_menu', 'my_admin_menu',102);
function my_admin_menu() {
  delete_admin_menu_item('themes.php','theme-editor.php',array(
  	'find_section_by' => 'slug',
  	'find_item_by'    => 'slug',
  ));
  delete_admin_menu_item('plugins.php','plugin-editor.php',array(
  	'find_section_by' => 'slug',
  	'find_item_by'    => 'slug',
  ));
}

As I write that last example I think maybe it would be better if I modified it to "sniff" out the distinction between title, slug, and/or index; i.e. look first to see if is_numeric(), if not then look to match slug, if not look to match title. Your thoughts?

BTW, I'd be happy to evolve of change this in whatever direction would be needed to use it in core, if potential. Let me know.

-Mike

As I write that last example I think maybe it would be better if I modified it to "sniff" out the distinction between title, slug, and/or index; i.e. look first to see if is_numeric(), if not then look to match slug, if not look to match title. Your thoughts?

I think we should just use slugs. Indexes are what we're trying to get away from and titles are translatable.

As such, you wouldn't need the third parameter for delete_admin_menu_item().

Replying to scribu:

I think we should just use slugs. Indexes are what we're trying to get away from and titles are translatable.

Good point about indexes. When writing code for my own use they made sense, but as a potential inclusion into core I agree they probably aren't needed.

As such, you wouldn't need the third parameter for delete_admin_menu_item().

Agree.

That said, I'd hate to loose the titles even though translatable.

I'd expect 85% of use for this would be people developing custom themes for a specific client so they only ever have to test with one site, and 15% for use in commercial themes plugins. For plugins I think the slugs make much better sense but for the average theme developer I know it took me a while to wrap my head around how the slugs were used so I think the ease of use of titles would have a huge "usability" benefit.

For the titles we can of course test against the translated version too.

So for those who barely know what they are doing they can use the easy way with titles and those who know what they are doing can use the more robust and performant way with slugs. This could easily be documented as 1.) the easy way for single sites or 2.) the robust way for commercial themes and plugins, and we don't need the 3rd parameter because we can do a case-sensitive comparison for slug first then for title.

If you agree I can give it a go later today and refactor to simplify the code. Also, do you see other functions to implement?

-Mike

Ok, I guess also looking at the title makes sense.

Looking forward to the cleaned up patch.

Replying to scribu:

Ok, I guess also looking at the title makes sense.

Looking forward to the cleaned up patch.

As promised, major positive changes. Your input helped me greatly to simplify it:

 https://gist.github.com/792b7aa5b695d1092520

I removed the $args parameter and now lookups are done first via slug and if that fails then via Title. I did not (yet?) perform Title translations because I was thinking maybe we shouldn't; that way use of Titles would really only apply for specific sites. I could possibly add a check for a constant something like 'FIND_ADMIN_MENUS_BY_TITLE' that must be set for Find-By-Title to work. That way they'd have to find the docs to learn how to use which could say to only use Titles for individual sites, not for distributed themes nor for distributed plugins.

One thing I did was recode it so that any properties or methods of the objects that might expose the internal structure became private. The idea is that the developer should not be able to use this API in a manner that would disallow us to later replace the underlying structure with something more rational (as you requested above) and not break the developers code. Because of this constraint I placed on the code architecture I had to write some code less than straightforward in one place but I think we'll greatly benefit down the road by this encapsulation.

Also, I used the terms "Menu Section" and "Menu Item" because AFAICT there hasn't been consistent naming and the names that have been used are unclear (at least to me.) If there are terms that others would prefer, I can change.

Here is some example use cases:

  // This example creates one menu in place of Dashboard called "My Menu", adds a few things, and removes all else.
  // This example assumes this might only be done for end users, not administrators

  $dashboard = rename_admin_menu_section('Dashboard','My Menu');
  delete_admin_menu_item($dashboard,'index.php');         // Dashboard
  delete_admin_menu_item($dashboard,'update-core.php');   // Updates
  
  $movies = "edit.php?post_type=movie";
  copy_admin_menu_item($dashboard,$movies);
  $movie_genre = 'edit-tags.php?taxonomy=movie-genre&post_type=movie';
  copy_admin_menu_item($dashboard,$movies,$movie_genre);
  rename_admin_menu_item($dashboard,$movie_genre,'Movie Genre');
  delete_admin_menu_item($movies);
  delete_admin_menu_item($movies,$movie_genre);
  delete_admin_menu_item($movies,'post-new.php?post_type=movie');
  delete_admin_menu_section($movies);
  
  $actors = "edit.php?post_type=actor";
  copy_admin_menu_item($dashboard,$actors);
  delete_admin_menu_item($actors);
  delete_admin_menu_item($actors,'post-new.php?post_type=actor');
  //delete_admin_menu_section($actors);
  
  rename_admin_menu_item($dashboard,'Pages','Other Pages');
  
  delete_admin_menu_section('edit.php');                  // Posts
  delete_admin_menu_section('upload.php');                // Media
  delete_admin_menu_section('link-manager.php');          // Links
  delete_admin_menu_section('edit-comments.php');         // Comments
  delete_admin_menu_section('edit.php?post_type=page');   // Pages
  delete_admin_menu_section('plugins.php');               // Plugins
  delete_admin_menu_section('themes.php');                // Appearance
  delete_admin_menu_section('users.php');                 // Users
  delete_admin_menu_section('tools.php');                 // Tools
  delete_admin_menu_section('options-general.php');       // Settings

This is definitely still an alpha; no hooks, no real code documentation, etc. but I wanted to get a working version up to get feedback. Let me know your thoughts.

-Mike

The examples look great, but the code still needs to be compatible wth PHP4 for it to be included in WP 3.1.

Replying to scribu:

The examples look great, but the code still needs to be compatible wth PHP4 for it to be included in WP 3.1.

I haven't paid much attention to what isn't supported in PHP4; I can research but can you point out any specifics you see which our incompatible?

These are the things that aren't supported in the antique version of PHP4:

• static method calls: WP_AdminMenuSection::add($section,$args);
• access modifiers for methods or properties:
  • private function add(...)
  • private $index;

Replying to scribu:

These are the things that aren't supported in the antique version of PHP4:

• static method calls: WP_AdminMenuSection::add($section,$args);
• access modifiers for methods or properties:
  • private function add(...)
  • private $index;

Figures. Much of the work I did was rearchitect so I could use private. Ah well.

New version up:

 https://gist.github.com/792b7aa5b695d1092520

• Attempts to make PHP 4.x compatible by removing static and private functions and vars (though I can't test against v4.0)
• Added hookname property to both section and item
• Renamed property with list of items to be "items" instead of "submenus"

BTW, it would be really nice if we could figure out a way to transition to using named identifiers instead of file/slug for identifiers. For example, this is not a really great identifier to have to deal with:

edit-tags.php?taxonomy=movie-genre&post_type=movie

One issue (of many) if the ambiguity regarding the need to HTML encode/decode the ampersands as well the general length and complexity of this. It would be soooo much better to be able to simply call this the`"movie-genre-tags"' menu item or similar.

Ah, another "wonderful" thing about PHP4: objects are not passed by reference, but are cloned. That's why we have add_action_ref_array().

I think we should either:

• add a big warning in the docs, signaling that this doesn't work properly in PHP4
• punt to WP 3.2 and only commit #14666 in 3.1

Also, I used the terms "Menu Section" and "Menu Item" because AFAICT there hasn't been consistent naming and the names that have been used are unclear (at least to me.) If there are terms that others would prefer, I can change.

We do have a naming convention:

menu: a top-level item
submenu: an item inside a menu

Also, we generally use 'delete' when it's related to user data, like posts and comments. We use 'remove' for everything else: remove_action(), remove_theme_support() etc.

So, delete_admin_menu_section() would become remove_admin_menu() and delete_admin_menu_item() would be remove_admin_submenu().

Replying to scribu:

Ah, another "wonderful" thing about PHP4: objects are not passed by reference, but are cloned. That's why we have add_action_ref_array().

Well that's a bit of a bummer. I'm starting to understand why people have been ranting about the switch for so long now.

If that's the case I say we add the privates back in and even the static method unless you see the latter as an issue?

I think we should either:

• add a big warning in the docs, signaling that this doesn't work properly in PHP4
• punt to WP 3.2 and only commit #14666 in 3.1

I'm good for either. What does the core team think about adding PHP5-specific code right now?

Also, we generally use 'delete' when it's related to user data, like posts and comments. We use 'remove' for everything else: remove_action(), remove_theme_support() etc.

Sounds good. I'll make those changes.

Also, I used the terms "Menu Section" and "Menu Item" because AFAICT there hasn't been consistent naming and the names that have been used are unclear (at least to me.) If there are terms that others would prefer, I can change.

We do have a naming convention:

menu: a top-level item
submenu: an item inside a menu

Ugh. Those terms were some of what caused me difficulty when I first tried to understand the menus, and hence why I used different terms. The "submenu" term is particularly problematic for me because AFAICT they are options, not (sub)menus. And even "menu" is not so good because isn't the entire thing a menu?

Proposal: Evolve these terms and make a break from the past? After all, we are talking about deprecating the old architecture, why not deprecate the terms too? Here are some potentials:

• menu section, menu item
• menu section, menu option

And there might be others?

I'll wait to make the "delete_*" -> "remove_*" changes until we decide on this.

Also, can I lobby for deprecating the external use of the existing functions and moving to these or something else that is $args based rather than 6 or 7 parameters? They include:

• add_menu_page()
• add_object_page()
• add_utility_page()
• add_submenu_page()
• add_management_page()
• add_options_page()
• add_theme_page()
• add_plugins_page()
• add_users_page()
• add_dashboard_page()
• add_posts_page()
• add_media_page()
• add_links_page()
• add_pages_page()
• add_comments_page()
• add_plugins_page()

The new functions could call these with an "undocumented" additional parameter that, if not passed, would trigger a deprecated warning when WP_DEBUG is set. Doing this could really streamline this otherwise very obtuse API. Thoughts?

-Mike

Replying to mikeschinkel:

If that's the case I say we add the privates back in and even the static method unless you see the latter as an issue?

No can do, since that would cause syntax errors. So, I think we should go with #14666 for now.

Replying to scribu:

No can do, since that would cause syntax errors. So, I think we should go with #14666 for now.

As an alternate, or a stop gap?

I don't know. We'll discuss it today at the dev meeting.

Replying to scribu:

I don't know. We'll discuss it today at the dev meeting.

I'm really motivated to get some of this included in core (even if that means v3.2.) I just want to feel like I've contributed real tangible value to core.

Just tested Mike's latest github version on a site I'm currently building. So great that now I can't live without it. Implemented. Hugely liberating to an obsessive compulsive like myself to quickly be able to lose the unused fluff and reorganize in such a way that best benefits my clients. It would be great to see this implemented in 3.1 with an optional fail for PHP4. +1

Made the change of function names from delete_*() to remove_*() as per @scribu:

 https://gist.github.com/792b7aa5b695d1092520

I just from Mike's class and I will be using it on every WordPress CMS install from here on out. I hope it's implemented into the core as soon as possible because the admin menu is extremely important for new WP users and the current set-up is very difficult to customize.

I think "menu section" and "menu item" should be used to describe $menu & $subMenu as it better describes what is shown to users in the admin area.

john

I'm happy that you find using github and gists good for collaboration but could you put snapshot patches on tickets in future so it is easy to follow the development in one place :-)

Replying to westi:

I'm happy that you find using github and gists good for collaboration but could you put snapshot patches on tickets in future so it is easy to follow the development in one place :-)

I'm still trying to get to the point where I understand how to efficiently create patches. Will move to that when as soon as I can.

Sorry, wrong ticket. Closing, as per post-dev-meeting IRC discussion today.

(In [15753]) Introduce remove_menu_page() and remove_submenu_page(). Fixes #12718

Why not dogfood the existing nav_menu cpt and make a new, internal one for the admin menus? Full and complete manipulation if you want it, and completely hidden from view if you don't.

[15753] was supposed to reference #14666. Sorry, again.

Replying to johnjamesjacoby:

Why not dogfood the existing nav_menu cpt and make a new, internal one for the admin menus? Full and complete manipulation if you want it, and completely hidden from view if you don't.

After working with it a lot, I don't think the current nav_menu system is anywhere near mature enough to extend to another context. It should be given time to mature before tackling.

Replying to scribu:

[15753] was supposed to reference #14666. Sorry, again.

One thing my code implicitly addressed that we didn't cover in IRC (my bad) was moving to an$args based solution for what is currently add_menu_page() and add_submenu_page().

The fact that there 6 or 7 parameters that are hard to remember and must be called in the correct order makes the current API less approachable than if it were to use args instead.

Can we discuss that on this ticket before closure?

The original goal of this ticket was to not have to bother with separators.

So, I think a new ticket should be opened for passing an array of arguments, similar to #13937.

( related: #11517, #16342)

+1