Modules page: add link to its settings page for each module

Ok, here is a rough implementation that surely needs some work (e.g. icons for links, better positioning of links, etc.).

I like where this is heading but I wouldn't change the font size of the links. Instead, I'd try to re-organize things a bit; e.g. by expanding each module's entry to two rows.

For example, why not move the description down and make it span the entire row?

Another solution might be to get rid of the labels, and to just go with the icons. Just icons could work in this case, IMHO.

It might be personal taste though, so I'm interested in reading other people's thoughts.

@Dries: maybe something like the one shown in the attached mockup?

subscribe.

The screenshot in #7 works for me, but I'd stick with 'Operations' in the header. We just have to make sure that the icons have an title-tag. It would work for me.

Ok, I've put the links right below the description and added the icons eigentor used in his mockup.
Admin links now have classes based on its type:
* module-help-link --> misc/help.png
* module-permissions --> misc/permissions.png
* module-settings --> misc/settings.png
* module-admin-link --> currently no icon

(maybe all classes should be named "module-...-link"?)

Copy the attached icons into /misc.

The page admin/by-module is affected also by this patch (see second screenshot).

New patch, other approach (taken from #7).
Icons are the same as in #11.

@eigentor: I've split the links array in system_get_module_admin_tasks().

I like the screenshot in #13 but it would be good get some feedback from some UX people.

Looking at the screenshot, it looks as the icons have slightly inconsistent gray values. If I had my choice, I'd make them a tiny bit darker to emphasis them a bit more.

What I like about #13 is that it keeps things simple and uncluttered, yet it is a clear improvement over what we have today.

@Bojhan: Here is a screenshot of the whole page.

And here is a full screen with much more modules enabled.
As you can see, Locale inserts 4 configuration links. How do we handle this?

I had a discussion about the links with Bojhan in IRC.

1. elements should be re-ordered --> Settings , Permissions, Help
2. only one settings-link should be shown!

The first one is really simple to solve but te second one needs further thinking.
locale inserts 4 links: admin/config/regional/language, admin/config/regional/language/configure/url, admin/config/regional/language/configure/session and admin/config/regional/translate.
They all share the part admin/config/regional which is the main settings page for module locale.

Question: do we want to trust all modules to put their settings under one main path and extract this? Or do we want to introduce a new hook (e.g. hook_settings_link where modules can provide a link to their main settings page so system_get_module_admin_tasks can grab this and insert it on the modules page?

@Dries: what about image rollovers for the icons?

@Dries We will need direction on that second point. We ran into that issue continuously as we redesigned the module page.

I think this is a good step in the direction of redesigning the module page, we are still faced with the problem of having a mess of links - where some modules will have (n)either permission or configuration links (turning the operations part into inconsistent grid of icons). This has to be solved with more radical design changes, which was tackled in the module redesign issue. Let's not ignore the thinking that has gone into that thread, and assume they did not account for the problems we run into now - since they did.

I want to make a conscious decision, that we take this as a middle step to achieving the user experience that we want, and we use the direction given in the redesign module page as the optimal experience we want to be aiming for.

chx proposed linking the module name to the admin/$bymodule section, but this seems a step beyond that. I hope we can address the concerns he had regarding configuration links, which seem solid - if we want this pattern to scale.

I had an idea about the settings link: why don't we add another menu item type, e.g. MENU_DEFAULT_SETTINGS?

Every module that likes to add a general settings link can do this by defining this in hook_menu:

<?php
function system_menu() {
  $items['admin/config/regional'] = array(
    'title' => 'Regional and language',
    'description' => 'Regional settings, localization and translation.',
    'position' => 'left',
    'weight' => -7,
    'page callback' => 'system_admin_menu_block_page',
    'access arguments' => array('access administration pages'),
    'file' => 'system.admin.inc',
    'type' => MENU_NORMAL_ITEM | MENU_DEFAULT_SETTINGS,
  );
}
?>

Ok. Here we go.
I've added a new flag for menu item types (MENU_SETTINGS_DEFAULT) so a module author can mark a menu item as the default settings item (this only works in combination with MENU_NORMAL_ITEM --> 'type' => MENU_NORMAL_ITEM | MENU_SETTINGS_DEFAULT).

Attached is an icon set with darker images (thanks to eigentor) and a new screenshot of the whole modules page.

@eigentor We can just align to the right, that would fix that issue. No need to add table columns.

You say, I do, my master :-)
Icons are now aligned right.

I've found a solution on how to set MENU_LOCAL_TASKs as default settings items and will post a patch later today.

Btw.: There is nothing in yet to stop maintainers from marking several links as MENU_SETTINGS_DEFAULT. Doing so will result in multiple settings icons on the modules page for this module. Do we want to limit this to one? And choose which?

Personally having trouble groking this. Maybe it's an artifact of looking at static screenshots - you tell me. Here are semi-unbiased (haven't seen these UI's before) comments:

1. I don't understand why there's a lock icon, next to a gear. If it's locked, can you unlock it? If so, maybe the gear doesn't show until it's unlocked.
2. Help as I've seen it has been "more help." As an aside, I personally dislike "more help" because it's overkill and suggests that I previously wanted help. But whatever it is, we need to be consistent.
3. Why do some visuals have multiple gears?
4. Will all UI's have this model? or will modules be the only UI in Drupal that uses icons as actions? Despite the simplicity, users will have to grok how to use UI's that are different from others, e.g., "it said edit over on content types, I wonder if this gear will also be edit"?

Now, I've spent more time on this UI and read comments from the top so understand a bit more of what's going on. So here's some biased feedback:
1. The icons seem like they're inactive (unclickable). They're missing key affordances to suggest they're clickable.
2. The lock icon doesn't say permissions to me. I think if it were a group of people with an edit pencil maybe then I'd get it. Lock tells me it's locked down and I can't edit it
3. Feedback is the same regarding multiple gears and more help? Hopefully the screenshot showing mutliple gears was a mistake. "More help", I'd like to see change to "help" across the board. The usage should be consistant across Drupal as well.

@Noyz:

3. Why do some visuals have multiple gears?

If you look at the latest screenshot you'll that there is only one "gear" icon left for each module (if the module provide settings).

Ok, heres the new patch with MENU_SETTINGS_DEFAULT for local tasks. Yeah!

Re-rolled the patch.

@eigentor a 'key' for permissions is genius! <3

I thought we were able to avoid the text-on-every-button because the text was at the top of the column earlier. With that text now coalesced into 'operations', putting text back on the buttons would bring us full circle.
No, follow the lead of #473268: D7UX: Put edit links on everything and allow us to just use icons where just icons fit.

eigentor: I like the key better than the lock. I still think it could better though. Yoroy, Boyhan and I went back and forth on this a bit earlier. I think vista does a great job at this (http://skitch.com/jeff.noyes/nn9pm/fireworks), but of course there is not a GPL'd 16 x 16 alternative. That said, until an entire suite of consistent, informative, GPL'd icons turns up, I'm sold on where you've landed.

Now we have the concept of a main settings link, we could also consider switching back to regular links (using the normal font size). Would be great to have a screenshot of that for final comparison.

I attached a simple mockup with text links instead of icons.
Hm, icons really look better imo.

So after looking at the comparison some more, I think we thought all to agree on not using text links. It will make the table look inconsistent and unusable. Lets go with just icons and fix any other issues regarding the UX later, as we usability test this page - because that is required to validate any of these icons.

I am going to defer to Jeff Noyes on the icon issue, it seems like the best approach for now.

Using text links make it look really messy. Let's go for the icons. Any thouch ups on the icons should be done in a overall rework of the graphics we're using throughout core, not in here. For now, these icons are good.

Nice work eigentor.

(edit: and if we commit this new grey help icon, make it really replace the blue one we have now. I made that one but it makess way to much noise on pages)

Got a few offsets and 1 hunk failure while applying this, the failure was just whitespace, but I'm still gonna reroll this, just to keep it fresh

I really like the idea behind this, but there are a few quirks with the way it's done...

The currect implementation creates an empty <a>, slaps enough padding (in px I might add) on it to accommodate for the background image to be visible and leaves it at that

imo, it would be a lot better to have the <a> wrap a <img/> and then simply put some margin (in ems) on the image, so we can have some sort of proper text-only zooming and then we can set an alt-text on the image just in case something happened to it (404 or it got blocked by some addon or w/e)

also, we should seriously consider moving towards sprites instead of making more and more icons, but I suppose that can happen in follow-ups

too bad it's too late to get spriteme in core :P

attached is a clean reroll, nothing changed

imo, it would be a lot better to have the wrap a and then simply put some margin (in ems) on the image, so we can have some sort of proper text-only zooming and then we can set an alt-text on the image just in case something happened to it (404 or it got blocked by some addon or w/e)

Yes, that's the better way to do it. Look at how the icons in the Views UI are handled:

<a class="tinkywinky dipsy" title="lala" href="po">
<span>Fallback text</span>
</a>

a.tinkywinky span {
  display: none;
}

a.dipsy {
  background: url(etc…);
}

So, needs work

Actually, I was talking about something like this:

<a href="some/path"><img alt="Descriptive text" src="some/path/img.png" /></a>

as advised by the WCA guidelines at http://www.w3.org/TR/WCAG10-HTML-TECHS/#link-text-images (note the absence of a title attribute on the anchor)

granted that this would make it slightly harder for themes to override this, as it isn't just css

it also wouldn't hurt to have some sort of visual indication when one of these items is highlighted (like regular links do with a dotted outline)

Goes to show you should never trust my coding advice, I'm a hack! :-)

@seutje visual cue is hover we still need to design.

@Bojhan: I don't see how hover state has anything to do with what I posted

my points are:

• anorexic anchors === BAD
• no textual-fallback for missing/blocked images === BAD

Text does look a bit sloppy, but I think that might have been because of the lack of styling.

Regardless, I like the idea of using icons, however we need to make a conscious effort standardize. I reached out to some icon designers for an estimate - under the guidelines that after purchase we have the right to GPL the icons. Hopefully the price will be reasonable. If not, I started working on my own set which you can see on my site (http://www.jeffnoyes.com/icons)

@seutje See the last part of your reply.

@Noyz I agree that we need a standardized effort, although I do believe we cannot just grab any icon set and make it our own. It should be customized to fit the brand that Drupal is, and should also be accompanied by guidelines that allow us to reuse the same icon style. So I am not sure.

Bojhan, totally agree. If your comments are coming from looking at the icons on my site, I'd say that's because it's just a start.

Here is a new patch with <a href="..." title="..." class="..."><span class="element-invisible">...</span></a>. This makes the link text visible by screen readers only.

This kind of feels like a D5 throw-back. I'm really not very happy with the implementation - you are adding extra baggage to a router item that's totally unrelated to serving pages?

We are building most of the info on this page from the .info file - why would you not specify this in the .info? That's already for module meta data, right?

this makes me run screaming... agreed with pwolanin. The whole concept needs some thinking.

So, in the .info file maybe an array of paths for the links to settings? I think that's easier than a hook, which seems a bit overkill.

If you have the paths, you can run some code to get the page titles if that's needed, or just allow one link per module.

@pwolanin:

I think that's easier than a hook, which seems a bit overkill.

There is no hook for the settings link. Its just an additional menu type.

@chx: ideas? thoughts? We're running out of time so this needs to be done very quick.

.

@eigentor:

If there is only one, this MENU_LOCAL_TASK is automatically defaulted as MENU_SETTINGS_DEFAULT.

No: only menu items with flag MENU_SETTINGS_DEFAULT are used on this page. If a module doesn't provide such an item there is no settings link for this module on admin/config/modules.

@52: how exactly do u see this? something like this perhaps (using locale as example):

; $Id: locale.info,v 1.11 2009-06-08 09:23:52 dries Exp $
name = Locale
description = Adds language handling functionality and enables the translation of the user interface to languages other than English.
package = Core
version = VERSION
core = 7.x
files[] = locale.module
files[] = locale.install
files[] = locale.test
settings[] = admin/config/regional/language
settings[] = admin/config/regional/translate
help = admin/help/locale
permissions = admin/config/people/permissions#module-locale

or would we only specify a single path for every type?

maybe it'd be better to make a metalinks array? like this maybe?:

; $Id: locale.info,v 1.11 2009-06-08 09:23:52 dries Exp $
name = Locale
description = Adds language handling functionality and enables the translation of the user interface to languages other than English.
package = Core
version = VERSION
core = 7.x
files[] = locale.module
files[] = locale.install
files[] = locale.test
meta[settings][] = admin/config/regional/language
meta[settings][] = admin/config/regional/translate
meta[help] = admin/help/locale
meta[permissions] = admin/config/people/permissions#module-locale

or is pretty much all this info considered "meta"?

no - both chx and I object to this approach.

I would consider this a usability bug personally, but up to Dries/Angie if they want to look at further patches.

The permissions link can surely be auto-generated if the module implements hook_perm().

How many settings link would we support per module? for a 1st pass in D7 I'd support just one in the .info file.

Last try from myself.
I thought about putting the settings link into .info before I found the solution with MENU_SETTINGS_LINK but decided not to do so (because it felt somehow ugly to me).

As there are no other approaches yet (as patch!) this is the only way in D7 to do this (and its a working solution).
The number of settings links on this page should be restricted to 1 but this can be done in bugfixing / follow-up patches.

patch has a lot of unrelated local changes from the other patches. Try applying your .info patch to a clean checkout (e.g. do cvs diff | patch -p0 -R).

@pwolanin:

patch has a lot of unrelated local changes from the other patches.

No, it doesn't. Its a reroll of the "old" patches, not a patch that puts the link into .info!

I don't like the .info-solution so there will be no patch from me implementing this.

Stefan

I'd like to get some review from other people on my proposed solution (thats *not* writing the link to the .info file but using MENU_SETTINGS_DEFAULT

update.module has changed so this patch needs a reroll.

Personally, I'm _not_ a fan of storing this in .info files either. The .info files should not become a settings file, especially if no one is supposed to change these settings.

@Dries - we gave a hook to alter the settings in .info files. A link to the settings should be stable fir any modules, so I don't understand your comment as to why this is a bad solution.

Putting this in the router get a strong "no" from me.

It's pretty much a toss-up, but conceptually, I'd say .info files contain clean meta-info that would be useful to aggregate before installing the module - the version number, the dependencies, the description of what it does.
The exact path of the settings URL is an implementation detail that is not relevant until after the module is installed. It could be manipulated by code even. So I'd expect to see this in the _menu hook.
Help pages are done through code, and I don't think we'd expect to see the link to the help pages in the info .

The modules page is built using the .info file, so to me this seems pretty natural and an easy implementation. The .info file also contains version info, etc, that's used for update status, so it's not only useful before you enable the module.

This patch is working, but does not yet have all the paths added to .info files. Setting to needs review for testbot, but it's not rtbc for at least that reason.

Repackaged the icon set from attachments above also. The css is the same as the last patch above - not my realm so I just used it.

test bot

Worked on this with pwolanin in IRC.

- Works as advertised. yay.

-order should be settings, permissions, help
- use a table cell for each individual icon (= consisten with how we show operations links)

There's some decisions to made for the paths to be used for some links. Can we have a list to work from? Enable all core modules and make notes?

Thanks for this.

I put in reasonable paths for all modules I think. Reworked the ordering and stuffed the icons inside TD elements. Tweaked css a little, though someone else may have some other ideas.

icons are the same as above

Sorry, the .info file is not the place to store meta-information about links. It's limiting and prone for errors. For example, we should not display the settings link if the user does not have access to the settings page -- that would be inconsistent with the rest of Drupal, and makes the modules page a lot less useful.

@Dries - Yoroy and I agreed that in fact we *must* display the link to the settings page even if they user does not have access to it. The most common wtf after installing a module is not setting the permissions so you can't access the settings page. If you have the link and it's a 403, that's a very useful clue that you need to configure the permissions. The rare case where a user has power to enable a module but not administer permissions this would be a clue that they need to talk to the next guy up the chain to configure the module.

I don't see why putting it here is any more limiting or error prone than putting it in a hook. Discussed with Yoroy that the goal is to expose one and only one link per module at most, so for that use, the implementation is totally sufficient. I can think of other implementations - e.g. a set of menu links and the module should save a link to that menu when it's installed - but that seems far worse in terms of DX for no real gain compared to this. We have a alter hook for .info data, and hook_form_alter, which are 2 distinct opportunities for altering what's on this page.

on principle, the router should hold data relevant for serving a path, or checking access to a path. This would be neither of those, and so I would consider it a violation of the router's design.

In addition, that approach would seem to require a variety of hacks to try to make a one-to-one connection between a module and this flag on one or more router items.

Also, this method adds zero overhead to the operation, since we already get the .info data - we don't need to call another hook or anything. If for some reason this approach isn't satisfactory, I'd then think the remaining option is a hook instead of violating the router.

I haven't reviewed all the comments (nor the code) here yet, just the comments related to where the links should be defined.

I find myself agreeing with Dries in #79. The point about access denied in #81 is very valid in Drupal 6 and below, but Drupal 7 ships with an administrator role, so the chances of this situation occurring in Drupal 7 is *drastically* reduced. I don't think it's worth adding inconsistency to the way links are handled in Drupal for this one edge case.

Yoroy and I agreed that in fact we *must* display the link to the settings page even if they user does not have access to it. The most common wtf after installing a module is not setting the permissions so you can't access the settings apge.

There are a lot of places where we can introduce this discovery pattern then. I don't see why the modules page is fundamentally different from any other administration page where things are protected by a permission.

I think it's fundamentally different because you can only see this page if you have can administer the site configuration. Does the administrator role automatically get all new permissions? It would seem not from my local install, in which case I stand by my position.

honestly, I don't care much about this issue - I just rolled the patch to demonstrate an implementation that is (imho) less broken, and since this seemed to be a significant usability issue. Feel free to rip out the settings path part completely and just keep the added link to permissions.

It would be easy enough to check the access for each link (e.g. call http://api.drupal.org/api/function/menu_get_item/7 and check $item['access']), and then if there is no access, the settings link could send them to another page with instructions and even a link to the right module's permissions using a little GET param.

I think this would be an important and useful improvement. Is this issue going to happen?

A link to configuration, in the context of module enabling page would be really useful, and it would give users easy access to module configuration. Currently the link to 'administration by module page' is the one link to show people where to configure each module (in intro text).

I am getting more and more concerned about the help text available for each module, and the intro text throughout. We've been cutting down on UI text, and we could meet the user half-way by introducing easier access to common needs: configuration per module being a specific case.

Looking at the Forum, #613272: Forums not useable out-of-the-box: disabled comments, no forums for example, it's not entirely obvious where to go after you enable the module. And a user can easily create an error by adding a topic without setting the forum/container.

Here's an old but related issue: #89205: Usability: Better message on module activation/deativation

@eigentor or @heather - feel free to re-roll with access checking + directing to a help page using the .info, or a hook, or something else. I see hacking this into the router as simply unacceptable from an architecture standpoint.

re: #79, 81, about showing the permissions link: Thought about it some more and I'm changing my position on that a bit:

It should not actually be a link. Willfully linking to something a user doesn't have access to is not so nice. But we don't need to actually link to show that there might be a permissions issue. Can we show a 'dimmed' (needs design) state of the permissions icon that communicates that there are actual permissions for this? We want to show a hint of permissions being part of the mix here but not actually link to them if you can't access them.

@yoroy - the gray is pretty dimmed already... but certainly something like this is possible. Maybe make the icons blue again, or a a little bolder? or make the disabled ones a little yellowish?

It may be a bit easier to style and have a fall back if the operation tasks become a list with a custom bullet per list item (css class). In such a case module are also able to define more than one settings link and nobody need to hover the icons first to figure out what they mean. This may be better for accessibility.

Another solution might be to get rid of the labels, and to just go with the icons. Just icons could work in this case, IMHO.

Using icon links on a page like this seems inconsistent with the rest of the admin pages in core. Icons may very well be a good idea, but in that case we should use them on similar pages too, e.g. admin/structure/taxonomy, admin/structure/taxonomy.

@eigentor This is a UX patch and it can still make it into D7. We won't use the .info files though.

@Dries - the earlier patch trying to mis-use the menu system is a "won't fix" - so if you are calling the current patch a "won't fix", then someone needs to roll a patch using a hook, or we should just close this issue or push to 8.x

Hm, I should rename my account to "someone" :-)
Here is a new try which introduces hook_settings_link (don't nail me for the name, I didn't found a better one).

To test use one of the icons attached in several comments above.

Hm, there was a strange character in block.module.
Fixed this.

And especially for eigentor a version with a column for each operation.

I think this is actually a good line already, help - permissions - settings. We should optimize for useage, settings on the right is useage.

@pwolanin would love your feedback on this issue

+++ modules/system/system.admin.inc	7 Nov 2009 19:25:22 -0000
@@ -718,13 +718,24 @@ function system_modules($form, $form_sta
         // Module has a help page.
-        $extra['help'] = theme('more_help_link', array('url' => url("admin/help/$filename")));
+        $extra['links']['help'] = l('<span class="element-invisible">' . t('More help') . '</span>', "admin/help/$filename", array('attributes' => array('class' => 'module-help-link', 'title' => t('More help')), 'html' => TRUE));

This is definitely a no-go:

• This is HTML generated outside a theme function.
• "element-invisible" is not a semantic class. The choice to hide or display the content of the <span> will be made by the theme itself. "element-invisible" is not an intrinsic property of the text.

This review is powered by Dreditor.

By the way, I actually happen to like the idea of using the menu *links* for displaying those (not the menu *router*, as in previous implementations). This ad-hoc hook approach makes me run screaming.

It would be great if one of the persons running away would stop, sit down for a while and come back with a working solution.

Subscribing.

@Damien - I'd be ahppy to use menu links if we can think up some reasonable DX for getting the links into the table.

e.g. we could have a menu jsut for these links, and then each module would have to save its links on hook enable (or hook install)?

Note that the menu_links table has a module column already

-1 for introducing a hook that contains the same values that are already specified elsewhere.

Until we split hook_menu() into hook_menu_router() and hook_menu_link() in D8, I see less of a problem of adding a property to hook_menu().

1) This property, we do not save in {menu_router} or {menu_links}. It's just a single flag per module.

2) Upon rebuild of menu router, we iterate over all items anyway. While doing so, we grab 'path', 'title', and 'description' from all items that have the flag, keyed by module. This list, we store as variable.

3) On admin/structure/module, we simply grab this list to display links to settings pages.

Alternatively to 2), we could store even less in the stored variable, namely $module => $path. On admin/structure/module, we simply query the corresponding links.

