We're currently hard-coding the menu path of a module's configuration page to a specific, often subjective, parent category page.

The problem is best described by webchick in #34:

Currently, your only option is to hook_menu_alter() the path from admin/config/development/XXX to admin/config/butterfly/XXX. But when you do that, suddenly all of your help text/drupal_set_message() links are broken, the tests (which are all pointing to the default path) no longer pass, etc.

Original post:

I'm sorry. I predicted this fact already, but no one wanted to listen.

I'm trying to port http://drupal.org/project/compact_forms to D7 as part of my personal D7 porting effort.

But.

There is no suitable category.

A suitable category would may be "User interface".

But that does not exist.

My little module has, as of now, 609 users.

It doesn't make sense for a module like that to define a new "category".

The "expected" result is to have 4,000+ Drupal modules to define their own "categories".

Core is not able to pre-define the categories.

We already know that. And we really should have learned from admin/build/modules.

I mentioned exactly that before.

The thinking is 2D.

The reality is 3D.

Files: 
CommentFileSizeAuthor
#41 menu_group_ouch.patch3.53 KBchx
Passed on all environments.
[ View ]
#39 menu_group_ouch.patch3.53 KBchx
Passed on all environments.
[ View ]
#27 hide-empty-summary-pages-591682-26.patch6.32 KBDavid_Rothstein
Passed on all environments.
[ View ]
#27 example-category-empty.png142.74 KBDavid_Rothstein
#27 example-category-not-empty.png151.56 KBDavid_Rothstein
#18 admin-page-empty-categories.png49.56 KBDavid_Rothstein

Comments

I'd put it in 'appearance'. There's nothing else interesting in there at the moment.

Are you kidding me? :P

User interface sounds good to me, I agree with your understanding that we did not account for 4000+ modules that need their own categories. We tried to account for 50/60% of them, and the other 40% should be handled by carefully choosing the first generation of categories that will pop up. I know its not perfect, or far from ideal even really - but coming up with an IA without knowing the contents will always be a mess.

More seriously, since this potentially applies to any form on the site, I'd consider putting it in 'system'. Although it's javascript/user-interface it's also a plumbing module, not completely removed from trigger which is already in there.

Priority:Critical» Normal

I second the new 'user interface' category.

Priority:Normal» Critical

Fine. So we create a new "User interface" category.

In the meantime, http://drupal.org/project/compact_forms lives in "Content authoring". You can expect that many contributed modules will similarly just use "anything".

Or they just create a new, arbitrary category, if they feel brave. Like http://drupal.org/project/admin_menu does.

But it goes on.

http://drupal.org/project/render is neither "User interface", nor "Media", nor "Content authoring", nor any other category. That one would indeed probably fit best below "Appearance", but the IA below "Appearance" is not at all prepared for being extended with many advanced configuration pages by contributed modules.

Hence, again, I'll stuff that module "anywhere".

Just not where the user will expect it.

category 'module administration'?

Issue tags:+D7 API clean-up

Tagging absolutely critical clean-ups for D7. Do not touch this tag. Please either help with this or one of the other patches having this tag.

ok. What I intended, but due to the limited time-frame did not manage to do here is:

  1. Introduce hook_module_category_info(). By default, only system_module_category_info() exists.
  2. system_module_category_info() defines a bloat of around 20 to 30 categories - each with 'title' and 'description', as in:
    <?php
    function system_module_category_info() {
     
    $category['system'] = array(
       
    'title' => 'System',
       
    'description' => 'blah blah blah',
      );
     
    $category['media'] = array(
       
    'title' => 'Media',
       
    'description' => 'bleh bleh bleh',
      );
     
    $category['administration'] = array(
       
    'title' => 'Administration',
       
    'description' => 'bluh bluh bloh',
      );
      return
    $category;
    }
    ?>
  3. As long as no contrib module extends these categories (and there may be only one that may do on behalf of many others, working as a global depedency), only those catgories exist. But at least, it's 20-30 instead of poor 8.
  4. This information is used in two ways: 1) For the modules page. 2) For the menu system.
  5. Modules page is too easy to waste a word on.
  6. Menu system is the part where it gets tricky: Instead of implementing hook_menu() for those administrative items, modules would implement hook_system_menu() (for above example implementation) and the registered menu items would NOT contain the path prefix "admin/config/$category/".
  7. Instead, system_menu() basically invokes module_implements('system_menu'), and for each module, retrieves the internal (machine-readable) category name defined in the $module.info file, consults the category registry, and registers the returned menu router items on behalf of the originally implementing module - and is thereby able to prefix and inject the proper $category into "admin/config/$category/".
  8. This ensures that there is at least a sane amount of usable default categories.
  9. This ensures that module categorization makes any sense after all. It's horribly confusing that you find all core modules in a "Core" category on the modules page, but not in the management menu.
  10. This ensures that modules do not simply create their own categories, nor put themselves into totally different categories.
  11. This ensures that categories are still pluggable, i.e. extensible by contributed modules (though preferable one central contributed module that takes over this job for all modules that do not suit into any of the default categories).

Either that, or we revert this categorized "configuration & modules" part of the new IA.

Anyone who does not see or understand the problem should stop thinking 2D. Drupal is 3D and the IA has to account for that.

I came up with a similar approach when thinking about #508458: Config and modules page but at that time it was so close to freeze I gave up trying to take things in that direction, not sure it ever got written down anywhere but encouraging that you mention a similar thing now. Apparently we used to have a hook_settings() which did something similar.

