admin/by-task is confusing since it lacks links to config pages and looks similar to admin/config and friends

No alphabetical sorting, it should be as it is now - but including the links to all of them.

Subscribe. Spent the last hour confused by this myself.

Hrm, I get a 404 at admin/index, am I forgetting something?

@cha0s look at /admin vs. admin/config

#698278: Fix the main admin ("by task") page so it actually makes sense is a couple days older and seems to have a more specific proposal for what to do about this situation.

Reversing what @David this, because this is the better issue to do it. Basicly the suggestion in that issue was :

To have /index link the admin/config pages correctly by for example doing :

* System
Site information | Actions | Shortcuts
* Media
File system | Image styles | Image toolkit

Although I think that might be best, we do have to take into account the height and width this might need - ie taking the whole right side of admin/index for admin/config links?

Excuse me for getting all meta on this thread, but...

Shouldn't this be considered a UI issue, and therefore frozen?

@Catch:

Thanks for the explanation, but I'm afraid I don't understand it. I think you're saying:

1. The page was added just before the UI freeze.
2. It was supposed to be taken out when the Dashboard was ready. (Ergo, it should be gone now.)
3. ??? something something related to D6 ??? (There was no Dashboard in D6, so what's "the D6 one"?)

Thanks,

--Tom

Well, it doesn't really matter what issue we do it in - but so far, this one has mostly consisted of people being confused as to what the issue is about. I was confused too, so trying to change the title to reflect the plan here :) I assume the idea is that part of this will involve changing the URL from admin/by-task to admin/index, which would make sense.

This is an important issue, because currently with the new IA, there is no single page you can go when you know what you want to administer something but don't know where to find it. #102254: Add an administrative search feature would help with that too, but is almost certainly not going to happen for Drupal 7.

The index its purpose is to server as the traditional by task page. This is what we intended out after long conversations with community members, however the plan as proposed here - would need to tackle one problem. How to properly display the items under admin/config.

Given that, a patch - that we can start trying out things would be most appreciated.

I'm not arguing with the reasoning behind the fix, and can see how it'll make Drupal better. I *am* irritated that I just lost dozens of hours of work because the UI changed substantially after the alleged "UI freeze". But I guess that's another matter.

Will it at least stay as /admin, and not become /dashboard or something like that?

As far as I am concerned it won't. But the tabs will still change, @tgeller I am sorry that you lost hours over this - its almost unavoidable not to change things that are not release worthy. In contrary to D5/D6 cycles, we have changed so much in the interface part of Drupal that some parts turn out to work worse.

Bojhan: Thanks for your sympathy. Clearly I took the "UI Freeze" too seriously.

You say "the tabs will still change". Which tabs do you mean?

Catch:

I'm not going to argue your post, because I agree with most of it. Clearly these changes are necessary and good for Drupal.

IMHO for D8, Dries et al. need to better define what "freeze" means. I waited through the string freeze and the UI freeze and made what I thought was a reasonable assumption based on, well, the usual meanings of those words. The other problem arose, I think, because I underestimated how much these late behind-the-scenes changes would necessarily affect the interface.

Anyway, this is a side issue. I'm sure you understand my frustration, and I certainly understand yours.

It would have been nice to time the book's release with D7's. Not just for me personally, but to demonstrate Drupal 7's support and readiness from Day 1. Maybe next time.

Thanks for all you're doing.

Subscribing.

@catch Could you whip up a patch? This issue seems to be stalled, purely on no active patch - the actual issue isn't that huge.

Tracking to come back to later.

I agree that what we have as 'by task' right now is a mess. 'By task' is kind of the underlying concept for the re-ordered IA as a whole anyway. I'd be fine with dropping it and making 'by module' the one full index.

Yhea sure, if you feel that's better.

Yeah, just using admin/by-module seems to make a lot of sense - that is already pretty close to an index anyway.

However, there are a number of (important) admin pages that currently do not show up there for whatever reason, because they are somehow missed by the crazy database query that is used to generate that page. I think we need to modify that query to pick them up, in order to make this page a true admin index.

Similarly, I wonder if the "show/hide descriptions" functionality should move to this page also? (One of the main ways I'd use this page is with Ctrl+F in Firefox to search for something on it, and the descriptions are very helpful for that.)

Finally, and perhaps for a separate issue, from the patch I see this:

-  // Check for status report errors.
-  if (system_status(TRUE) && user_access('administer site configuration')) {
-    drupal_set_message(t('One or more problems were detected with your Drupal installation. Check the <a href="@status">status report</a> for more information.', array('@status' => url('admin/reports/status'))), 'error');
-  }

If admin/by-task goes away and this message goes away with it... is there another page where it should be added? The Dashboard (if that module is enabled)? I see it's on the Configuration page now too, but that seems like a historical accident more than anything. A little tricky to figure out where to display this if there is no longer really a single central 'admin' page any longer.

If we get rid of admin/by-task, it's going to be really hard to find "appearance" related modules, as admin/config doesn't list them. For example, Skinr is purely an "appearance" related module and its settings belong there IMO, but if we put it there, it ends up being very hard to find.

It seems like a combo of the current admin/config & admin/by-task would be ideal, but maybe I'm missing some of the backstory?

@catch, well not exactly... admin/appearance is just the theme listing page. Only way to get it (Skinr) to actually "show" in that section was to make it a tab, which we did, but it feels very awkward and isn't exactly easy to find.

It would appear on by-module though.

Yeah, personally I think by-module is less than ideal, and would prefer if the appearance group was also listed on admin/config. It just seems like anything under /appearance is a second-class citizen and it's definitely awkward. But, maybe it was done that way on purpose we are not supposed to put anything under appearance?

I definitely don't want to hold this issue up because it needs to be fixed, and the more I think about it, having an appearance listing probably isn't something that should be discussed here.

Aha! I suspected this might have been the case, and agree with you about Appearance not deserving such valuable real estate on the toolbar, but in any case thank you for clarifying this.

Another option would be to put themes admin into an 'appearance' section under admin/config.

This would be perfect. It would solve the tab issue and allow the module to be located easily (not to mention it would finally get me some satisfaction since I keep clicking on admin/config over and over again expecting it to be there). ;)

Can we before/after screenshots here. I read most parts of this issue, but do not understand the problem/goal as well as the actual changes.

Yeah, multiple topics. We haven't fixed our administrative entrance(s) yet.

- Dashboard is on /admin. /admin always had the full site map of admin links. It doesn't anymore. Configuration page isn't either.
- Then there's 'by task' too, which is also incomplete.
- The need for the full index is squarely in the 20% use case, but of course that's us builders ourselves. I find myself missing it too. The page that has all subs of top level IA items as similar boxes added to the Configuration page, basically.

Catch has a long-standing issue with Appearance as a top level item because it's such a narrow hub, which was triggerd by Jacine's application-design questions :-)

So, 80% UX wise there's no real need for this full index page to exist. Sitemaps work for sites, less so for web operating systems.

We'd rather see module developers try and 'blend in' with what we have now: Toolbar, Dashboard, Configuration, Modules…

From that perspective, this issue is about getting rid of 'by task' and 'by module' without putting up too many new buckets elsewhere. I'm sure the technical summary sounds different!

Thanks! I'm going to interpret and translate that into: (feel free to correct me)

When considering users who do not install admin_menu, then I can perfectly see the requirement for a fully recursive administrative index. Effectively, admin_menu is exactly that recursive index, but always exposed. Since the new IA hides more than it exposes, users need a way to actually find stuff deeply buried down below /admin. I don't think the 20%/80% numbers are correct with regard to that. Every site administrator is going to need that index.

For a proper, usable index, we'd have to

1) remove the "Configure permissions" and "Get help" links. The first should already be part of User module, the second already part of Help module (if enabled).

2) It doesn't look like the current list of links is recursive. I'd like to find all related links for each module. For example, the links for Locale look good, but those for Taxonomy are... moot.

If we'd do those changes, I could even see admin_menu users to find that full index useful (as the menu is "collapsed", and this page would expand everything as overview).

We can't do links for dynamic paths in core yet. So Taxonomy actually was a bad example, as that really has this one item only. :) But for everything else, we should "recurse", taking all static paths being either normal items or local tasks (no actions) into account.