@sun - store as a variable? if you want to go this route (which I still don't like much), we should just save an extra menu link for each as DamZ suggests in a menu jsut for this purpose?

Hm. Why do we want to duplicate paths/links? Whatever additional data we store, we also need to maintain. So this would mean to truncate all these duplicated links in that custom/hidden menu when we rebuild the router, and re-fill it afterwards. I still do not see why we would want to duplicate any data. All we need is a 1:1 relationship from $module to $settings_path.

Attached patch may provide some clues.

@sun - if that's all we want... then the .info approach should be good enough.

Alrighty, this would be the fully working implementation.

And of course, we want to display the description as link title.

I like how nice and tiny this is, one tiny weirdness though: the Configure settings link of the Color module points to /admin/appearance/settings, but the global settings page doesn't show any color settings, only themes that implement it show it.

I don't suppose we could make that link to the active theme's settings page if it implements Color stuff and otherwise just don't show it?

oh and this shows that the menu entry for dashboard's settings page doesn't have a description, which causes it's Configure settings link to have an empty title, I guess menu entry description should be solved somewhere else, but perhaps drop the title attribute when there is no description for the menu entry?

Isn't Dries against using .info files for this task ? Maybe he changed his mind, but I think he has a pretty good point in #70 too.

I've read through the comments for the first time now.

I do not understand the objections of putting this information into the .info file. We need a 1:1 relationship between "module" (which has no other entity or thing we could attach info to) and "settings path". The settings path is a router path, not a menu link. And the settings path is not alterable. You cannot say "oh, but I'd like this module's settings rather there", because "there" you won't find the module's settings.

The .info files should not become a settings file, especially if no one is supposed to change these settings.

The implementation is not a setting, but meta data - a pointer to a module's settings path. Just like the "dependencies" is a pointer to the module's dependencies on other modules. No one is supposed to change these -- although you could even alter those via http://api.drupal.org/api/function/hook_system_info_alter/7

There is only one settings path per module, and we want to use the link description from the already registered menu item for the settings path.

1) Using a property in hook_menu() would mean that there could be more than one settings path. No option.