Having this do both the modules page and admin/config sounds good - potentially it could handle /structure and friends too. It's a massive API change though, so we'll need to get Dries/webchick's nod on a very late exception. Note that 508458 is still open because I wasn't at all happy with with the current way we do things, but no-one replied to that issue for the six weeks after I re-opened it.

Sorry, but I believe IA fixes can go in UX polish phase so I see no reason to revert. But instead only work harder on this.

I agree that we should probally ship with more default categories, but accusing us of thinking in 2D to me sounds rather stupid. Because we where fully aware, that this IA would cause problems if we didn't have enough programming efforts behind this.

It seems what you are proposing is rather complex, could you explain some more?

So the admin/build/modules page doesn't have any pre-defined categories at all, it's just a free-for-all for contrib modules to put themselves in - you can easily end up with user interface, interface, interface enhancements categories if you manage to install three modules which make those slightly differently named categories.

The admin/config page does have some predefined categories, but only ones which directly apply to core modules.

The idea, is to register categories appropriate for both core and contrib in system module, and use these for both pages - those categories would be intended to cover 90% or more of contrib modules, not only those which fit into the ones core modules do.

Modules would be able to add their own category if they absolutely had to, but it'd be much more discouraged.

To implement this, means that instead of defining hook_menu() for their own admin paths, they'd define them in a hook_system_menu, with a 'category' key. system module would then put those in the proper place in the menu system on their behalf.

That means, for example, if we renamed 'config' to 'settings' in Drupal 8, it'd be a few lines change in system.module and updating tests, but no API change for the vast majority of contrib modules.

I was asked to chime in here.

Rolling back the IA is not happening.
Changing the way that admin categories are defined is not happening in D7, because we're past code freeze now.

Seems the sensible way to solve this is to create a "meta issue" for contrib module authors who have no idea where to put their stuff, and we work together to come up with some sensible place-holder categories in core to help guide them along.

I shouldn't have mixed co-beneficial changes to admin/config/modules into this issue, because it seems there's quite some confusion here now. I only mentioned admin/config/modules, because we have the identical, multi-dimensional categorization problem on that page since Drupal 5 - but that wasn't so much of a problem, because you only need to work on that page infrequently.

This issue is about the menu system, resp. the new IA below admin/config.