@catch(#39): how would people navigate / get to know that a by-category admin/config exists, which might be a better overview for them? Looks like this would make the admin not explorable for those not running toolbar or a replacement, or am I missing something?

@catch: Seven does not have a way to display a menu block in a meaningful way, as it was intended to provide navigation on-page. So I'm not seeing how the management/navigation menu would show up there. So this could cripple the way we could navigate Drupal admin without a toolbar or toolbar replacement module in my view.

Seven does not have a way to display a menu block in a meaningful way

Does it even deserve the title of a "theme" then? As a themer, I'd consider that as a major bug in the theme, but not in the system. If the theme wasn't designed to work like a proper theme, then it shouldn't be a theme in the first place.

Yeah, well, I'm saying if the proposed solution is to use blocks, then we need ways to use blocks in Seven. The solution proposed here so far is not compatible with Seven's current state and makes navigation bad in different ways while fixing some admittedly confusing screens.

I have applied this patch to CVS HEAD and disabled dashboard and it seems to work. I ran all the system tests and I think it is OK, there were two failures in modules/simpletest/common.test line 1361 and 1374 in the test testDrupalRenderThemeArguments(), but I don't think those tests were related.

I applied the patch in #26 against todays drupal 7.x-dev. and no more admin by-task menu :)

errr, I'm not sure whether we reached a consensus here...?

Dashboard and the By-module page are the two contenders for living at /admin.
By-task can go because the task-driven IA is expressed through dashboard/toolbar/configuration page now
So, lets rename 'by-module' to 'Index', it's a good name. Then:

With dashboard enabled, make it look like this, meaning dashboard lives at /admin.

Without dashboard, 'Index' becomes the fallback /admin:

Also, this issue needs cross-ref with #652122: Fix dashboard as the default /admin local task, where I'll post these same two mockups because this needs to be seen as a whole.

Implementation wise, this patch should probably focus on removing admin/by-task, renaming admin/by-module to 'Index', and make sure it is/can be the full module-links index it wants to be.

#26: by-module-index.patch queued for re-testing.

is yoroy in #54 suggesting a different solution than the one in the patch in #26 gives? Is the decision needed here on which direction to go #54 or #26?

#26: by-module-index.patch queued for re-testing.

Here's a spreadsheet with:

column A: what's currently on /admin/by-module (to be renamed to /admin/index)
column B: what's currently on /admin/config
column C: what's currently on/under toolbar pages Content, Structure, Appearance etc.

This is with all core modules enabled.

If I understand it correctly, A must show what's on (B + C) combined. Haven't started really comparing the lists yet.

Nitpicky text change only, changing
"This page shows you all available administration tasks for each module."
to
"All available administration tasks for each module."

Attaching full page screenshot, it's quite the page!

You'd think I learned not to edit patch files directly by now…

Reviewed patch #66 - works just fine - thanks @yoroy. Nice UI

Reviewed patch #66 - works as advertised... even has some nice comments in code ;-)

(part of LA Drupal code Sprint #LADrupal)

This could use 1 more person checking the list in #59 though

Back to 'needs review', per yoroy.

I really hate to ask this, but is there any way to put that spreadsheet in a, er, less open format? :P~ I'm on airport wifi and can't really download NeoOffice to view. :( Maybe a Google spreadsheet that reviewers could just click a link to see?

Here is your google spreadsheet guy ;)
I imported the .ods in google spreadsheat (you can do this to webchick so you don't need to download any office programme ;) )

http://spreadsheets.google.com/ccc?key=0AnMmC4VjMWMXdDZzRHEzNU1mTzFfM3At...

Just went through the spreadsheet at #59. Columns 2 and 3 (admin/config and toolbar) are all fine. admin/index [designate] displays the following with all core modules:

Blocks on Left side:

AGGREGATOR
Feed aggregator
CONTACT
Contact form
FIELD UI
Field list
FORUM
Forums
IMAGE
Image styles
MENU
Menus
PATH
URL aliases
SEARCH
Search settings
Top search phrases
STATISTICS
Recent hits
Statistics
Top pages
Top referrers
Top visitors
TAXONOMY
Taxonomy
TRIGGER
Triggers
USER
Account settings
People

Right Hand Blocks:

BLOCK
Blocks
DATABASE LOGGING
Recent log entries
Top 'access denied' errors
Top 'page not found' errors
FILTER
Text formats
HELP
Help
LOCALE
Languages
Session language detection configuration
Translate interface
URL language detection configuration
NODE
Content types
Content
PROFILE
Profiles
SHORTCUT
Shortcuts
SYSTEM
Actions
Appearance
Clean URLs
Configuration
Date and time
File system
IP address blocking
Image toolkit
Logging and errors
Maintenance mode
Modules
Performance
RSS publishing
Regional settings
Site information
Status report
TESTING
Testing
UPDATE MANAGER
Available updates

Which is different to column 1 of the spreadsheet at #59. However, this is the same listing the current DEV of D7 looks unpatched, so believe the patch is fine.

Changed to RTBC again....

But just following up from #72...

But note that neither 'Configure permissions' nor 'Get help' links are shown at admin/index. This means that none of the following are listed at admin/index in a 'Standard' installation (with only core modules enabled): Book, Comment Content translation, Contextual links, Overlay, PHP filter, Poll, Toolbar, Update manager

#73 sounds like needs work to me

As per #40 and #61, that behavior is intentional - but sounds like it definitely needs some discussion as to whether it is a good idea or not.

The patched admin/index [desig] without the help/permissions (and associated modules) is simple and clean; this is because the admin/index [desig] listing is now only listing features and functionality that can be configured. Help/permissions is not really a functional aspect of the site's features... this is more related to the 'User' piece (as in D6), 'Help' piece and the expanded 'Module' piece (now featuring in D7). My view is that that admin/modules has a much better UI [than D6] for 'everything' and launches straight into related help and permissions for each module - the only thing that will trash this page will be if the projects/packages for D7 contrib modules is a little random (as is the case in D6), leaving Cmd-F the only way to navigate admin/modules.

I'm a little reluctant to RTBC this, as the most recent status changes have been quickly reversed. However, we reviewed this issue among 4 people (talking face-to-face) at a code sprint, which is probably more productive and comprehensive than forum postings. I know we leapt in to this issue late in the day, and we initially negated to formally check off the checklist at #59, but this patch (#66) will make site admin less confusing... and it works.

So if someone else agrees - who maybe has been working on this issue a little longer - then please RTBC to get this patch committed.

thanks a lot for that recap snorkers and I agree with your assesment.

Yeah, I think that makes sense. An additional reason is that the Help and Permissions links would look very out of place on this page now since they do not come with descriptions but all the real menu items do. (However, if we are relying on the module page for this stuff, that makes the proposed rollback at #293223: Roll back "Hide required core modules" even more important...)

Looking at the actual patch here:

1. -  // Only continue if provided arguments are expected. This function serves
   -  // as the callback for the top-level admin/ page, so any unexpected arguments
   -  // are likely the result of someone typing in the URL of an administrative
   -  // page that doesn't actually exist; for example, admin/some/random/page.
   -  if (isset($arg) && substr($arg, 0, 3) != 'by-') {
   -    return MENU_NOT_FOUND;
   -  }
   

   As ugly as this code is, I think we need to preserve some version of it. Otherwise, when the dashboard module is turned off, visiting admin/some/page/that/does/not/exist will show you the index whereas we actually want it to return "page not found".

   I think the way the new patch works the new code could be a lot simpler though, hopefully just something like this:

   if (isset($arg)) {
     return MENU_NOT_FOUND;
   }
   

   If we're lucky? :)

2. -  // Check for status report errors.
   -  if (system_status(TRUE) && user_access('administer site configuration')) {
   -    drupal_set_message(t('One or more problems were detected with your Drupal installation. Check the <a href="@status">status report</a> for more information.', array('@status' => url('admin/reports/status'))), 'error');
   -  }
   

   Do we really want to remove this? D7 currently displays that on the configuration page too, but for a site that isn't using the dashboard, /admin is likely still going to be their main central administration page, so they should possibly get that message there.

   This might be better dealt with in a followup, rather than here.

3. A side effect of this patch which I don't think is intentional is that in addition to removing the "Configure permissions" link from the index page, it is also removed from the individual module help pages (e.g., admin/help/block, admin/help/filter, etc).

   That needs some discussion - I think by the time you are looking at a module's help page, you are really trying to figure out how it works and see everything it is able to do, and the "Configure permissions" link is an important part of that. I think help.module needs to add that link back in.

4. -      return '<p>' . t('This page shows you all available administration tasks for each module.') . '</p>';
   +    case 'admin/index':
   +      return '<p>' . t('All available administration tasks for each module.') . '</p>';
   

   The new version is not a grammatically complete sentence.

   (And why are we even changing it as part of this patch? The purpose of the page didn't change, so I think rewording/shortening the help link doesn't necessarily have to be part of this issue.)

5. -      return '<p>' . t('This page shows you all available administration tasks for each module.') . '</p>';
   +    case 'admin/index':
   +      return '<p>' . t('All available administration tasks for each module.') . '</p>';
   

   An additional issue here - because we are only checking 'admin/index', this means that people who do not use the Dashboard module won't ever see this help text when they visit the admin page. (On the other hand, we can't simply just add case 'admin' to this list because then it will show up on the Dashboard too, which we don't want.)

   I think we can fix this by adding case 'admin' to the list and then having the Dashboard module use hook_block_list_alter() to prevent the System Help block from displaying on the Dashboard page at all. That's ugly, but probably acceptable. If the solution turns out to be any more complicated than that, though, let's leave it for a followup issue, because the Help module is kind of a mess and we don't really want to rewrite it as part of this issue :)

6. +        if (!empty($items[$path]['localized_options']['attributes']['title'])) {
   +        $items[$path]['description'] = $items[$path]['localized_options']['attributes']['title'];
   +      unset($items[$path]['localized_options']['attributes']['title']);
   +    }
   

   (minor) Crazy indentation here.

7. (minor) Grepping the codebase for "by-module" shows one instance still left over, in menu_set_active_trail():

         // The title of a local task is used for the tab, never the page title.
         // Thus, replace it with the item corresponding to the root path to get
         // the relevant href and title. For example, the menu item corresponding
         // to 'admin' is used when on the 'By module' tab at 'admin/by-module'.
   

   We should ideally rewrite that code comment to remove the reference.

Ping pong from #293223-50: Roll back "Hide required core modules":

Another reason in favor of [displaying required core modules] is that [this patch] is close to going in, and one of the effects of that patch is to remove the "Help" and "Configure permissions" links from the index page at admin/by-module (soon to be renamed admin/index). The theory is that those links are best accessed from the modules page, but that won't work for required modules unless we get this one in too.

As far as that other issue is concerned, the theory sounds wrong to me, since the current help and permission links are located on a page that can be accessed by "site configurators". The modules page requires additional permissions, and actually allows a user to change all available configuration options.

If one needs to RTFM, then that one should rather not deal with module installation/enabling/disabling/etc.

Re #80: Well if you are looking for help in the first place, then there is this big 'ole Help link for you to click, where you will currently (but sadly not with the latest patch) find links to the respective permission pages. So there is no net loss for people without access to the modules page (but with access to the site-configuration).

#25: by-module-index.patch queued for re-testing.

So can anyone fix the issues and roll a new patch? Lets fix this critical

tag

This is bugging me big time. It is hard enough finding modules on the Admin screen in D6. But in D7 some modules are not even listed on the Admin by Task, nor the Admin by Module page. For example:

The Admin by task page does not show

• (Users) Configure permissions
• (Users) Account settings

The Admin by modules page does not show

• Any Reports
• Modules!!!

There MUST MUST MUST be an "Administer page Index" where EVERY modules is listed on the ONE page. I don't want to take two clicks to get to a module admin page. And where do I find a newly installed module admin page... how many sub-menus must I search through.

The D6 Admin page was fine because it was complete.

Great to see you support the obective of this issue. Could you maybe update the patch to fix the problems you found? Thank you.

I wish I had enough programming know-how to do so. For now, I can only offer usability and testing, and occasional documentation of various modules.

rerolled #66 @ HEAD and addressed the points in #79 and #83

#66 already addresses all the concerns in #86, save "configure perimssions", which is restored by the attached patch.

Fixed notices, links to permissions.

Looks good, I would argue Configure permissions - should say Configure *module name* permissions. But thats not really something I would keep from this going through.

Uploading a screenshot of what the patch does.

Almost there then but needs work

Regarding @catch's second point I think that is already done in the patch - he removed it from the code in one place but added it back correctly in the other.

Looking at the code, it mostly looks great now - I only see two issues:

1. +    // Since Dashboard takes over the admin page, unset the help text.
   +    foreach ($blocks as $key => $block) {
   +      if ($block->region == 'help' && $block->delta == 'help' && $block->module == 'system') {
   +        unset($blocks[$key]);
   +      }
   +    }
   

   I don't think we want to check the block region here - the module and delta should be enough. If someone moved the block to a different region, we still don't want to show them the wrong help text in it.

2. +    case 'admin/index':
   +      return '<p>' . t('This page shows you all available administration tasks for each module.') . '</p>';
   +    case 'admin':
          return '<p>' . t('This page shows you all available administration tasks for each module.') . '</p>';
   

   Instead of repeating the return statement, the case statements could just be combined like this I think:

     case 'admin':
     case 'admin/index':
       return '<p>' . t('This page shows you all available administration tasks for each module.') . '</p>';
   

**********

In terms of the screenshot, I have two comments:

1. The configure permission links look odd to me at the top of the list - I suspect they would be better at the bottom (where they would look less out of place, especially without a description)? I realize they've always been at the top since at least Drupal 6, but it just looks weirder now with the new page design.
2. The page title and tab titles look a little odd, no? This is the index page, but the main title on the left says "Dashboard", etc...

Regarding @catch's comment in #95

the new param should be documented.

I wondered whether the 4 lines of comment already in the patch are sufficient or do we also need an @param in the function header?

+  // Only continue if provided arguments are expected. This function serves
+  // as the callback for the top-level admin/ page, so any unexpected arguments
+  // are likely the result of someone typing in the URL of an administrative
+  // page that doesn't actually exist; for example, admin/some/random/page.

I also agree with @David_Rothstein's comment in #97, the case statements can be combined.

@David Regarding 2. Yes, lets move them to the bottom - I would recommend adding in *module name* then, if possible.

That the page title is something you probably did wrong :P

1. Since we now have a full page of modules displayed, can we not also display a link/icon to its
   • Drupal Help page
   • Project home page.
2. Can we use less technical jargon in places, eg.
   • Field UI --> Fields
   • Nodes -- > Nodes (Content and types)
   • Clean URI --> Clean web addresses
   • Taxonomy --> Taxonomy (categories)

@iantresman: This is out of scope for this issue, so if you want this to be discussed/changed, I suggest you open a new issue for it

I've amended the patch so that it includes some of the suggestions from #97

1. Combined the case statements for admin and admin/index
2. Removed the check for $block->region
3. Moved the Configure permissions links to the bottom of each module section

Using a clean install of HEAD, patch applied cleanly.

Had to clear cache to see updated Dashboard main page, otherwise I was seeing these errors:

Warning: call_user_func_array(): First argument is expected to be a valid callback, 'system_main_admin_page' was given in menu_execute_active_handler() (line 476 of /home/patrick/projects/personal/d7/includes/menu.inc).

(Not sure if that matters or not...)

Confirmed "Configure Permissions" are on and at bottom of each Module listing, except Database logging, and Update Manager (not applicable, I'm guessing).

From a higher level viewpoint, I'm confused, though, by the "main" Dashboard page being empty, then a tab that says "index". Isn't admin the index page? Can't admin/index just be admin? Maybe I'm missing the point (customized dashboard vs. default), and maybe it's a nomenclature issue (should "index" be renamed to something else), but this is my initial impression.

-pat

Same experience as thund3rbox, had to clear cache but otherwise fine. Removed Dashboard module and all worked as expected. I have attached a new patch with the added documentation for $arg in system_admin_by_module() per #95. I think this is good to go.

#103, #104 The experience should be :

1. Simple profile, no dashboard installed; admin/index lives on /admin.
2. Disabled dashboard; admin/index lives on /admin

3. Default profile, dashboard installed; admin/index lives as local task of Dashboard which lives on /admin.

Yes, all these cases are functioning as specified.

@heyrocker, the patch on #104 looks identical to that on #102 - wrong patch maybe?

Yes you are correct, thanks.

I think this is looking good now, ready to be committed?

Another screenshot of the full index page. Should there be another re-roll, than bojhans suggestion to rename 'configure permissions' to 'configure permissions is still a good one.

Re; comment 2 in #97: "The page title and tab titles look a little odd, no? This is the index page, but the main title on the left says "Dashboard", etc...
". This happens everywhere. When you choose a not-default local tab, the page title is not updated to that page's title. Is how it goes in D6 and apparently wasn't changed.

Tested this again, and all these scenarios work as intended:

1. Simple profile, no dashboard installed; admin/index lives on /admin.
2. Disabled dashboard; admin/index lives on /admin
3. Default profile, dashboard installed; admin/index lives as local task of Dashboard which lives on /admin.

And 4: Default profile, dashboard disabled: admin/index lives on /admin works as well. I'm going to trust heyrocker and matason on fixing the last code comments and but this rtbc again.

I've re-rolled the patch adding in the module name so that the links now read:

Configure module_name permissions.

These links are also displayed on the admin/help/module_name pages, which I think is fine.

Thank you!

Looks to me from a quick glance at the screenshot and code that this is using the machine-readable module name rather than the human-readable one?

If so, this should be needs work - we shouldn't use the machine-readable name here.

Yes, the patch used the module's machine name.
Rerolled #111: replaced module's machine name with human name from system_get_info(), static cached.

Ah, yes, I've a lot to learn!

rerolled with catch's suggestion

Is this really the right issue to be adding static caching to system_get_info()? Shouldn't that be its own issue? I really feel like this is getting sidetracked.

Is it really worth adding (another) static cache here? I believe one wasn't added originally to this function because it's generally only used on admin pages anyway, like this one....

I think we could optimize it just as well by making the human-readable name a parameter to system_get_module_admin_tasks() and then having the caller call system_get_info() once and pass it in. Maybe a little ugly, but it's not a commonly used function.

(Or just not bother optimizing it at all, honestly? Again, this is an admin page, so it doesn't need to be micro-optimized, in my opinion.)

removed overzealous $reset param.

no calls to drupal_static_reset() were added.
Only 3 functions call system_get_info(), and none during operations that touch {system}, so that should be OK, right?

cross-posted with David and heyrocker.

Here's a new and improved patch with a more holistic approach (i think).

There are only 2 functions that call system_get_module_admin_tasks():
system_admin_by_module() - which calls system_get_info and iterates over it -- no big change there.
help_page() - this function prints a page listing general help for a module. Currently, this function locates and displays the name of the module by parsing the .info file.

So, in this updated patch I'm killing a few kittens with one stone:

• rerolled #111 without static cache
• added $info param to system_get_module_admin_tasks()
• updated help_page() to get module info from system_get_info() instead of parsing the .info file.

+++ modules/dashboard/dashboard.module	26 Jul 2010 20:46:35 -0000
@@ -87,6 +87,14 @@ function dashboard_block_list_alter(&$bl
+  else {
+    // Since Dashboard takes over the admin page, unset the help text.
+    foreach ($blocks as $key => $block) {
+      if ($block->delta == 'help' && $block->module == 'system') {
+        unset($blocks[$key]);
+      }
+    }
+  }

Not related to this issue? (and also, the comment does not really explain anything - I can already read from the code what it does, but have no idea why on earth Dashboard module would want to hi-jack each and every implementation of hook_help() that tries to display help text on a page that contains the dashboard?)

+++ modules/locale/locale.module	26 Jul 2010 20:46:35 -0000
@@ -132,6 +132,7 @@ function locale_menu() {
   $items['admin/config/regional/language/configure/url'] = array(
     'title' => 'URL language detection configuration',
+    'description' => 'Configure URL language detection',

@@ -139,6 +140,7 @@ function locale_menu() {
   $items['admin/config/regional/language/configure/session'] = array(
     'title' => 'Session language detection configuration',
+    'description' => 'Configure session language detection',

These two should be MENU_CALLBACKs actually. They are accessed via individual "Configure" links only. Hence, instead of adding a description, we just want to add 'type' properties. The 'title' is taken over as page title though, so must be kept.

+++ modules/system/system.admin.inc	26 Jul 2010 20:46:35 -0000
@@ -143,8 +79,24 @@ function system_admin_menu_block_page() 
+function system_admin_by_module($arg = NULL) {
...
+  if (isset($arg)) {
+    return MENU_NOT_FOUND;

Would it make sense to rename $arg into $unexpected_argument ?

+++ modules/system/system.module	26 Jul 2010 20:46:36 -0000
@@ -2202,7 +2195,6 @@ function system_update_files_database(&$
  * @param $type
  *   Either 'module' or 'theme'.
- *
  * @return

Please revert. See http://drupal.org/node/1354

+++ modules/system/system.module	26 Jul 2010 20:46:36 -0000
@@ -2210,12 +2202,11 @@ function system_update_files_database(&$
 function system_get_info($type) {
-  $info = array();
   $result = db_query('SELECT name, info FROM {system} WHERE type = :type AND status = 1', array(':type' => $type));
   foreach ($result as $item) {
-    $info[$item->name] = unserialize($item->info);
+    $info[$type][$item->name] = unserialize($item->info);
   }
-  return $info;
+  return $info[$type];
 }

I can only guess that these changes date back to a static cache approach of this patch?

Powered by Dreditor.

I've just applied the patch, seems like line 2762 needs to be:

$permissions_menu_item['title'] = t('Configure @module permissions', array('@module' => $info->info['name']));

Instead of the old $info['name']

I think my comment inadvertently changed the status set by @sun, setting to "needs work".

Sorry for the half-baked patch - too many irons in the fires.
In addition to the changes described below, fixed an undefined index array in help_page().

Re: #125:
Thanks for the thorough review.

1. Dashboard / Help hack - see comment #97, bullet 1. I would love to do this differently, please advise. Leaving in for now.
2. Locale - removed descriptions from patch, switched to type MENU_CALLBACK
3. $unexpected_argument - I like this. Updated the variable name.

4. +++ modules/system/system.module	26 Jul 2010 20:46:36 -0000
   @@ -2202,7 +2195,6 @@ function system_update_files_database(&$
     * @param $type
     *   Either 'module' or 'theme'.
   - *
     * @return
   

   Oops - I was rolling back changes and following the convention used elsewhere in the same file (e.g. system_theme_settings()). Reverted to HEAD.

5. system_get_info() - yes, you are correct, these are vestigial changes.

Re #126:
Actually the wrong parameter was passed by system_admin_by_module() - should have passed an info array, not an info object.

Last one from me today.

Dashboard / Help hack - see comment #97, bullet 1. I would love to do this differently, please advise. Leaving in for now.

Also see bullet 5 of #79 for the original motivation. I'd love to do this differently too, but couldn't think of a better way.

I am not sure but maybe instead of unsetting the variable we could do something with #access = FALSE to make it easier for another module to override this if it wanted to? (And it sounds from @sun's review that a more verbose code comment would at least be a good idea.)

#access FALSE sounds nice, but definitely needs a more verbose comment that explains the potential "WTF?" - we can basically copy bullet 5 of #79 and slightly rephrase it.

Can you elaborate on the suggestion 'do something with #access = FALSE' in #129 and #130?

Are you suggesting there's some way of controlling the display of the block by setting #access = FALSE which would be cleaner because it does not destroy the block as the patch does now?

I am reasonably sure I am misunderstanding this suggestion as I could not see that this was possible. I did however try setting the region to BLOCK_REGION_NONE which seems to achieve the desired result:

function dashboard_block_list_alter(&$blocks) {
  if (!dashboard_is_visible()) {
    foreach ($blocks as $key => $block) {
      if (in_array($block->region, dashboard_regions())) {
        unset($blocks[$key]);
      }
    }
  }
  else {
    // Since Dashboard takes over the admin page, unset the help text.
    foreach ($blocks as $key => $block) {
      if ($block->delta == 'help' && $block->module == 'system') {
        $blocks[$key]->region = BLOCK_REGION_NONE;
      }
    }
  }
}

Forgive me if I am being silly and missing something obvious here, I am just trying to keep up with this issue and help where I can :)

comment 125:

no idea why on earth Dashboard module would want to hi-jack each and every implementation of hook_help() that tries to display help text on a page that contains the dashboard?

Actually, this patch will only hi-jack a single implementation: system module's block with delta "help".
Since (module, delta, theme) are a unique key in {block}, we can rest assured that no other modules' blocks will be overridden.

AFAICT, in order to use #access to hide the block, we'd have to tap into theme_block() since System Help uses the default implementation. Seems like that would be pushing the logic into a layer where it doesn't belong. Documentation for hook_block_list_alter states that setting the block's "content" property will prevent the block's hook_block_view from firing.

So, the attached patch is a reroll of #128 and:

• sets System Help's block->content = FALSE, instead of the more destructive unset().
• adds more verbose documentation to explain why this is done:
  

      // System module defines help text for 'admin' and 'admin/index', since it
      // serves both those URIs when Dashboard is not enabled. When Dashboard
      // module is enabled, 'admin' is served by Dashboard, and System Help is
      // irrelevant. Setting System Help block's content property to FALSE
      // prevents system_block_view from running, effectively hiding the block.
      // @see http://drupal.org/node/699848#comment-3095302
  

+++ modules/dashboard/dashboard.module	27 Jul 2010 15:57:48 -0000
@@ -87,6 +87,19 @@ function dashboard_block_list_alter(&$bl
+    // System module defines help text for 'admin' and 'admin/index', since it
+    // serves both those URIs when Dashboard is not enabled. When Dashboard
+    // module is enabled, 'admin' is served by Dashboard, and System Help is
+    // irrelevant. Setting System Help block's content property to FALSE
+    // prevents system_block_view from running, effectively hiding the block.
+    // @see http://drupal.org/node/699848#comment-3095302
+    foreach ($blocks as $key => $block) {
+      if ($block->delta == 'help' && $block->module == 'system') {
+        $blocks[$key]->content = FALSE;
+      }
+    }

This hides any help text added via hook_help() on pages that display the dashboard, not just the specific help texts mentioned here, while it intends to only hide System module's help text. That's what I tried to say in #125.

At the very least, we need to document it (the WTF) similar to how I phrased it above, potentially prefixed with @todo. ;)

btw, in comments, all function names should be suffixed with parentheses, i.e., myfunction(), and we only link to drupal.org issues in completely totally weird cases that really need more in-depth essays about why the world is so ugly and bad; in all other cases, code comments should sufficiently explain the problem space for all mankind 2010 A.D. and after.

40 critical left. Go review some!

@matason's suggestion in #131 about using BLOCK_REGION_NONE sounds pretty reasonable, although I guess the $block->content = FALSE setting works well too. (I only suggested setting #access = FALSE based on a hunch, because I knew at some point this gets rendered by drupal_render() where anything with #access = FALSE gets skipped, but it sounds from above like that won't work.)

For the @todo, I'm not sure what it would say... "Rewrite the help system so this isn't necessary"? :)

Another option here is to use some kind of arg() check to make sure we are on 'admin' or 'admin/index' before removing the block, and thus we more specifically target the help text which system.module added, addressing at least partly the issue @sun brought up. Then there might not be a @todo at all, as I think there is an implicit @todo about removing all uses of arg() throughout core whenever that becomes possible...

The help text is

This page shows you all available administration tasks for each module.

Here's the patch with the help text removed, for argument's sake.

+++ modules/system/system.module	27 Jul 2010 15:57:51 -0000
@@ -87,7 +87,8 @@ function system_help($path, $arg) {
+    case 'admin':
+    case 'admin/index':
       return '<p>' . t('This page shows you all available administration tasks for each module.') . '</p>';

It's this help text that is removed. Doesn't really make sense on a page displaying a dashboard, true.

Counter proposal: Drop this help text entirely. Explains "This is a list." on top of a list, thank you.

P.S.: @todo D8: Tie help closely into menu router to prevent such issues.

Powered by Dreditor.

Ok. Here's the summary of this issue as I understand it:

1. The "admin/by-task" page, and the "admin/config" page are two slightly different looking screens that both intend to set up a screen for navigating around the administrative section by general topic. This is weird.
2. It's especially weird, given that if you don't have Dashboard module installed, this becomes your default admin page, so you effectively end up with two top-level navigation items that end up at sort of the same place.
3. The "admin/by-module" page is a useful way of getting access to just about all of the configuration options that modules expose. However, it's buried in a tab called "by module" that no one ever finds.

Solution: Rename "By module" to "Index", remove "By Task", and have "Index" slot in for Dashboard when it's disabled.

I'll take a look at this a bit later today. The only real argument I can think of against it is that the D6-style "cockpit of options" IA really didn't test well at UofM, and "By module" adds even more options, as seen in http://spreadsheets.google.com/ccc?key=0AnMmC4VjMWMXdDZzRHEzNU1mTzFfM3At....

OTOH, they're alphabetically sorted by module name, so it'll probably be easier to find stuff than before, once the user understands what a "module" is.

Oh, hrm. Well, and also that the default admin page before, for non superusers, was something you could mostly put in front of content editors and the like; it head headings like "Content management" and links like "Content" and "Comments".

Now, that heading gets changed to "Node" on the default admin index. So will effectively be saying "You either need to use Dashboard or a Dashboard module replacement for your content editors in Drupal 7 if you don't want them exposed to scary words like node." That's also going to be the absolute last place content editors would think look for "comment moderation" links.

This is mostly worrying about the case where dashboard is disabled though. Disabling it is already an act of superuserism and then yes, you get this bigass list. The new, hopefully more sensibly organized cockpit for most functionality is the configuration page. This module-index page is here because people in here (which we can safely assume are 24/7 drupal superusers) insisted on having a single page with access to everything.

So will effectively be saying "You either need to use Dashboard or a Dashboard module replacement for your content editors in Drupal 7

Well, that's been a guiding principle from the start so yes, that is what we're saying :) Toolbar, shortcuts, configuration page were all introduced to give people (a sense of) control without having to expose them to a long list of jargon-ish links.

Disabling it is already an act of superuserism and then yes, you get this bigass list.

Er, no. Bear in mind that every single site upgrading from D6 to D7 will not have Dashboard enabled...

And who does this upgrade? Probably not someone who is only a content editor.

Right, so... what I'm saying is the person who does that upgrade (user 1, the sysadmin) is going to have to do extra work in order to not get absolutely befuddled e-mails from their clients when they attempt to get to the comment moderation screen from /admin (to which their theme might well have a nice link to automatically).

We can accept that this is an okay tradeoff, and just lump it under the other myriad of things you need to do when you upgrade major core versions, but the point stands that for non-web admins, this change is pretty massively invasive, and directly exposes them to system guts they never previously had insight to without a lot of digging.

If the UX team is ok with that, then I am too. I just want to make sure that the ramifications of this patch for the 80%+ of Drupal 7's first users are clear.

I find it someone strange to provide buy-in on a bad upgrade user experience, obviously thats not desired. But I think we assumed that we can't enable Dashboard by default for purely technical reasons, hence why we didn't put that as a top priority for this getting in. Even without that, this patch should probably be oke - for people who upgrade, enabling new features is likely to be a first goal?

We could help the upgraders along by including a one-time message after the upgrade has finished like:

In order to have the best possible experience administering your Drupal 7 site, enabling the following new modules is recommended:

• Contextual Links
• Dashboard
• Toolbar
• Shortcut
• Whatever else I'm missing...

Ok, I reviewed this patch functionality-wise and am still pretty hrm about it.

I like this general concept a lot:

Dashboard being the admin landing page, custom-tailored to day-to-day tasks, "Index" being essentially a "site map" for the admin panel.

There are a couple of problems with Index:

The fact that "Comments" isn't showing up here at all appears to be a bug, at any rate, so needs work for that. But the permissions links add a whole lot of weight to this page that the old "By task" didn't have, with its "relatively" manageable 7 categories of stuff, showing off the new IA front and center. Also, "index" is misleading; despite the fact that we add the permissions links, there are still modules which expose admin pages, that don't show up here.

Frankly, in Drupal 7 we already have a "by module" page with direct links to configuration pages and permissions: it's called the "Modules" page. So in that light, "By module" feels totally redundant, and we still have two screens that show essentially the same information, slightly differently formatted. I'm not sure that this help us out much in solving this challenge, and in fact wonder if we'd be better off dropping the "By module" tab instead.

This puts a large burden on everyone upgrading from Drupal 6 (as well as everyone currently installing with the Minimal profile) to go from having a relatively simple, 7-category(ish) link hierarchy organized according to our new IA, to an infinitely-growing morass of links grouped by module name, when content editors, forum admins, etc. and others with access to this screen almost certainly have absolutely no idea what the heck a module even is. This will thrust a bunch of undue work on site upgraders to deal with this issue for their content editors. Overall, it leaves me feeling like this issue is headed in the wrong direction. :\

Why can't "Index" be "By task" again? Because it's too similar to "Configuration"? I kind of agree, but it has several other important links that are not on Configuration, and unless we're planning on changing that (which I don't think we should), we still need an administrative "index" for "mere mortals". At this point, "By task" feels much better for this.

Also just because Configuration on 'by task' looks more crowded now, it's not going to stay that way once you beign to switch on contrib. A lot of modules will move themselves into Structure, a lot into Content and so on.

I know that many of my users, if they see a link named 'Index', will expect to see an index of the site's content that they can use for moderation etc. That might actually be quite a sensible tab to have up there, come to think of it, but I agree with webchick's implication that 'Index' ought to change. 'By task' is better.

@webchick I am not sure, you are saying it needs work but the points you mention are mostly inherent to the by module grouping. By task is basically Drupal 6 sitemap, if you want that sure. But it will probably take a lot of effort to make that page usable, I thought the reason of having this is for those upgrading and developers who need to get quick access to functionally.

Index is definitely better then by task, I have no idea why one would think "by task" is better thats only because you understand that label, any new user or somewhat experienced user has no idea what a "by task" page is. I think you are overstretching the upgrade thing way to much, there is no reason those upgrading this should be the end-all. Content editors should not bother with this page.

Anyways I don't know how to continue on this issue as I am feeling unsure about focusing this on content editors, that was not its intended target audience at all. This was for developers or those with deep understanding finding functionality very fast, the Drupal 6 way. I will leave it up to chx or catch to provide direction here.

re: Comment administration page: Damien prepared a patch over in #652122: Fix dashboard as the default /admin local task, which technically allows to bit-add MENU_VISIBLE_IN_TREE to any local task (or any other item), so as to make it appear in link trees, and thus, also on this index page.

Jacine in #148:

We could help the upgraders along by including a one-time message after the upgrade has finished like:

In order to have the best possible experience administering your Drupal 7 site, enabling the following new modules is recommended:

Yup, and we are already working on that idea here :)

#774316: Remind users about new Drupal 8 features after migration from D7

Trying to parse the other recent comments here: So we don't want to make this page be the main place to start navigating your admin screens - but how many people will that be the case for? Even forgetting all the new D7 modules, so many D6 sites use admin_menu for that anyway (which they will upgrade to D7)....

So maybe the main issue is just that we are putting it at /admin, where the URL indicates it has more importance than it should, and also where it is at the root of all breadcrumbs if the dashboard module is disabled?

So one possibility would be to leave it at admin/index mostly as it is (which worked well in D6 for people who knew about it, and that includes the "configure permissions" part), but never show it at /admin (even when Dashboard is disabled). Instead we could show a very simple listing of the top-level admin categories at /admin, just like we do on admin/structure or admin/reports but one level up. In other words, something like the attached screenshot but with the list mirroring the items in the toolbar:

• Content
  Find and manage content.
• Structure
  Administer blocks, content types, menus, etc.
• Appearance
  Select and configure your themes.
• etc.

Maybe?

First, to Bojhan:

@webchick I am not sure, you are saying it needs work but the points you mention are mostly inherent to the by module grouping. By task is basically Drupal 6 sitemap, if you want that sure. But it will probably take a lot of effort to make that page usable, I thought the reason of having this is for those upgrading and developers who need to get quick access to functionally.

My understanding was that the purpose of the Index page is a one-stop shop for all admin links that a user has access to. A "site map" of your admin section, if you will, as opposed to an intentionally limited list of the tasks/views one needs 80% of the time (that's the Dashboard). Am I mistaken?

Index is definitely better then by task, I have no idea why one would think "by task" is better thats only because you understand that label, any new user or somewhat experienced user has no idea what a "by task" page is. I think you are overstretching the upgrade thing way to much, there is no reason those upgrading this should be the end-all. Content editors should not bother with this page."

In case this wasn't clear, I definitely wasn't suggesting to re-title "Index" to "By task". "By task" is a horrible name. "Index" is great. But I was suggesting to make the current patch use the "by task" page instead of the "by module" page for the index, and fix whatever is wrong with the by task page that makes you feel exposing by-module classification to people who have no idea what the hell a module is, is a better solution.

And you say "Content editors should not bother with this page", but by virtue of them having "access administration pages" permissions, which they need to have in order to get into the things they might need to do, they will see (and therefore bother with) the page. Additionally, it will be front and center on any Drupal site that doesn't have Dashboard module installed, which means all Drupal sites that are not fresh installs or have not explicitly turned it on, which will be most of them, at least for the next 1-2 years.

Basically, getting the UX right here is really key, because this is not some "sub-tab FYI user 1" kind of page like it's being treated. At least not with the way the patch works currently, due to this hunk:

@@ -523,7 +522,7 @@ function system_menu() {
   $items['admin'] = array(
     'title' => 'Administer',
     'access arguments' => array('access administration pages'),
-    'page callback' => 'system_main_admin_page',
+    'page callback' => 'system_admin_by_module',
     'weight' => 9,
     'menu_name' => 'management',
     'theme callback' => 'variable_get',

That's a change to system.module, which means every single Drupal site is affected by this change. Compare the initial admin user experience on the minimal profile (which is the same as a D6 upgrade) before/after the patch to understand why I'm raising a red flag here.

Now, to catch:

On the overall patch I have no energy to fight this but I predict massive confusion with the current situation as it is, and this at least removes one part of this, while clearly not fixing everything that's wrong with the IA in general.

I don't want you to expend a bunch of energy on this, but one thing that I think would really be helpful (esp. with all the various twists and turns this issue has taken) would be to re-summarize what the massive confusion / IA wrongness is that you feel we need to try and fix with this issue. In the OP, you mention 1. the incompleteness of the link listing, and 2. the fact that it looks too similar to admin/config.

A couple of responses to that:

1. I think it's fine that the "by task" page doesn't show every single link; it shows the admin IA / hierarchy, and IMO that's what's important to convey here. http://www.apple.com/sitemap/ doesn't show every single individual knowledge base article on it, nor does it even provide a link to the knowledge base. But it points me at http://www.apple.com/support/ and from there I can find the right set of documents for my product. It's important to note too that this is a categorical listing of a bunch of links, not links grouped by the filename of whatever Python/PHP/AppleScript script is generating them (roughly analogous to what the "by module" page exposes for non-site-builder people).

2. I do see this argument; admin/structure and admin/config look similar, too. The by task page is slightly variant in that the headings are blue instead of black, but that's pretty subtle:

Just throwing this out there, what if we hid descriptions from the index, like so:

Then the index would be more of a true "index", it would look visually distinctive from the rest of admin/*, and that feature to turn off descriptions already exists in core, so we wouldn't have to write much code to get 'er done.

And finally, to David, did you maybe upload the wrong screenshot? That just looks like admin/structure to me.

And finally, to David, did you maybe upload the wrong screenshot? That just looks like admin/structure to me.

I uploaded the right screenshot, but my explanation wasn't good enough :) Basically, I am too lazy to make an actual mockup, so the idea of my proposal is that /admin would look like that screenshot, but with different content (which I started to write at the bottom of #156...)

In other words, just like admin/structure currently displays a simple list of the top-level menu items underneath admin/structure, /admin could display a simple list of the top-level menu items underneath /admin. It would basically contain the same exact information that the toolbar does, and would be a starting point for drilling down into the admin area. Dashboard module could still hijack it if it wanted to.

Oh, ok. I could see that approach too.

I think what we really need to do here is stop and take inventory of what the actual problems are with "By task" that we need to solve, and solve them, rather than radically altering the default administration landing page in everything but new "default" D7 installations, in order to cater to a very small subset of Drupal's audience. That's a complete no-go for me.

I was talking to Bojhan about it in #drupal-usability, and we were scratching our heads with what is so incredibly bad about By task. I mean, sure, UX-wise it's a big ass list of links to greet people, but in that respect isn't any worse than D6's big-ass list of links. Then he pointed out that given the age of this issue, and thought that maybe it was created back when the "By task" page was missing some very critical links, like to /admin/modules, /admin/appearance and such. Is that it? Are we fighting a legacy problem?

I reviewed aspilicious's spreadsheet up above. If you take out all the permissions/get help links (which I would strenuously argue don't need to be on the default admin landing page - if you're a site builder, you can get to these easily from the modules page or "by module" tab), you're basically left with the sub-links under "Configuration". And the fact that we don't show every single link to every single configuration page on the default /admin landing page is a feature, not a bug, since we saw with our own eyes at the UMN usability lab that that many links is totally and utterly overwhelming to new users. So instead, we communicate the hierarchy of the admin panel on /admin, so that users can subsequently drill down and find what they need to find (same as what apple.com is doing on their site).

I'm really, genuinely lost on why this is a critical, beta-blocking, OMG the world is ending issue. It might be that we're missing a couple of key links on by-task, and if so we should fix this. It might be that we need to make this page look visually separate from other admin panels, and if so, we should fix this.

Can someone help me out here?

@webchick: I agree with you. I always thought that having a page with *all* the links as the default /admin would be overwhelming. We should keep /by-task, as long as there aren't any critical tasks, like changing themes or enabling modules, that you can't do from it.

The only thing I'm won't fixing is the solution that replaces /admin/by-task with /admin/by-module, for the multitude of reasons so outlined. So, updating the issue title to reflect that, and also the true nature of this issue.

Ok, cool. So it sounds like the problem now is indeed less about the lack of individual links available on /admin/by-task (it looks like most of those concerns have been cleaned up, and where they aren't, we can fix it in a separate, "normal" issue), but more about the following:

- admin/ doesn't contain links to stuff under admin/config, and admin/config doesn't contain links to other areas of the admin panel, like admin/structure.
- Because the pages look so similar, people will lose track of which page they're on and end up befuddled why they saw a link to content types on here at one point and can't find it anymore, only to realize eventually they're actually at /admin/config instead of /admin.

Does that about sum it up?

subscribing

Does that about sum it up?

I think that's about 95% of it.

The rest is that the other major thing this patch does is that in the course of making admin/by-module into more of a site index, it adds the menu item descriptions to the listing on that page (rather than just the bare menu item names like in D6).

I think that may be a very nice thing we should keep, because of this which I wrote way above:

This is an important issue, because currently with the new IA, there is no single page you can go when you know what you want to administer something but don't know where to find it. #102254: Add an administrative search feature would help with that too, but is almost certainly not going to happen for Drupal 7.

In other words, that thing everyone does every once in a while on /admin of their D6 sites with 100 contrib modules installed: You use ctrl+F in Firefox and search on the page for the thing you're looking for because you don't know how else to find it :)

In D7, we currently don't have any page that makes that (last resort) sort of action easy, which is why making this page into a real index seems like a good idea (even if it doesn't wind up living at /admin).

Allowing that is not a critical issue but is something to keep in mind.

+1 for #166.

Also, I became curious from all the comments and installed D7 taking several screenshots of my experience along the way. Check them out :)

On a personal note, I never used the pages everyone talks about here except Modules in d6, since the fantastic Administration Menu module solved all the issues. With it I dont have to click around in a click bonanza looking for a specific admin link. Also was a bit surprised that it has not gone into core, since it is a massive usability improvement over the standard Drupal admin interface with over 140 000 installs...Every admin option is reachable from every admin page with it. It also distinguishes the admin interface from the default one without needing a separate theme. I don't have to go finding an admin page where I can do stuff.

If the admin page would be accessible for Google, lots of it would get filtered out due to duplication and redundancy. :) Perhaps we would get a penalty as well for operating a link farm :D I think we can take the googlebot view as well, and aim for as little duplication as possible. +1 as well for the #1 post by catch.
Basically we should cut down on a link page linking to another page of links. To me it is extermely frustrating. A single admin link page even if it is long is sufficient to me.

So here come the screenshots.

PS: Just found these
#402058: Requirements for Administration menu in Drupal core
#543914: Administration menu or admin_menu should be in core
"It was the UX team's decision that Administration menu is suitable for advanced users only. I won't (ever) agree with that, but it is their (wrong) assumption."

WTF? Has the UX team taken a look at the stats???? Half of the Drupal sites out there use the awesome module. They surely cannot be wrong.... JUST A BIG WTF

@David I agree, that there is a use case for such a page, but then again its quite a user hacky use case. I mean I am split on this issue, I can see both use cases and not sure which one is more valid long-term.

@giorgio79 With your arguments, Views should be in core, Pathauto ect. This is not the right place to discuss this either. We already agreed we need "one page" index.

@bojhan:Usage stats was just a small part of my arguments, but if we come to that while Pathauto and Views is a massive functionality improvement, Administration Menu is a massive usability improvement and I thought D7 is about exactly that, hence it is reasonable to expect admin menu in core. Anyways, if it was agreed to have a 1 page index and remove duplicated functionality, then I think that will be a step forward.

We are seriously, seriously not re-hashing that admin menu discussion again here (nor the "why is XXX not in core?" discussions). Feature freeze was 11 months ago. Let's focus on "beta 1 before Copenhagen", folks. When D7 gets released you get to pig-pile whatever features you want into D8.

So, who has suggestions on actionable steps forward, given the constraints?

Ok, cool. Let's try that then.

And let's go ahead and carry over the "Index" renaming (which is quite nice) and the improvements to the "By module" tab, and see what we end up with.

Didn't read anything. But if we end up with a short-list and an index we should include deeply hidden drupal 7 config pages as "roles". In drupal 7 roles is a subpage of permissions. Could'nt find this on the current "by task" or "by module".

If this comment is useless ignore it ;)

"We already agreed we need "one page" index."

It will be nice and USEFUL and practical to have this page as overlay / drop-down available at a single click from any admin page ( like Add content is available non-contextually in all pages) so that we get access to this handy at ALL points during the admin and not have to click, re click, grope etc.

In another way of putting it : if we have an "one page" index, it should be available easily and always.
Keeping it as a soft popup / dropdown / thickbox / overlay HELPS in the same way as the "overlay" helps.

I was away on hols for two weeks so missed out on the recent discussions, I'd be happy to work on a patch based on @webchick's comment in #172, also taking in what @catch says about tabs in #174 too if we feel that's the best way forward?

Meh, so what are we doing here? @catch You have been incredibly vague on this one, what is that we want to be coded?

I think we are agreeing that we want to keep what the patch did at admin/index, but only show it at that URL, not at /admin.

And at /admin, show the simple listing of the top level items, as I described above.

Does anyone disagree with that? :)

The only remaining question I can think of is if Dashboard is enabled and takes over /admin, is that simple listing page not accessible at all, or via a tab/local-task? (Adding a new tab to the UI there might be weird, but having this page totally disappear from your site might be weird also.)

#178 makes sense, keep it as separate tab. I'd simply call that tab "Administer", i.e. the original page/link title that Dashboard replaced. Thus, users enabling or disabling Dashboard are not totally lost when searching for "that page that was previously there and called Administer, IIRC..." #drupal-support will love you, thanks.

No, administer is a bogus word. It has no descriptive value to a user - lets keep it at the agreed upon Index label. Having the index moved to front when the dashboard enabled - as a label adds no more confusion then administer.

Spoke to David to really understand what he is proposing :

Two pages
1. By module/Index (as per patch)
2. By-task

This would mean two tabs on the Dashboard, obviously not acceptable. I don't get why we don't just cut down to one page, name that Index fill it with what a useful index would be like and be done with this issue? I mean that proposal was on the ground for weeks and now got changed again.

To clarify, this from above is what we are focusing on now:

The only remaining question I can think of is if Dashboard is enabled and takes over /admin, is that simple listing page not accessible at all, or via a tab/local-task?

Because that is what affects how many tabs show on the dashboard. Various options are being discussed in IRC :)

So the option we suggested:

Index

This shows a filter/secondary navigation that links to by-module.

Note that with http://drupal.org/node/652122#comment-3340494 we'd be able to meet the UX goal of avoiding tabs on the Dashboard altogether - the dashboard would just be a page unto itself, not mixed up with other /admin stuff at all, so no tabs would show there.

And then the new (simplified) /admin page we are introducing here in this issue would work perfectly as the root of all admin breadcrumbs, since it's nice and simple and doesn't assault people with a disorganized list of 8,000 links - thereby removing the rationale for Dashboard needing to take over /admin in the first place.

Maybe with that we can finally be done going in circles? :)

Ok, patch please :) I dont know what you want to show here now - so.

My understanding of this: http://img.skitch.com/20100818-g6crx7a4jbmwsrrpc9aws4iu5q.jpg

Edit to just show the link to the image, not the image itself. Updated mockup below

For the record, I agree with webchick in #166. IMO the current /admin (with dashboard disabled) is decent, and definately not a critical bug. I'd like to see more config links there, but thats a normal priority bug IMO. I'm gonna be humble for once and not waltz in here and demote. But I do think thats accurate.

Lets get a patch up, I applaud the effort to reduce the critically of issues. But its somewhat misleading, because things like this do need to get fixed for the product release.

Like this but without the dashboard tab? I added some notes expected issues with directly mapping the top level IA + subs on here in a contrasting color :)

To me that looks a lot like admin/config when the descriptions are hidden... and not too different from the current /admin.

I thought we were leaning towards a solution that really tried to separate the two pages visually, which is why I suggested #156/#160 since that's the only thing I can think of that (a) makes sense at /admin, and (b) looks nothing like admin/config.

Are we all on the same page here? :)

yoroy's #188 is basically what I suggested at the bottom #158. The removal of descriptions on /admin I believe fixes the problem of it looking too similar to other admin pages. David's suggestion at #156/160 resolves this as well, but does so by removing most links from the page, which is the exact opposite of what was asked for earlier in the issue, a "one-stop shop" for all admin links.

I don't have a really strong preference either way. It sounds like we have two viable proposals, we probably just need some patches so we can compare/contrast them.

I think yoroy's #188 would make sense to people coming from D6, but should have a different title than "Admin". I don't think something like "Administer Site" would be meaningless - the concept of a site administrator is fairly common.

Since Configuration and Help don't have sub-links the top-level headings should probably be links. I don't think Configuration should be subordinated to modules.

It seems to me that possibly this issue originated in the first place because Configuration was put a level down in the IA - burying things like that might have actually made it less easy to administer a Drupal site than it was in D6.

@webchick, you are right; somehow I missed that even though you had a screenshot (I guess because it was a screenshot underneath another screenshot)...

Either approach results in at least one page on the site that is a "one-stop shop" for admin links, but the difference is in where those pages live and how many they are.

Agreed on the need for a patch - this issue is now having a lot of people talk past each other :) And now is a good time since #652122: Fix dashboard as the default /admin local task was just committed.

I will work on a patch (one of them, at any rate).

***

EvanDonovan: I think the assumption (good or bad, and maybe not even explicit) was that everyone would always be using some kind of toolbar in D7. If you are using a toolbar, then Configuration is just as easy to get to as it was before. A "one-stop shop" is necessary to have somewhere, but the question is whether it should be anywhere near as prominent as in D6.

OK, I started with the previous patch (#139) and made the changes I proposed above. The new patch is attached along with before/after screenshots of the two admin tabs and the dashboard.

Some notes about the UI visible in the screenshots:

1. admin_AFTER.png: The top-level /admin page is now just a standard lead-in to the rest of the IA (similar to the toolbar, admin/structure, etc). No chance of confusing it with admin/config.
   • You can see by comparing the screenshots that I left out the "one or more problems were detected with your Drupal installation" message because it's really not clear what pages that should and shouldn't be on but it didn't quite seem like it belonged on this one (we need another issue maybe).
   • The missing periods on the menu descriptions are killing the grammarian in me - I tracked it down and they appear to have been done that way on purpose in #620616: Fix inconsistencies in menu description labeling so they would look good as tooltips in the toolbar, but we'd need to reconsider that here; sentences should have periods at the end :)
2. admin_by_module_index_page_AFTER.png: The admin/index page is a spruced-up version of the former admin/by-module page (with descriptions), just like the previous patch. This probably could still look a bit like admin/config (there is inherent tension in that the more we make a page useful as an index, the more it will look like admin/config) but I think this is much less likely to cause confusion because it's hidden on a tab rather than front-and-center and of course the categories are very different.
3. dashboard_AFTER.png: The dashboard is now without tabs, as promised :)

Some notes on the code:

1. We don't seem to need the $unexpected_argument stuff anymore (at least I don't think), since just like the current behavior where typing, say, "admin/structure/blocks" (when you meant "admin/structure/block") automatically sends you to "admin/structure" which has a nice listing that quickly lets you click on "Blocks" and go on your merry way, same thing can happen here if you type "admin/contents" when you mean "admin/content"... no difference there, so no longer any reason to be inconsistent and have extra code to show Page Not Found to those people.
2. With the old /admin page ripped out, we can roll back some complicated code that was added in to system_admin_menu_block() in #550718: admin/people needs to have a default tab (was: Tabs on admin pages are not accessible from overviews and menus) to support showing local tasks in the listing on that page (and in fact, I need to do that or otherwise admin/index would have shown up as one of the items on the list at /admin). We might need to add some of that back elsewhere though, if we want to show a larger, more complete set of links on admin/index as was discussed recently above.

Can webchick chime in that this is oke or not?

Thank you David. Visual summary of what the latest patch does:

Quick fix for the broken test so we can keep this at "needs review" :)

Makes sense.

Summary:

- by-task becomes a mirror of links in the toolbar
- by-module becomes the index (though styling is too heavy for a full index)
- dashboard gets no tabs

+++ modules/dashboard/dashboard.module	20 Aug 2010 20:51:02 -0000
@@ -30,9 +30,8 @@ function dashboard_menu() {
-    'type' => MENU_LOCAL_TASK | MENU_NORMAL_ITEM,

This also hides Dashboard from the Toolbar though.... ERR. No, it's a normal item now! Nice! :)

+++ modules/locale/locale.module	20 Aug 2010 20:51:02 -0000
@@ -136,6 +136,7 @@ function locale_menu() {
+    'type' => MENU_CALLBACK,

@@ -143,6 +144,7 @@ function locale_menu() {
+    'type' => MENU_CALLBACK,

Those two need to be removed, since these changes break breadcrumbs.

+++ modules/system/system.module	20 Aug 2010 20:51:03 -0000
@@ -87,7 +87,10 @@ function system_help($path, $arg) {
+    case 'admin/by-task':
+      return '<p>' . t('It feels like we need some text here...') . '</p>';

I don't think so.

+++ modules/system/system.module	20 Aug 2010 20:51:03 -0000
@@ -538,15 +541,12 @@ function system_menu() {
   $items['admin/by-task'] = array(
...
+    'title' => 'This tab needs a new name!',

How about "List" ?

+++ modules/system/system.module	20 Aug 2010 20:51:03 -0000
@@ -2008,6 +2008,16 @@ function system_preprocess_block(&$varia
+  // If we are calling this function for a menu item that corresponds to a
+  // local task (for example, admin/by-task), then we want to retrieve the
+  // parent item's child links, not this item's (since this item won't have
+  // any).
+  // @todo Not sure if this is completely correct.
+  if ($item['tab_root'] != $item['path']) {
+    $item = menu_get_item($item['tab_root']);
+  }

The @todo can basically be removed, but then we have to wait for #576290: Breadcrumbs don't work for dynamic paths & local tasks to land first, so you're able to pass tab_root_href here, because menu_get_item() expects a (translated) href, not a (untranslated/dynamic) path, and tab_root is a untranslated path.

+++ modules/system/system.module	20 Aug 2010 20:51:03 -0000
@@ -2017,26 +2027,12 @@ function system_admin_menu_block($item) 
-    SELECT m.load_functions, m.to_arg_functions, m.access_callback, m.access_arguments, m.page_callback, m.page_arguments, m.delivery_callback, m.title, m.title_callback, m.title_arguments, m.theme_callback, m.theme_arguments, m.type, m.description, m.path, m.weight as router_weight, ml.*
+    SELECT m.load_functions, m.to_arg_functions, m.access_callback, m.access_arguments, m.page_callback, m.page_arguments, m.delivery_callback, m.title, m.title_callback, m.title_arguments, m.theme_callback, m.theme_arguments, m.type, m.description, ml.*

Odd. You're removing $item['path'] here? Note that {menu_links} have a link_path, no path.

+++ modules/system/system.module	20 Aug 2010 20:51:03 -0000
@@ -2783,15 +2774,28 @@ function system_get_module_admin_tasks($
+  $permissions_menu_item = menu_get_item('admin/people/permissions');
+  $permissions_menu_item['description'] = NULL;
+  $permissions_menu_item['title'] = t('Configure @module permissions', array('@module' => $info['name']));

menu_get_item() checks the permission to that path, so $item['access'] may be used instead of existing code to determine whether the user is able to access that link.

Not sure why 'description' is not simply unset() ?

Powered by Dreditor.

+1 for #195
Otherwise this is a big step forward.

It seems there is a lack of focus due to new naming conventions in D7 vs D6. Like "People" vs "User" , "Appearance" vs "Themes"...

If we combine the new admin_AFTER.png with admin_by_module_index_page_AFTER.png and make it one page the Drupal admin ui would get a massive focus.
The new admin_AFTER.png while contains the new menu structure which is quite poetic with words lke "People" "Appearance" "Structure", yet it essentially links to the same functionality as admin_by_module_index_page_AFTER.png.

The only functionality that the admin_by_module_index_page_AFTER.png is not linking to is "Appearance" aka Themes...

Otherwise they overlap, as both have links to Help, Reports, Configuration,Content etc.

Yeah from my point of view this needs 2 major things as I pointed out before.

1)

User
- ....
- People (==> two links inside, account settings and IP address blocking)
- Permissions (==> has a secondary tab called roles)
- ....

Should become

User
- ....
- Account settings
- IP address blocking
- People (In this case people link is useless as it only redirects you to a page with 2 links alrdy in this list but maybe there are some top level links with more information inside)
- Permissions
- Roles
- ....

If I'm correct this is what David wants to do (expressed in a less technical way)
I won't be satisfied if it turns out we can't do this. But will accept the technical limits.

2) It needs some basic styling, with basic I mean I think the current page (with "hide descriptions" on) is still not good enough. (you will need to scroll alot)

3) FOR D8: we need to add a filter on this page :), like we were planning to do on the modules page, sometimes I'm like: "let's add a role" ==> "Crap I need to scroll until I find it".

subscribing

I couldn't find catch here at the summit (I saw him earlier, but now he is gone :\), but the screenshot looks good to me (barring minor labeling issues others have mentioned), and I cleared it with chx who's a prominent user of the minimal profile and he says he's good with it, too. Sounds like the UX team is as well, so WOOT! Progress!

Let's address sun's feedback and get this sucker in!

To clarify, I was referring to Bojhan's screenshot in #195.

#200 seems like it's talking about yoroy's suggestion in #188, which we're no longer doing.

I'm quite happy with this approach: a simple list that won't be confused with configuration page and it simply makes sense: It's not the most elegant, but you almost certainly won't feel lost when landing on this screen.

We don't need text along the top. As for this page name, 'Administration' is probably our best bet.

I am working on addressing sun's feedback given that everything else seems to be ok

I am working on addressing sun's feedback given that everything else seems to be ok

Amazon asked for my opinion.

I agree we need to get this fixed asap.

I maybe not entirely agree about which links should be on the index BUT
1) That isn't critical at this point.
2) maybe I misunderstood some things, they maybe get clear when I played with it some more when this is in.

So this gets a (soft) green light for me ;).

I agree with @aspilicious that we definitely want to get a bigger set of links on the index page here. That is the only way in which it will really behave like a true index. (And it's related because currently some of those deeper links do show up on the listing at /admin, which we are removing.)

In terms of the tab names, "List" (in addition to "Index") might work although honestly I'm not convinced that either of those are an improvement over just leaving it at "By task" and "By module"? Which at least had the advantage of being explicit :)

The reason I thought we needed some text on the top of /admin maybe had more to do with the way Seven themes this than actually needing some text for the text's sake. We'll see...

From @sun's review:

+++ modules/locale/locale.module 20 Aug 2010 20:51:02 -0000
@@ -136,6 +136,7 @@ function locale_menu() {
+ 'type' => MENU_CALLBACK,

@@ -143,6 +144,7 @@ function locale_menu() {
+ 'type' => MENU_CALLBACK,

Those two need to be removed, since these changes break breadcrumbs.

Does anyone remember (from above) why those changes were included in the first place? I thought it had something to do with getting the correct links to appear on the index page.

+++ modules/system/system.module 20 Aug 2010 20:51:03 -0000
@@ -2017,26 +2027,12 @@ function system_admin_menu_block($item)
- SELECT m.load_functions, m.to_arg_functions, m.access_callback, m.access_arguments, m.page_callback, m.page_arguments, m.delivery_callback, m.title, m.title_callback, m.title_arguments, m.theme_callback, m.theme_arguments, m.type, m.description, m.path, m.weight as router_weight, ml.*
+ SELECT m.load_functions, m.to_arg_functions, m.access_callback, m.access_arguments, m.page_callback, m.page_arguments, m.delivery_callback, m.title, m.title_callback, m.title_arguments, m.theme_callback, m.theme_arguments, m.type, m.description, ml.*

Odd. You're removing $item['path'] here? Note that {menu_links} have a link_path, no path.

Mostly what I was doing there is rolling back the change from #550718-29: admin/people needs to have a default tab (was: Tabs on admin pages are not accessible from overviews and menus), and I think it's OK because we are treating these as pure menu links in the code that follows, but maybe needs to be looked at more closely. Overall, this code is a bit sketchy no matter what since it is basically trying to build up a "fake" menu link rather than loading one the normal way via menu_link_load().

ok so here is an updated patch addressing sun's main concerns and working against current head.

Name: I've named it "Administration List" because "administration" was too much a repetition of "administer" and too generic

the variable here

$permissions_menu_item['description'] = NULL;

needs to remain null (and not unset) because things fail in various ways if not (theming looks for description). Left the todo in as a reminder.

The rest were dealt with as above.

Amazon asked me to review the patch, and I've done so, mainly with regard to sun' #198 comments. I think they have all been addressed, either in the patch, or otherwise in comments.
I'm slightly confused by the #199 comments though, not sure how they should be addressed.

I highly doubt that this is RTBC, it hasn't gotten a thorough code review yet. Also administration list is somewhat overdoing it, I would go with just "administer"

I am not going to get into a semantic argument but "Administer" is a verb... and Index (which is next to it) is not.

Sun reviewed the code - my changes are fixes to conform to that review and the poster above you had a further look.

Functionally the patch is done. But the 'Administer' page title is not a good choice imo. All page titles are ( or should be) using nouns, not verbs. Verbs are for actions and links, nouns are for labeling form elements, pages, field sets etc. Seems minor but considering it's location near the actual action links ('add foo') it's worth it to avoid any confusion there.

Let's do 'Administration' (we refer to adminstration pages all over the documentation, right?) for the page title and either use the same term for the tab (no reason to use two different words to indicate the same thing, it adds confusion) or only 'List'. I would suggest 'Administration' on the tab: [Administration] [Index] will make a nice pair that support eachother's meaning, whereas [List] [Index] are too similar and meaningless in comparision.

The overall solution feels great though, glad to see this resolved.

@yoroy ok - agreed let us to Administration and get this fixed :-) patch following.

here is the patch once more - with Administration instead of "Administration List" - hopefully this is ok now.

I meant to say use Administration for the actual page title as well so that 'Administration' will also be used in the breadcrumbs etc.
http://skitch.com/yoroy/dugx7/administer-d7

Patch to use Administration both for page title and the tab name as yoroy's suggestion above.

Alright, makes sense. Can we get a technical review?

+++ modules/system/system.module	Mon Aug 23 14:48:41 CEST 2010
@@ -87,7 +87,10 @@
-    case 'admin/by-module':
+    case 'admin':
+    case 'admin/by-task':
+      return '';

Please remove the 'admin' + 'admin/by-task' cases entirely.

+++ modules/system/system.module	Mon Aug 23 14:48:41 CEST 2010
@@ -538,15 +541,12 @@
   $items['admin/by-task'] = array(
...
+    'title' => 'Administration',
...
+  $items['admin/index'] = array(
+    'title' => 'Index',

1) Why is the internal path still admin/by-task? Needs to be updated accordingly.

2) Isn't "Index" the administration index? So "Administration" will get you to your site's administration, but "Index" not? We already clicked on "Administration" (admin). Perhaps "Tasks" (not "by task") wasn't entirely wrong. Tasks + Index would work better for me.

+++ modules/system/system.module	Mon Aug 23 14:48:41 CEST 2010
@@ -2008,6 +2008,15 @@
+  // @todo Not sure if this is completely correct.

Please replace this line with:

@todo Use tab_root_href instead, when made available.

+++ modules/system/system.module	Mon Aug 23 14:48:41 CEST 2010
@@ -2783,15 +2773,28 @@
+  $permissions_menu_item['description'] = NULL;

Still should be unset() ?

Powered by Dreditor.

I've addressed sun's technical concerns and tried his "Tasks suggestion" the url is now "admin/tasks" and the tab is "Tasks".
It's still "Administration" in the breadcrumb and title though.

Attaching the patch and a screenshot, let me know how you want to proceed.

Lovely! Thank you!

Can we all agree that we're finally done with this now?

Well, oke then - because you ask it so nicely.

What do you mean 'finally' :)
Yup, looks good.

1. +      // Move 'Configure permissions' links to the bottom of each section.
   +      if (isset($admin_tasks[-1]) && $item = $admin_tasks[-1]) {
   +        unset($admin_tasks[-1]);
   +        $admin_tasks[] = $item;
          }
   -
          // Sort.
          ksort($admin_tasks);
   +      // Move 'Configure permissions' links to the bottom of each section.
   +      if (isset($admin_tasks[-1]) && $item = $admin_tasks[-1]) {
   +        unset($admin_tasks[-1]);
   +        $admin_tasks[] = $item;
   +      }
   

   Why is this code now being duplicated? Also, it doesn't seem to work anymore - with this latest patch the configure permissions are showing at the top, not the bottom.

   Also just realizing that the code is a little strange for a couple of reasons. Can't we just do

     if (isset($admin_tasks[-1])) {
       $admin_tasks[] = $admin_tasks[-1];
       unset($admin_tasks[-1]);
     }
   

   instead?

2. +  unset($permissions_menu_item['description']);
   

   The comment by @ronald_istos in #209 was correct - this leads to a ton of PHP notices when I visit the index page. We probably should go back to explicitly setting it to NULL (or else fixing the root of the problem in the theme function, if that's where the real problem is).

3. +        // The link 'description' ? either derived from the hook_menu
   +        // 'description' or entered by the user via menu module ? is saved as
   +        // the title attribute. The title attribute is then unset to reduce
   +        // redundancy in admin pages for screen readers.
   

   Those question marks are supposed to be dashes - looks like a text editor messed them up somewhere along the way?

4. We need some kind of change to the Locale module in this patch (which was in earlier patches) - because otherwise with the Locale module enabled I get two menu items ("Session language detection configuration" and "URL language detection configuration") showing up on the Index page with no descriptions. I looked back through the issue and it seems like earlier patches in this issue (e.g. #124) added descriptions for those two items, then later patches changed them to MENU_CALLBACKs instead (which probably made them not appear on the index page at all), then later patches removed that :) Do they belong on the index page at all? Maybe we should just go back to adding descriptions for them and leaving it at that?

Otherwise, this is looking great - there are some followups to propose (like especially trying to get more links to show up on the Index page) but I think that can be handled later.

Me again. Doesn't address everything, but it's a step forward in any case.

Fixed the mangled comment (dash)
Added a chunk that fixes the notices on Index (isset() in seven_admin_block_content, wasn't sure whether to add it to theme_admin_block_content too). Perhaps the NULL approach was better...
Fixed the reordering of tasks as suggested by David (tested, works).

Is there any reason why the admin index callback is still called system_admin_by_module? The function name and the comment could be updated...

Edit: just saw that I replaced the other dash with a comma instead. That can be fixed in the next iteration, let's agree on the next steps first (David's #4, etc)

Thanks, bojanz!

However, studying these functions and their usage a bit more, I'd call them a total mess. Additional changes in this patch:

- Fixed $item['description'] PHP notice.

- Renamed system_admin_by_module() and corresponding theme functions into system_admin_index(), corresponding to the new purpose and link title.

- Moved system_admin_block() into system.admin.inc. This code is needlessly loaded on every single request.

- Use proper db_select queries for better code readability and maintenance.

- Cleaned up theme_admin_block_content() functions.