2) Using a new hook would mean that we would duplicate the information from the existing menu item that's registered in hook_menu(). Doesn't make sense.

3) We need a 1:1 relationship between module and module settings path, and we also need to check whether the current user has access to administer the module's settings. Hence, we need to check the menu anyway.

@sun - yes, I'd agree that this is not a "setting" but rather meta data, which is why I suggested the .info file.

So what's holding this up from RTBC?

I'd say that if Dries changed his mind, it shouldn't be a problem, but we'll have to get his opinion about it as he clearly disagreed on doing it this way (I forgot to mention another one #79 : Sorry, the .info file is not the place to store meta-information about links. Note: I highlighted the important part in the quote).

There are few things that changed between stBorchert's last patch and yours :

+++ modules/filter/filter.info	9 Nov 2009 19:04:06 -0000
@@ -1,4 +1,4 @@
-;h $Id: filter.info,v 1.12 2009/06/08 09:23:52 dries Exp $
+; $Id: filter.info,v 1.12 2009/06/08 09:23:52 dries Exp $

You're messing with the $Id of filter.info.

+++ modules/system/system.admin.inc	9 Nov 2009 19:47:10 -0000
@@ -721,10 +721,36 @@ function system_modules($form, $form_sta
+          '#type' => 'link',
+          '#title' => t('More help'),
+          '#href' => "admin/help/$filename",
+          '#options' => array('attributes' => array('class' => 'module-link module-link-help', 'title' => t('More help'))),
...
+        '#type' => 'link',
+        '#title' => t('Configure permissions'),
+        '#href' => 'admin/config/people/permissions',
+        '#options' => array('fragment' => 'module-' . $filename, 'attributes' => array('class' => 'module-link module-link-permissions', 'title' => t('Configure permissions'))),
...
+          '#type' => 'link',
+          '#title' => t('Configure settings'),
+          '#href' => $settings_link['href'],
+          '#options' => array('attributes' => array('class' => 'module-link module-link-settings', 'title' => $settings_link['description'])),

• The CSS class names are not right and thus the icons do not display for permissions and settings.
• I think the class names should be passed as an array, but I am not sure as I do not know the exact implementation of '#type' => 'link', but chances are it passes through drupal_attributes which handles class names as array now.
• The "#title" of the link should be invisible (what a wierd name by the way... it is the "text" of the anchor, not the "title"... anyways, another mystery about '#type' => 'link' ;)). See #6 and #7 and #106 which somewhat implies that we should create a specific CSS class with a semantic name for the <span> used to "hide" the text. It should reuse the properties of "element-invisible", so it stays accessible to the screen-readers.