Let me try to explain the current situation and major problem once again:

  • Drupal core provides certain categories below "Configuration and modules".
  • These categories are only available when the implementing module is installed and enabled.
  • Without the implementing module, a category does not exist.
  • Those categories are meta categories. Which means that a large amount of modules should use an arbitrary, but unique and consistent category.
  • Modules that try to re-use a category provided by a Drupal core module need to ensure that the Drupal core module is enabled. Otherwise, the category does not exist.
  • Since these are still meta categories, there is no reason and there should be no requirement for a module to require a core module just to get a predefined category.
  • A module that needs to use a category not provided by any Drupal core module needs to register its own category.
  • Any other module that may intend to use the same category like that previous module needs to register the category on its own, because these are meta categories and any connection or dependency between those modules cannot be presumed.
  • Each module author will therefore have to register a suitable category. Based on my very own D7 porting statistics thus far, less than 25% of all modules fit into the existing categories of Drupal core.
  • Several modules will register their own category, and a Drupal site can easily have 50-150 modules.
  • The result will be an always visible and unusable categorization below admin/config:
    admin/config/administration => Administration
    admin/config/admin => Administration tools
    admin/config/management => Management
    admin/config/user-interface => User Interface
    admin/config/user_interface => UI tools
    admin/config/interface => Interface elements
    admin/config/ui => User interface widgets
    admin/config/e-commerce => E-Commerce
    admin/config/commerce => Shop
    admin/config/ecommerce => Payment tools
    admin/config/cart => Shop integration
    admin/config/ec => E-Commerce
    ...
  • There is no way to defer the responsibility of using proper categories to contrib. If that would work, then we would not have the same categorization problem on admin/structure/modules. There is simply no way to achieve an agreement among 100+ module maintainers to use a consistent category. (And I know what I'm talking about here, because I tried to achieve it once, and that simple change took 1.5 years - and while that issue is marked as fixed, the change is still not deployed everywhere, because a variety of modules still has that change in the development snapshot only.)
  • These menu paths and categories cannot be altered in any way, because all module logic depends on those paths. (form submissions, links, help, conditionally executed logic based on the current path, etc.)
  • Which ultimately means: Whatever decision any module maintainer in contrib makes about the module's category - you have to live and work with that category - regardless of whether there are 5 other categories that intended to introduce the same meta category already, and regardless of whether each category contains one item only.
  • Every single Drupal site having contributed modules installed therefore has a 100% chance to get a weird, confusing, and unusable IA below admin/config like in the example above.

Why a simple approach does not work out:

  • If system_menu() would simply register additional categories, those would be unconditionally displayed by the menu system.
  • There is no way to conditionally and reliably hide parent menu items in the menu system, because the menu system no longer provides a concept of MENU_DYNAMIC_ITEM nor any other special property, which indicates that a menu item should not be exposed when there are no child items.
  • (Re-)introducing a concept like MENU_DYNAMIC_ITEM is a really major challenge, which I highly doubt of being possible anytime soon. That is, because any possible child items also need to be checked for access to determine whether the parent item can be displayed or not. And due to how the menu system works, this check does not end at the immediate/direct children of the parent item - all possibly available items below the parent item need to be taken into account.
  • Without built-in logic and support in the menu system, any other hack will break all contributed modules that render a menu.
  • The benchmark is not the admin/config page, but instead the "Management" menu block, which should not display any empty categories without any modifications to the building and rendering of the menu block in menu.module or system.module.

The proposed solution:

  • Works around the critical bug and misconception of not accounting for the multi-dimensional categorization problem, which was totally known, but simply ignored when the new IA below admin/config was introduced.
  • Works around the limitation of the menu system of not being able to hide certain menu items without accessible children - by not registering them in the first place.
  • Does not break any modules that are already ported to D7, because the categories are plain additional, and hook_menu() is not removed at all. Module maintainers are simply free to change their hook_menu() implementations into hook_system_menu() to leverage one of the new, optional categories defined by Drupal core.
  • Allows the community to create a dedicated, contributed module during the D7 release cycle that can act as single dependency for all kind of modules that belong to the 10% or more modules, which do not fit into any of the additional categories defined by core.

Because all of this, I only see two options to resolve this monster #fail:

  1. Implement the solution as explained above.
  2. Remove the categorization from admin/config/* menu paths.

Note that 2) would be the proper solution from a conceptual and API design perspective, because 1) mainly is a workaround "fix" for an introduced change, which just wanted something - without taking the horrible consequences into account at all. Actually, I'd say that the consequences entirely defeat the intended purpose of the categorization. Instead of a better UX, the actual, resulting categorization makes the UX far worse. The menu router item path in menu system is simply the wrong place to implement categorization.
Someone in IRC raised the question why those categories aren't rather working like Taxonomy, i.e. using user-defined and user-alterable tags instead of hard-coding the crap into the menu system. Not sure whether I'd agree with that proposal, but I definitely agree with the fact that we hard-coded something insanely irrelevant into the menu system.

Thanks for the additional details. I'm slowly warming up to this concept (still need to discuss such a radical API change at this point in the cycle with Dries), but I am curious to know how this actually solves the problem, as long as people can still define any menu path they want to under admin/* through hook_menu()?

Short answer: It does not solve the problem.

Long version: It would only be a workaround for a very challenging problem that was simply ignored when people asked for a UI gimmick, which lead to an API change that is technically wrong. The proposed workaround in 1) attempts to solve the problem (but not the cause) by making Drupal core take on the lead of providing a rather large list of predefined categories, which may be consumed by contributed modules.

It is a workaround, because it only works for 90% (or less) of all meta categories that may be required by contributed modules. In general, this fact sheds a very negative light on the architectural implementation, because it is something un-core-ish that would have worked for Drupal 3, but not for Drupal 7. In fact, the entire API change of introducing categories into menu router paths now requires us to architecturally go back to an approach that is similar to hook_settings() we had in Drupal 4 -- except that hook_settings() didn't have the problem of meta-categories, which is the ultimate root and cause of the problem.

To summarize the options once again:

  1. We try to work around a major flaw in the current API by trying to provide as many meta categories as possible to decrease (but not eliminate) the possibility of contributed modules that register new meta categories, which are already registered by another module (but differently). This does not solve the real problem, but provides guidance and a solution for the majority of modules.
  2. We remove the hard-coded categorization from menu router paths, and if absolutely required for D7, we re-think the purpose and goal of the categorization, by either applying it to generated menu links (optionally investigating a tag-alike approach that allows the user to reshape insane/weird/duplicate/confusing/unusable categories), or by applying it to the output of the admin/config page only (which seems to be the primary purpose anyway).

We knew that the IA was not done, so I guess this constitutes as the change we need to make. Although my response might be short, I would like to empower this idea - in regards to importance. If we are unable to create a few more very sensible defaults for contrib modules to live in, the whole IA could fall down when we get into the real world of 30 modules enabled. Can we please get some more reviews on this issue though, so we are not making something only tha_sun can understand.

StatusFileSize
new49.56 KB

I had to read this issue a few times before I fully understood the magnitude of the problem, so to help summarize, I've attached a screenshot which helps illustrate the key point, which is this:

If system_menu() would simply register additional categories, those would be unconditionally displayed by the menu system.

As can be seen from the screenshot, this is true: Empty admin categories are automatically displayed right now, and look really bad.

If we could get around that issue, then this problem would be easier to solve (would not require major API changes) because we could just define extra admin/config/somecategory, admin/config/somecategory2, etc the normal way in Drupal core - which core would not use but which would be standard categories (documented somewhere) that contrib modules are all supposed to put their stuff in. However, if we did that now, all of those extra categories would show up on the screen in every single Drupal installation.

***

As for possible solutions that require minimal API changes, I am trying to decide if the following would work:

  1. Notice that in the {menu_links} table, we currently have a 'has_children' column, which as far as I can tell, already stores information about whether a category is empty or not?
  2. So can't we just define a new menu constant (or possibly reuse an existing one) that means "I am a summary page, so if I don't have any children, don't display me in the menu tree"?
  3. Then, core could add several empty categories the normal way in system_menu(), which would not be displayed unless contrib modules fill them:
    <?php
    $items
    ['admin/config/user-interface'] = array(
     
    'title' => 'User Interface',
     
    'description' => 'This is here for contrib modules which need it.',
     
    'type' => MENU_SUMMARY_PAGE,
      ...
    );
    ?>
  4. Because it uses information in the {menu_links} table, this also seems like it would work correctly even if the site admin has rearranged stuff via menu.module, whereas I'm not totally sure how that would work with the proposal in #9.

Does this make sense, or am I missing something totally?

David: That sounds like that'll work for the empty case, but doesn't work for the "Menu has children, but currently logged in user can't access them so for all intents and purposes, it's empty." For that matter, I'm not sure if sun's proposal addresses this either.

We had this issue fixed already in HEAD, but had to roll it back for performance issues. See http://drupal.org/node/296693#comment-1904074.

One one hand, that "Menu has children, but currently logged in user can't access them so for all intents and purposes, it's empty." case isn't addressed in D5/D6 either. But on the other, we're dealing with far fewer possible categories in those older versions.

@David: Thanks for reviewing and providing the trigger!

@webchick: Awesome! Thanks for pulling the trigger! :)

For that matter, I'm not sure if sun's proposal addresses this either.

Bam. Thanks, at this point we can forget about the entire workaround proposal.

that [...] case isn't addressed in D5/D6 either.

Tiny correction: MENU_DYNAMIC_ITEM existed in D5, which had an entirely different menu system, and therefore, only D6 does not support that kind of dynamic item that is only to be displayed if there's something.

Ultimately, this means:

  1. Do the impossible and fix #296693: Hide empty admin categories, and afterwards, either implement the proposal outlined here, or implement an API change to re-introduce a new 'type' => MENU_DYNAMIC_ITEM, which allows certain menu links derived from menu router items to disappear in case there are no accessible children. (even typing that sounds veeeery unlikely to be feasible soon)
  2. Accept the regression and revert the categorization below admin/config. Afterwards, either accept that it's too late and it should have been taken into account in the first place, or do a major exception that allows us to work on a different/new categorization feature (and optionally menu system integration) for those items.

Hm, that is true - I'm pretty sure neither proposal addresses that.

On the other hand, I'm guessing it's not that hard to fix in the particular case of the admin/config page, though? It doesn't seem like there can be a performance issue there, because on that page we have to render the whole thing anyway - so we'll know pretty easily if it's empty or not. So it seems like we can at least make it so that admin/config does not show "empty-for-me-only" categories to those users?

Trying to fix it in the general case (anywhere where the menu tree is rendered for that particular user) - that does sound trickier... but also probably less important?

Unfortunately, fixing it for admin/config is only half of the battle. The "Management" menu is the other half.

admin/config already only shows categories with viable links in - that's handled in theme_system_admin_menu_block() and has no serious performance impact - and anyway, if it did, it would only happen when actually viewing that page. The problem is checking expensive access callbacks on the entire admin tree on every page view.

The main reason we have this problem now, is because the toolbar was committed without even cursory benchmarks or performance testing, so we have been firefighting for the two or so months since then. #296693: Hide empty admin categories would not be a serious problem in Drupal 6, because the management menu defaults to the 'admin' item being collapsed - and that item does not use system_admin_menu_block_access(). It would be a problem for admin_menu.module, except for the fact that module implements client-side caching per-user, so doesn't take the hit on every page of building up the menu structure. However the combination of it + the toolbar is a nightmare.

I fully agree this is a critical issue which needs to be solved before release. It should not be solved by re-reverting system_admin_menu_block_access(). Since modules are already porting, a new API vs. reverting to an uncategorized IA is probably equal work.

I think it would also be worth just seeing what an uncategorized admin/config would look like - making use of two column layout etc. rather than the very long thin block which /settings was would still make it a friendlier page. I already get confused where blocks like 'People' are now - and the space they take up means I'm often scrolling the admin panels. A more compact, alphabetical listing, making use of the full available space might not be bad at all. That wouldn't really revert the IA, since the categories were never properly specified in the original wireframes or anything, and we're feeling the effects of that lack of detail now. The main purpose of the IA changes was to move /structure and other paths out of the toolbar, which even though I know sun strongly disagrees with it, he's not arguing to revert here.

What about that. We remove the categories below admin/config from the routher path (so below admin/config we immediately have the individual admin pages). We add two new properties for router items, "categories" and "category".

"categories" is an associative array, like that (all code a little pseudo, no codebase at hand right now):

<?php
function system_menu() {
$items['admin/config'] = array(
  ...
 
'categories' => array(
   
'media' => array(
     
'title' => t('Media'),
      
'weight' => -10,
     ),
   
'services' => array(
     
'title' => t('Web services)',
     
'description' => t('Tools related to web services)',
    ),
   
'default' => array(
     
'title' => t('Other'),
    ),
  ),
);
?>

"category" can then be defined for any child item of admin/config, like that:

<?php
  $items
['admin/config/system-file-system'] = array(
   
'title' => 'File system',
    ...
   
'category' => 'media',
  );
 
$items['admin/config/system-rss-publishing'] = array(
   
'title' => 'RSS publishing',
    ...
   
'category' => 'services',
  );
?>

Both of these properties have to be stored with the router item, of course. Now, whenever a router item defines a "categories" property, the menu tree theme function has to react and sort the loaded children in the tree into the appropriate categories (in a block it could just be presented like a normal tree hierarchy, the categories themselves would just not be links).

The list of available categories can easily be extended via hook_menu_alter(). All category properties are easily alterable. Child items that haven't set the "category" property are sorted into the "default" category. Sorting the items into the categories happens at the theme level, so as all actual config pages (that is items in the admin/config categories) are direct children of admin/config, the access problem should be solved, I think. And it's more of an API addition than an API change.

  1. We need to accept that there is no way around fixing #296693: Hide empty admin categories for a multitude of reasons, not limited to this issue.

    I also want to amend that admin_menu is not the only "administrative menu rendering module" in contrib. There is also SimpleMenu, DHTML menu, and all the other related modules that are listed on Administration menu's project page. That is why the issue needs to be properly fixed within the menu system and must not be special-cased for an arbitrary function in system.module.

  2. To prevent any misunderstandings in advance: I am, and it seems also the UX team is, highly against this
        'default' => array(
          'title' => t('Other'),
        ),

    or
        'system' => array(
          'title' => t('System'),
        ),

    (where "System" is a "meta" category that holds items like "Shortcuts" - WTF?)

    The entire purpose of introducing categories in the first place was to provide a meaningful guide to the functionality you may search. If the functionality is just stuffed anywhere, then the whole categorization #fails.

    Likewise, this issue is about preventing contributed modules from registering the same meta category multiple times. Hence:

    How ever the parent link container (MENU_DYNAMIC_ITEM) issue is going to be solved, the three-dimensional problem of registering meta categories will remain -- unless all meta categories are basically just tags, which the site administrator can quickly alter via the UI, which is unlikely to happen for D7.

  3. As mentioned before, I'm favor of catch's and frando's direction of removing the categorization from menu router paths, because it technically is the wrong API to implement categorization. If we would want that, then we would have node/category/% instead of node/%, which would be equally wrong, because these are menu router paths.

    (To clarify: Drupal works very well without menu links, but not without menu router paths.)

Subscribe

Status:Active» Needs work
StatusFileSize
new151.56 KB
new142.74 KB
new6.32 KB
Passed on all environments.
[ View ]

OK, I am starting to get very confused and am a bit convinced that we are now discussing two or three totally separate bugs all in the same issue... so I decided to just write some code :)

The attached patch implements what I described:

  1. In the patch:
    • Core defines an "admin/config/example" category but does not populate it with any children.
    • Because it has no children, this category does not display either in the Management menu or on the screen at admin/config (see first screenshot).
    • If you use the Menu module to drag something into it so that it is no longer empty, it then automatically appears in both places (second screenshot).
    • Similarly, if you were to install a contrib module that provided "admin/config/example/mymodule", the category would also automatically appear.

    Therefore, this solves the basic problem raised in the issue. All we now need is a list of X number of categories along the lines of "admin/config/example" - which we can define using the standard hook_menu() method.

    The patch also needs some tweaking because there are about 40 different ways that menu trees get rendered in Drupal and I'm not sure I got all of them :) But as the screenshots show, the important ones are taken care of.

  2. This patch does not fix the problem at #296693: Hide empty admin categories, but neither does it make it any worse than it already is. That is a totally independent bug. While theoretically, one could imagine a solution to that problem that also automatically fixes this one, it sounds like we probably can't count on that due to the performance issues.

    Ripping apart the new Drupal 7 IA, however, will do absolutely nothing to mitigate that problem either. Taking the toolbar module out of core (or totally rewriting it) might, however :)

  3. @catch wrote:

    admin/config already only shows categories with viable links in - that's handled in theme_system_admin_menu_block()

    Maybe it did that at one point, but it definitely doesn't do it now -- if it did, I wouldn't have had to put a workaround for it in the attached patch :)

The problem of that approach is:

<?php
function menu_tree_page_data($menu_name, $max_depth = NULL) {
...
       
// Build an ordered array of links using the query result object.
       
$links = array();
        foreach (
$query->execute() as $item) {
         
$links[] = $item;
        }
...
     
// Check access for the current user to each item in the tree.
     
menu_tree_check_access($data['tree'], $data['node_links']);
}
?>

i.e. a category that is empty, because the current user does not have access to the items in it, is still displayed.

To summarize once again and hopefully make it clear:

  1. #296693: Hide empty admin categories is required to reliably and correctly hide empty category/grouping items. When that has been fixed, we could simply register many more categories in system_menu() that will use the re-introduced menu router item type MENU_DYNAMIC_ITEM, so the menu tree building functions have a target to apply the special behavior to.
  2. If we believe that #296693 can and will be fixed soon, then this issue could be considered as fixed after introducing those further meta categories, which hopefully will solve the underlying problem of multi-dimensional registered categories for perhaps 90% of all modules.
    However, that approach will still be a workaround, because we already know that it won't work for all modules (100%).

    Hence, the decision that has to be made for this point is whether we trust that 1) can and will be fixed soon and we can live with a half-baked architecture -- ignoring that menu router paths are the totally wrong location to implement categorization.

Yeah, like I said above, I make absolutely no claim that my patch fixes the problem at #296693: Hide empty admin categories -- it does not :)

However, I guess what I still don't understand is why these issues are necessarily coupled: As far as I understand, the bug at #296693: Hide empty admin categories has existed since Drupal 6.0. Why is it suddenly a release blocker now?

If someone is, for example, using admin_menu in D7, hovers over "Configuration & Modules", gets a dropdown menu with 8 or 9 items in it, but 3 of those items are empty because they, personally, do not have access to anything in it - is that really the end of the world? It's definitely not great, but people do understand the concept of empty folders, I think.

Whereas the problem you have identified in this issue is definitely new to Drupal 7 (given the way we are asking modules to conform to a new IA) and seems much, much more serious.

David, you're right, seems no-one bothered to test this when rolling #551080: List non-container items on /admin for a complete overview, which I've now re-opened.

It's sounds like there are a couple small bugs (with separate issues) to be fixed - but otherwise I'm not sure we should make further changes to the core API

Title:Categories below "Configuration and modules" make no senseCategories below "Configuration and modules" - need more default categories

Apart from changing the menu system and more API related issues, I would like to pull a more generic title - as we try to do what is proposed earlier. If this is better served as a new issue please notify me.

Title:Categories below "Configuration and modules" - need more default categoriesCategories below "Configuration and modules"

#627080: [meta-issue] Additional categories admin/config For the actual IA work, so the implementation can continue here.

We had a lengthy chat about this in #drupal today.

Sun's objections boil down to the fact that we encode categorical data in the menu path. Menu categorization is highly subjective; while core will go one way in order to try and hit the 80% mark, something like Open Atrium or Buzzr or Gardens may want to go a totally different way. Currently, your only option is to hook_menu_alter() the path from admin/config/development/XXX to admin/config/butterfly/XXX. But when you do that, suddenly all of your help text/drupal_set_message() links are broken, the tests (which are all pointing to the default path) no longer pass, etc. It also means that unless core defines an appropriate top-level menu path, contrib has to do so itself, and you might end up with 30 contributed modules all defining variations of admin/config/user-interface admin/config/interface admin/config/ui... Just like our lovely module "package" key. :P

For these reasons, I can get behind the idea of de-coupling the path itself from its categorization. The main reason we do this at all currently is because our default breadcrumb generation system is limited to only deriving breadcrumbs based on a hierarchical path. But we can do special tricks with drupal_set_breadcrumb() and system_admin_block() (or whatever it is) to help mitigate this.

However, we need to store that categorization somewhere. We can't go back to admin/config having 37,000 links in one gigantic list (esp. now that basically everything goes under config). Of the proposed options, the one that makes the most sense to me is simply adding a 'category' index to hook_menu(). I realize this is really ooky because it conflates page routing data with menu link display data, but it's not like this is anything new; we also have title, description, position, context, theme...

I know Peter is opposed to this, and I take his opinion as menu system maintainer very seriously. But I asked him if we were to split hook_menu() into hook_page_router() [for path, * callback and * access] and hook_menu_link() (for everything else) in Drupal 8 whether or not we could store a category association in hook_menu_link() and I believe his answer was yes (I'll let him speak for himself). It certainly makes sense to me though; the idea would be that hook_menu_link() would contain any presentational logic related to the link display itself, and from that respect fits well with the other items in that list (well, except theme which is just a bit weird :P).

So I caution against us defending too rigidly against a separation here that doesn't actually exist in reality. That said, I'm certainly open to debate on how else to store this category info, but of the presented options, this is my favourite.

Menu categorization is highly subjective; while core will go one way in order to try and hit the 80% mark, something like Open Atrium or Buzzr or Gardens may want to go a totally different way. Currently, your only option is to hook_menu_alter() the path from admin/config/development/XXX to admin/config/butterfly/XXX

Maybe I'm just not getting something, but:

1. If you want to use a different URL, you can add it as a URL alias.
2. If you want to create an "admin/config/butterfly" category, you can do that in hook_menu().
3. Finally, if you want to take an existing menu item and recategorize it into your new butterfly category, you can do this by dragging and dropping it via the menu.module UI..... (Granted, I have never tried doing the equivalent of that via writing code, but if Drupal lets you do it via drag and drop, surely it lets you write code for it as well :)

So there doesn't seem to be a new issue here that requires massive changes. The only problem we currently have is that if core wants to define a bunch of (empty) categories like admin/config/butterfly for contrib modules to use, currently they will be displayed even when they are empty, and we don't want that. The patch above solves part of that problem, and (at least the last time I checked) the patch at #296693: Hide empty admin categories seemed to solve the rest... both via similar methods which could probably be combined, and overall without any major API overhauls.

Is this a reasonable summary or am I missing something?

All of the mentioned "workaround" suggestions in #35 are fully in line with the problem: They all talk about menu links, NOT menu router paths.

To technically re-phrase them correctly:

1. If you want to make the same path available under a different URL, you could add a URL alias. (has nothing to do with this issue)
2. If you want to create an "admin/config/butterfly" category, you can create a new menu link. (so also breadcrumbs will work properly)
3. If you want to take an existing menu item and recategorize it into your new butterfly category, then you can move the link via menu.module's UI.

4. If a module wants to re-categorize itself to a more sane default, then it won't need to rewrite all redirects, help arguments, etc. and it won't break a ton of other contributed modules that depend on the paths by doing so.

I've tried to group modules 100 to 300 into some categories. Should at least give us some idea on which ones we want to add by default.

http://spreadsheets.google.com/pub?key=tWJHejIEuEGpeRk4a-SI_bQ&single=tr...

The current menu configuration is this: 4 categories that are important for one kind of websites (that the whole D7UX is designed for, totally missing the target http://www.disambiguity.com/designing-for-the-wrong-target-audience/) and then the kitchen sink. If this would not be UX it would be rolled back. It, however, will not.

But all is not lost. We CAN solve this issue. All it requires is a permission "access admin/foo" and then it becomes mandatory that this permission is not a blanket to see this page but actually is a permission for something. So if the menu system renders a category then it will know it's not empty. This requires exactly zero menu.inc change.

Status:Needs work» Needs review
StatusFileSize
new3.53 KB
Passed on all environments.
[ View ]

Thinking further I can write a hacky patch to help. I let you guys debate, comment and test it. It's not perfect because it relies on router item parent-child relationship which is nonexistent but as we do not create menu links from install but generate from hook_menu I see no other solutions. Half baked mess is the name of the game in D7 anyways.

Hm, adding a reference so it reads $group_items[$path] = &$item; helps a bit.

StatusFileSize
new3.53 KB
Passed on all environments.
[ View ]

Bah! The patch mnetioned in the prev comment.

Ps. to those who have a problem with my attitude: I might whine but I still help whatever ways I can.
Ps2. I totally should have filed this under #296693: Hide empty admin categories.

As far as I understand it, is this a fix for the issue that tha_sun raised where empty categories are shown. Which is lovely :)

Status:Needs review» Needs work

Right, this belongs into #296693-102: Hide empty admin categories, and I followed up over there.

so really should be marked duplicate?

No this one is about trying to avoid contrib creating duplicate categories because the core ones don't fit them. However if core makes extra empty categories, that other patch starts to become a prerequisite.

Status:Needs work» Closed (fixed)

Closing this #296693: Hide empty admin categories is where we should continue this, and what most discussion past the initial commit is about

Title:Categories below "Configuration and modules" Categories below "Configuration and modules" break APIs badly
Status:Closed (fixed)» Active

Not sure why this was closed. Not a single of the points explained in detail above is resolved. Even including the original post.

While I was only half serious about putting sun's particular example in 'appearance', I was at least half serious - see #536440: Reconsider whether "Appearance" should be a top-level item in the IA for another example where a module doesn't have anywhere to live and 'appearance' is the best fit, but currently useless.

Title:Categories below "Configuration and modules" break APIs badlyCategories below "Configuration and modules" do not account for all possibilities
Version:7.x-dev» 8.x-dev

A suitable category would may be "User interface".

But that does not exist.

Now it does, shortcuts is in there. #536440: Reconsider whether "Appearance" should be a top-level item in the IA has a patch to move themes there too. I'd be fine with renaming 'user interface' to 'appearance' so it's more encompassing, we can discuss on that issue.

Unless more concrete examples other than your module and skinr can be provided, this is not critical.

I'm not happy that my initial proof of concept patch ended up being the only implementation for handling the IA, but webchick's said it's too late to roll back the IA itself months ago, and the API change (which I'm somewhat fond of) of having modules suggest categories and system module either handle the paths, or present the categories without paths, is too late for D7 and has no associated code. So I'm moving this to D8.

Component:base system» menu system
Issue tags:-Needs usability review, -Usability, -D7 API clean-up+MenuSystemRevamp

Priority:Critical» Major

Category:bug» task
Issue tags:-Release blocker

I was about to reply in
#653784: Separate out menu links from hook_menu
but I think it is better placed here.

I went through a lot of these thoughts myself in the past, and I think I have some solutions, that should at least be considered.

@catch (#653784-21, that other issue)

I don't want to take this off-topic, but the way that we construct admin menus now is extremely problematic, #591682: Categories below "Configuration and modules" do not account for all possibilities

Do I understand correctly, this is about the "which item/link goes into which submenu", and about "who decides what".
Imo, this is mostly a design problem, and a responsibility problem, and not so much an architecture / API / technical problem.
A good API can help us to manage the design and responsibility tradeoffs, but it can not eliminate the design conflict.

For the design aspect, I think this was easier in D6. The two categories "Build" and "Configure" can feel quite arbitrary sometimes, but at least it was only two places to look, in most cases. And from there down it's alphabetic, mostly by module name.
(I am talking past, but in fact I still spend most of my time in D6 land)
Some modules that feel more important than others (og, ubercart) would then add their own categories - fine with that.

For D7 / D8, ok, we now have more categories. These might feel more logical, but it also means, more places to look, before you can go alphabetic by module name. Again, this was a design decision, and no API in the world can eliminate the design tradeoffs.

Now, whatever the ideal structure is, I do have some opinion about how to get there:

  • Yes, I think an admin_menu-style dropdown menu, if done right, is preferable to any toolbar or whatever. Both for the site builder, and for the client (*).
  • The module that introduces a particular administration page, should decide where to put it in the tree.
  • On two different Drupal sites, I want to find the same administration page in the same place in the menu.
  • It is generally a good idea if the path of an administration page reflects its position in the menu.
  • DRY. I don't want to have to repeat the same information in two different hooks.
  • The site admin / site builder should not have to waste any time re-shuffling the admin menu. It has to work out of the box. Messing with this manually is a time sink, and only confuses your team members.
  • Modules altering the admin menu should get semi-reliable string keys to deal with, instead of totally arbitrary numeric auto-increment mlids. Mostly these can be identical with a respective menu link path, but they don't have to.

I think the current hook_menu approach is not so bad with all of this.
If every admin page needs a position in the admin menu, reflected by its url path, then why not continue to do this in the one unified hook_menu that we are used to? And then provide specialized hooks to alter the structure of the admin menu.

For the technical aspect:
I think this is all solved quite ok (from my pov) in DQX AdminMenu.
(if you don't want to use it, see it as proof-of-concept)
That's all D6 yet, but I think the basic technical approach should work just fine for D7 / D8.
This admin menu lives totally outside of menu_links.

Sun's objections boil down to the fact that we encode categorical data in the menu path. Menu categorization is highly subjective; while core will go one way in order to try and hit the 80% mark, something like Open Atrium or Buzzr or Gardens may want to go a totally different way. Currently, your only option is to hook_menu_alter() the path from admin/config/development/XXX to admin/config/butterfly/XXX.

Now, this does bring a new aspect I have not considered before. So I have to extend the "requirements" I outlined above.

  • Link paths to admin pages should be reliable, the same throughout sites. Every site is the same.
  • The position of an admin page in the admin menu should be flexible. Every site is different.

I agree, this flexibility is a desirable thing to ask for.

How flexible can or do we want to get?
And what does this mean for the "responsibility" aspect?
If module X defines an admin page, then who can decide about the position of the menu item?

  1. Ask module X. This will give us the same information (either a menu position, or a few "hints" for categorization), no matter which site we are on, if this is the same module X installed on these sites.
  2. Ask another module Y, whose only reason of existence is to re-shuffle the admin menu. This module can be written specifically for this site. However, we can only expect this module to handle a limited number of other modules' paths. Probably it will still use the categorization info given by module X, just do a few structural modifications on the tree or on individual items.
  3. Ask the site builder (via click-click). No matter how we do it, this is always going to be quite ineffective imo. And again, the site builder will for the most part keep the default structure, and do only a few modifications to the tree or to individual items.

Most parts of the tree will remain in their default shape, because there would be just too many special-case decisions to be made otherwise.
However, a few top-level tree modifications can already make the entire thing look quite different, even though it is for the most part the same thing.
In these modified parts of the tree, the tree positions will no longer match 1:1 with the url paths. But this is something we accept.

Luckily, this is all still within the scope of what DQX AdminMenu can do with its API - if we port it to D7.
(better than with classic admin_menu, afaik)
Something like Buzzr or Gardens could implement one of those hooks, and define an alternative structure, without messing with urls.
All of this with semi-reliable string keys, no mlids.

For these reasons, I can get behind the idea of de-coupling the path itself from its categorization.

I don't think this would help us.
The main issue is still the responsibility issue: Module X can give us a path, or it can give us a category. In both cases this results in a default position in the menu, that will be the same on any site. Another module can then come along and change this position.
Category or path, does not make a difference.
(Unless we want to use the path to express "ownership", instead of reflecting the hierarchy. Such as, every path by module x would begin with admin/x/. Nope, I don't like this.)

The main reason we do this at all currently is because our default breadcrumb generation system is limited to only deriving breadcrumbs based on a hierarchical path. But we can do special tricks with drupal_set_breadcrumb() and system_admin_block() (or whatever it is) to help mitigate this.

Breadcrumbs can be dealt with.
There can be different "indicators" for what should be the breadcrumb parent of a given path. Url path, admin category, menu position, etc. We can pick from these indicators to generate a breadcrumb. (this is what crumbs does)

Priority:Major» Normal

I'm downgrading this from major, although still agree the current situation is not good, but neither is there anything actionable here.

Do we have any new data about how the new admin categories perform in usability tests?

Hm. The only testing I'm aware of found issues with the IA/toolbar at a higher level than this. See http://groups.drupal.org/node/227168 for a new proposed IA and https://www.acquia.com/blog/journey-build-better-drupal-toolbar for the testing data.

This probally does need testing, we did do some testing on overarching "where is stuff" - but not specifically where things in content/strucute/configuration live. This is not to say, we haven't touched these pages in testing - we just haven't done exploratory tasks around the actual categories.

Title:Categories below "Configuration and modules" do not account for all possibilitiesCategories below "Configuration" break APIs on rename/relocation and do not account for all possibilities
Priority:Normal» Major
Issue tags:-Needs usability testing+API change

Clarification: This issue was never about UX. This issue is about the code/API consequences of the introduced categories only.

#36 still stands. (initial/early comments really meant the same, but failed to express it in the right terminology)

I stopped counting the number of modules that either renamed or relocated their configuration pages. Because if they did, the router path changed, too, and all of the module code had to be adjusted to account for the revised category name. Something that should have been a 1 line patch turned into gigantic monsters of patches having to change the router paths all over the place.

And that does not even account for the factor X^N - the module's eco-system - meaning an entirely unknown number of dependent modules, which suddenly had to be updated for the configuration path change. Likewise, for each of those, not a 1 line patch, but instead, entire module rewrites.

This is not hypothetical or made up - plenty of contributed projects suffered from this already.

What we need to do for D8 is:

  1. Remove the category names from (the internal) router paths.

    admin/config/media/file-system becomes admin/config/file-system

  2. Enhance the menu system code that's responsible for auto-generating menu links from router paths to actively support a router item definition key that references another router path, under which the generated menu link should be placed.

    $item['admin/config/media'] = array(
      'title' => 'Media',
      'page callback' => 'system_admin_block_page',
    );
    $item['admin/config/file-system'] = array(
      'title' => 'File system',
      'page callback' => ...,
      'parent' => 'admin/config/media',
    );

Result:

1) Visually: identical to now.

2) Technically: restoring router path sanity.

Priority:Major» Normal
Issue tags:+Needs issue summary update

I don't see this as a blocking task.

Title:Categories below "Configuration" break APIs on rename/relocation and do not account for all possibilitiesConfig pages are effectively hardcoded to some subcategory of 'admin/config'
Issue tags:-Needs issue summary update

I see that we need to do something about this problem.
And technically, we're on the way to a decoupled router and menu_link implementation right now, so this should be doable.
So, first of all, updated the issue summary.

But if I get it right, I actually dislike the idea of decoupling path from menu links / breadcrumbs, as raised in #34 and 58.

$item['admin/config/file-system'] = array(
  'parent' => 'admin/config/media',
);

is neither intuitive nor visually identical to the existing. The natural parent of 'admin/config/file-system' is and should always be 'admin/config'.

Finally, in the absence of breadcrumbs, for many people the path has always been something like generic breadcrumbs, and it continues to be even if breadcrumbs are displayed. The path gives an immediate and usually reliable indication of a page's placement within the site. Might be less the case for the younger front-end generation, but still a broken correspondence seems irritating.

Why don't we simply bind help text, or any kinds of links (including drupal_set_message(), the config link in foo.info.yml etc.) and tests to router names instead of paths as target? That would seem a sane, generic and straight-forward solution while keeping the path consistent with breadcrumbs and all that.

Issue summary:View changes

Issue summary updated