This issue is part of the task to update the hook_help texts of the Drupal 8 modules:
#1908570: [meta] Update or create hook_help() texts for D8 core modules

- review / write the hook_help text according to help guidelines
- review online docs

#9 interdiff.txt1.47 KBmarkcarver
#9 2031639-9.patch3.5 KBmarkcarver
PASSED: [[SimpleTest]]: [MySQL] 59,343 pass(es).
[ View ]


batigolix’s picture

new2.94 KB

- changed link to online docs
- removed some "marketing" speak ("fast & easy")

I'm wondering if we can remove some text such as the warning:

This means that if you make any manual changes to your theme's stylesheet, <em>you must save your color settings again, even if they haven't changed</em>. This step is required because the module stylesheets (in the files directory) need to be recreated to include your changes.")

I'm unsure if such advice should be part of the hook_help or should be on line.

Any opinions?

jhodgdon’s picture

Status:Active» Needs work

I think the note about the files important enough to be in the hook_help. The hook_help should have all necessary information, because not everyone is necessarily connected to the Internet at all times.

By the way, batigolix -- didn't the standard for referring to the online documentation previously say "For more information, see the online documentation for the Foo module"? I'm wondering if when you changed it to have its own t(), you took out the "for the Foo module" piece? If so, let's put it back. The problem is that the "online documentation" link is not very good for accessibility purposes -- the link text does not describe where the link really goes (it is non-specific).

Regarding the rest of the help text, it looks pretty good... a few things I think should be fixed:
- Be sure to refer to "the Color module" not "the color module" throughout.
- We should verify that the permission names are still accurate, and that this is the permission the Color module actually checks.
- Normally when we have URLs referred to in hook_help() or in API documentation, we'd prefer that the page being linked to has a URL alias rather than just node/12345
- URLs on are now https not http

batigolix’s picture

I've given this issue a second thought.

This help text was already reviewed for Drupal 7. I'm sure we can find small things to improve.

But we can better concentrate on the modules that, at the moment, have no or poor hook_help texts.

A second worry is that small change in UI strings destroy existing translations. Especially the help texts are long strings that are hard to translate. If we fine tune all existing help texts we will make many translators (including me) unhappy.

Regarding the standard for referring to the online documentation: yes, it was my plan to have a general sentence for this, without mentioning the modules name. Given my doubts above, I think it is best to stick to the old standard. Ill revert the changes in

So, shall we postpone this issue?

batigolix’s picture

Status:Needs work» Postponed
markcarver’s picture

Component:documentation» color.module
Issue tags:+documentation

Tagging for my exposure

jhodgdon’s picture

Issue summary:View changes
Status:Postponed» Active

There is no reason to leave this as "postponed" at this point.

batigolix’s picture

Status:Active» Needs review
new3.15 KB
PASSED: [[SimpleTest]]: [MySQL] 59,282 pass(es).
[ View ]
new2.96 KB

Patch addresses the points in #2

I removed the troubleshooting reference

jhodgdon’s picture

Issue tags:+Needs manual testing

This looks fine to me.

I think it's ready for a manual test:
- Make sure all the links work
- Make sure any permissions, page names, and other text appearing in the user interface are the same in the help text as what the admin would actually see in Drupal
- Make sure formatting is OK.

Also, the color.module maintainers should respond here that they're happy with the accuracy of the help, hopefully.

markcarver’s picture

Issue tags:-Needs manual testing
new3.5 KB
PASSED: [[SimpleTest]]: [MySQL] 59,343 pass(es).
[ View ]
new1.47 KB

Manually tested. Looks good, color.module maintainer signs off.

- Links work, I made one very small change to one of the links though:
<a href="!appearance">Appearance page</a> to <a href="!appearance">Appearance</a> page
- Permission, page names and other text appear correct
- Formatting is ok (follows same formatting as other help pages).

Leaving as CNR so testbot picks up, RTBC when it comes back green.

jhodgdon’s picture

Component:color.module» documentation
Status:Needs review» Reviewed & tested by the community

Great, thanks! Moving back to Documentation component so I will get it committed next time I'm doing commits, unless somoene else gets to it before me.

jhodgdon’s picture

Status:Reviewed & tested by the community» Fixed
Issue tags:-documentation

Thanks again! Committed to 8.x.

Status:Fixed» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.