+++ modules/system/system.admin.inc	9 Nov 2009 19:47:10 -0000
@@ -2323,6 +2343,10 @@ function theme_system_modules_fieldset($
+    // Display links (such as help or permissions) in their own columns.
+    foreach (array('help', 'permissions', 'settings') as $key) {
+      $row[] = array('data' => drupal_render($module['links'][$key]), 'class' => array($key));
+    }

I would strongly suggest that we use an unordered list to display the links as it is a list of administrative links. stBorchert did few versions like that, but honnestly this can always be settled later. The inconvenient with an unordered list is that it would be way more difficult to have an emty space when one of the links is not displayed. But at least it would be semantic which in my opinion is more important than the eye candy.

And last and optional, if we have time, maybe we should also add a test case so it checks that the link is present when modules are enabled and not present when they are disabled ? But I suspect you'll call that test-bikeshedding ;)

I attached a screenshot to show how the page looks with the patch in #117, icons are present but as mentioned above they do not display because of wrong CSS class names.

This review is powered by Dreditor.

You're messing with the $Id of filter.info.

Nope, that doesn't belong to the CVS Id keyword and is simply a bad character that somehow sneaked in there.

The CSS class names are not right and thus the icons do not display for permissions and settings. I think the class names should be passed as an array...

Right. I forgot to update the class names, and the value for 'class' should always be an array now. Thanks for catching that!

The "#title" of the link should be invisible

As eigentor mentioned, we can defer the question whether to use icons or not here to a separate issue. In case that happens, then the link titles will be hidden via CSS and at the same time, the links will be turned into icons, based on the already existing class names. That belongs into the 1x1 toolbox of themers.

I would strongly suggest that we use an unordered list to display the links

Nope, because that would result in a totally unreadable output. The table cells form a grid that can be recognized and understood very quickly.

I still don't like storing this in the .info file (where does it end?), but I think it is more important to get this patch in than anything else.

I think 'settings' should be renamed 'configure'. If I click the settings link for the dashboard module, it doesn't give me any settings. It let's me configure the dashboard. The same is true for many other settings links.

Nope, that doesn't belong to the CVS Id keyword and is simply a bad character that somehow sneaked in there.

I was only trying to locate the precise line which contained the error, I did not imply that you modified it on purpose ;)

