And replace instead with a general message at the top basically saying, "Check the administer >> help pages for more details on how to use specific modules." This message can probably actually go on the top of both tabs in this section.

Reasons why:
- Getting help is not an administration task.
- Users with only "access help" permissions and one or two others will see lots of blocks with only a single "get help" link in them, which is confusing.

This comes from hunmonk's feedback over @ Administration page block lists all items ignoring permissions.

CommentFileSizeAuthor
#20 admin_by_module_0.patch1.23 KBwebchick
#17 admin_by_module.patch1.06 KBhunmonk
#15 get_help.patch2.29 KBhunmonk
#13 Picture 11.png61.34 KBwebchick
#12 get-help_1.patch3.32 KBwebchick
#11 get-help_0.patch2.76 KBhunmonk
#8 get-help.patch2.55 KBwebchick
#3 admin_help_0.patch1.96 KBwebchick
#1 admin_help.patch1.73 KBhunmonk

Comments

hunmonk’s picture

hunmonk commented
Assigned: Unassigned » hunmonk
Status: Active » Needs review
StatusFileSize
new admin_help.patch1.73 KB

i think this will do it.

hunmonk’s picture

hunmonk commented

note that since we bag the help links per module, we can now choose to not display any sections that have no links for the current user. the whole patch is tested as working.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
StatusFileSize
new admin_help_0.patch1.96 KB

Works as advertised! "get help" links are gone from all of the module blocks, and enabling the Tracker module, which only contains a "get help" link, causes the module's block not to appear.

The t() stuff though looks like it needs some work...
a) you want to avoid HTML in the string where you can, so the p tags should be outside the t()
b) Translators need as much context as possible about the strings they're translating; therefore, the link and its title should actually go in the same t() string as the rest of the text that's being translated (the exception to rule a) :)).

This patch attempts to fix both of these issues in the help text that gets added by this patch. Looks like the whole system_help function (and likely all of them) is totally inconsistent and could use a review for this kind of thing, but that's for another patch, imo.

hunmonk’s picture

hunmonk commented
Status: Needs review » Reviewed & tested by the community

let's be bold... :)

drumm’s picture

drumm
he/him
Location NY, US
commented

I think the contextual help links might be important. The links are more noticeable and targeted.

However, they are not the same as other items in the list. What about moving the help link at the end of the module description line?

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Reviewed & tested by the community » Needs work
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

@drumm I like that idea.

Ok. So the new plan is ;)

1. Only show blocks when there are > 1 thing admin task
2. Move the "get help" links to the title of the block

There's still the problem of users never knowing that tracker module has help, but I guess that can't really be avoided.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
StatusFileSize
new get-help.patch2.55 KB

Let's try this.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Needs work » Needs review

Oops. Needs review.

drumm’s picture

drumm
he/him
Location NY, US
commented
Version: x.y.z » 5.x-dev
Status: Needs review » Needs work

Looks better, but wow those links are huge. What about after each description or in the top-right corner of each block?

hunmonk’s picture

hunmonk commented
Status: Needs work » Needs review
StatusFileSize
new get-help_0.patch2.76 KB

ok, i tried the help links after the description, and they looked really bad. here's a patch with them floated to the right of the title block, which i think is the best we've come up with so far. hopefully i did the css right--i'm pretty awful at it... ;)

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
StatusFileSize
new get-help_1.patch3.32 KB

Let's try this... essentially exactly the same as hunmonk's except:

  • Added get help link to the description, rather than the title so it's not so

    HUGE

  • Named the class 'get-help' rather than 'theme-get-help' since we don't seem to prefix any other classes with 'theme-'.
  • Used a call to l() rather than embedding the anchor directly.
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
StatusFileSize
new Picture 11.png61.34 KB

Screenshot.

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Needs review » Needs work

> getpatch.sh http://drupal.org/files/issues/get-help_1.patch | patch -p0
patching file modules/system/admin.css
patching file modules/system/system.module
Hunk #1 FAILED at 33.
Hunk #2 FAILED at 2245.
Hunk #3 succeeded at 2267 (offset -1 lines).
2 out of 3 hunks FAILED -- saving rejects to file modules/system/system.module.rej

hunmonk’s picture

hunmonk commented
Status: Needs work » Needs review
StatusFileSize
new get_help.patch2.29 KB

blasted t() changes... ;) re-rolled against latest HEAD, and tested as working. no functional changes.

dries’s picture

dries commented
Status: Needs review » Needs work

-1. The current UI looks better, IMO. These small, floating 'get help' links are really awkward.

hunmonk’s picture

hunmonk commented
StatusFileSize
new admin_by_module.patch1.06 KB

i think we're getting a little off track here. the main point of the initial patch was to _not include_ module sections which a user had no rights to access (ie, there were no valid links for that particular module for that particular user). believe me, if a user has only three modules that they have valid access rights to on this page, it's a real UI mess to include all those other module blocks with only a 'help' link.

can we please revisit the original idea of this patch? attached patch uses the current display, buy only includes module blocks where the user has _at least_ one admin priv. besides the help link.

hunmonk’s picture

hunmonk commented
Status: Needs work » Needs review

note that this approach mirrors the way block display works on the 'By task' page, so this will make the UI more consistent overall as well

hunmonk’s picture

hunmonk commented
Title: Remove 'get help' links from 'admin by module' page » By module admin page should not display 'empty' blocks

we need a more accurate title for what this patch is trying to accomplish.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new admin_by_module_0.patch1.23 KB

Created user "peon" and gave authenticated users "access administration pages" permissions.

Confirmed that before the patch, I saw blocks for every single module @ admin/by-module, even though the only module I should have access to is watchdog.

After applying patch, confirmed that the watchdog block was the only one to appear, with the get help link in-tact.

In other words, "works as advertised." Marking RTBC again.

Patch is the same, just fixes a single space that was added by mistake.

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Reviewed & tested by the community » Fixed

Committed to HEAD.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)