Ok then so be it for the list of operations. The grid is indeed more clear, unfortunately it is less semantic but I think we can live with that ;)

I agree that "Configure" is more appropriate and clear than "Settings" in certain cases and in the other cases "Configure" makes as much sense as "Settings". Also "Configure" is used many times in the core for this type of operation. For consistency, I also changed the name of the variable in the module .info. I was not sure if it was better to call it "configuration" or "configure", I went for the latter.

+++ modules/color/color.info	15 Nov 2009 09:54:32 -0000
@@ -8,3 +7,4 @@ core = 7.x
+configure = admin/appearance/settings

It was mentioned before that we want to remove this one.

+++ modules/system/system.admin.inc	15 Nov 2009 10:01:10 -0000
@@ -720,10 +720,37 @@ function system_modules($form, $form_sta
+          '#options' => array('attributes' => array('class' => 'module-link module-help-link', 'title' => t('Help'))),
...
+        '#options' => array('fragment' => 'module-' . $filename, 'attributes' => array('class' => 'module-link module-permissions-link', 'title' => t('Configure permissions'))),
...
+          '#options' => array('attributes' => array('class' => 'module-link module-configure-link', 'title' => $configure_link['description'])),
+++ modules/system/system.css	15 Nov 2009 09:59:52 -0000
@@ -159,10 +159,22 @@ tr.merge-up, tr.merge-up td, tr.merge-up
+.more-help-link a, a.module-help-link {
...
+a.module-permissions-link {
...
+a.module-configure-link {

1) We still need to turn the value of 'class' into an array.

2) Those CSS class names should be prefixed with 'module-link', plus suffix help|permissions|configure.

I'm on crack. Are you, too?

I assumed the things I reviewed were fixed ;) My bad. Anyways I would have left also the color.info changes so it doesn't really matter... Here's a re-roll with all the suggested changes by sun in his previous message.

I understand that we're deferring on the use of icons only, which I'm ok with. Here's one more screenshot of what it looks like with this patch.
UXteam says RTBC, maybe one more 'yay' for the code?

Well this looks pretty effing awesome, other than that .info file thing gives me the heebie jeebies. But it sounds like this has Dries's ok. I'll leave for him to commit, and go after it tomorrow night if it's still hanging around.

I do like the usability of this. Really want this to get into core! At the moment though I'm not sure how a keyboard only user can get to the Help, Permissions & Configure links.

I like that they are there, but tabbing through the site just jumps between checkboxes.

As with this issue here - #467296: Accessibility improvements for vertical tabs - we need to have way to add more navigation options for keyboard only users.

Alright, committed to CVS HEAD. Thanks.

I love you. :)

Sniff. I was not vocal enough that we needed admin/by-module/$modulename and link there and nowhere else and then we can have permissions, configure and any number of links there. The advantages are numerous a) we already have this code, just needed to move the section to its own page b) noone needs to learn another way of defining a path.

I thought Dries shut this patch down for good in http://drupal.org/node/598758#comment-2161952 and http://drupal.org/node/598758#comment-2238428 and by the time I look again, it's in. Oh well. I know. Rules have changed and I was told plain that these UX things are not for me and I should shut up. I still won't.

Example. Some module has a generic screen for the generic settings but also it has specific settings for each node type. What now? One link is not enough. It's only in this crazy mindset that we call D7UX that certain things are "enough", that certain workflows happen and nothing else.

chx: I also raised the posts you're talking about (not because I don't like this solution, just because it was raised before). Dries' answer and acknoledgment is in http://drupal.org/node/598758#comment-2267146

Maybe I'm a bit tired now, but I don't quite get the concept of "admin/by-module/$modulename", could you explain it (again?) please ?

I can't understand how you can even think of promoting awkward code like http://api.drupal.org/api/function/system_get_module_admin_tasks/7 ...

Excellent. One, if not the, most important ux improvements to go in.

Move one section to one page. What I attached is admin/by-modules/forum. Easy to code, easy to link...

@chx It has nothing to do with D7UX, there is simply no great solution to this problem yet. I think this is a good solid direction, and we should continue to work on it - if you see opportunities to make it better do so. But I don't think that the "there are several setting pages with equal importance" should be given priority over the massive usability issue of not finding any module its settings. Its an edge case that is hard to tackle, although your proposal does - it also exposes another problem, of creating a layered IA - where this page becomes a jumping board (which it wasn't supposed to be).

It should be possible however, to implement your solution to these edge cases - it should actually be just a matter of a link? We can just create a link to a per-module block, on the settings icon for those modules who have several settings?

Anyway, really glad we got this in - as hard as it may seem so late in the process, we need to be aware that this solves a highly critical issue.

Just discussed this with Bojhan and yoroy in irc. I think what chx is suggesting is something like this - added my own embellishments

1. admin/by-module doesn't generate any pages for modules, only blocks but it could, at admin/by-module/$module - and system.module could build these magically on behalf of all modules.

2. We can leave the settings link/icon on the modules page exactly how it is, and default it to linking to admin/by-module/$module

3. If admin/by-module/$module only contains one link - we can redirect to that the same as we do for node/add when it only contains one content type - yoroy and Bojhan were both concerned about an additional click step to a landing page with one link on it, which would suck.

4. As well as all the above, we can otherwise leave things how they are in HEAD, so that module authors can specify a single page which they want linked from here. However you'd get 2/3 for free if you don't do anything.

I retested what went into core in #134. Tabbing is working fine to get to the additional links. Not sure what changed or if perhaps it was a caching issue in my previous test.

I've just looked at this page: /admin/config/modules#main-content

It would still be useful to have better keyboard navigational tools as there are now a bunch more links required to get down the page to enable/disable modules. However, think that's bigger than this issue at the moment.

I don't see any other accessibility challenges with this issue at this time.

Thanks for verifying, mgifford.

chx/catch's plan at #143 makes sense to me, as well. I like the API change part of this being optional.

Which, btw, needs to be documented in the module upgrade guide under UNSTABLE-11. Ugh.

Still the API change must be rolled back, there is nothing in the plan that justifies another way of defining links. Yes, I can live with admin/by-modules/$modulename simply redirecting if there is only one link.

To summarize for the UX folks, here is what's being proposed:

1. We remove the ability to set a single "config" link in your .info file. Not only is it extremely odd to have to define admin paths in two places, for some modules (such as Panels, Project, etc.) a single admin screen is often woefully insufficient. (No impact on UI)

2. The admin/config/modules screen stays the same; you still get help, permissions, and configure links next to any module that has them. The difference is that "Configure" instead of pointing to admin/config/blah/dee/blah now points to admin/by-module/MODULE:

3. When you click "Configure" one of two things will happen:

a. In the case of a module such as Filter module, that only defines a single configuration page (admin/config/content/formats), you will be taken directly to that configuration page:

b. In the case of a module such as Search module, which defines multiple configuration pages, you'll be taken to a page that shows a list of links like this:

(Note to implementors: This is different than admin/by-module in that we remove the help and permissions links, and put the name of the module in the help text at the top.)

Let's see what Bojhan, yoroy et al think. I could argue either way on this. I really like that it cleans up the implementation, but OTOH from a user perspective it might be really jarring to get two wildly different looking types of pages when you click the same link on different modules.

so... more HTTP requests, less content, more empty pages?

but I guess it could solve the problem with color...

I posted one patch and will write another (for the configuration solution above) #638912: Point the permissions link to the module

Just as a reminder, the CSS code currently in CVS calls for a "misc/permissions.png" and "misc/configure.png", both of which are missing and still need to be added if this patch is going to stay in.

Thanks for the reminder. I committed those images, as well as the grey help.png that was in #130.

Just going through old issues here. Does the documentation still needs to be done still.

After doing a quick review with http://wave.webaim.org/toolbar

The only thing that jumped out was the same issue here #49428: Include node title in "Read more" link for better accessibility and SEO - which is a WCAG 2.0 A level issue

2.4.4 Link Purpose (In Context): The purpose of each link can be determined from the link text alone or from the link text together with its programmatically determined link context, except where the purpose of the link would be ambiguous to users in general. (Level A)

Help Permissions Configure are useful, but it would be more useful if they were tied to a module name. This can be done much has has been suggested with the issue noted above.

I am curious what the current status of this issue is, what needs to be done for it to be resolved.

@mgifford

I haven't tested, but I expect that the purpose of the links is determinable by context (table row).

The link is nicely seated in the table row, so it can be identified that way which does seem to fit inside of the framework "the link text together with its programmatically determined link context". So looks like this is addressed from an accessibility standpoint.

I think this is fixed. If there are still follow-up issues we can deal with them in follow-ups.