Support from Acquia helps fund testing for Drupal Acquia logo

Comments

batigolix’s picture

Title: Change link format in hook_help in Help module » Update hook_help in Help module
Component: documentation » help.module
Status: Active » Needs review
FileSize
6.46 KB

Patch:

- changes url() to \Drupal::url()
- changes @token to !tokens
- changes http:\\drupal.org to https:\\drupal.org

Unfortunately the text is outdated. Among others it needs updated wording for referring to line docs and drupal UI . It also refers users to the Drupal forums as starting point to get help. This is nowadays one of the worst places to find help ...

Needs plenty of work

Status: Needs review » Needs work

The last submitted patch, update-hook-help-help-2091335-1.patch, failed testing.

lostkangaroo’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, update-hook-help-help-2091335-1.patch, failed testing.

neclimdul’s picture

HelpTest as the following code:

    $this->assertRaw(t('For more information, refer to the specific topics listed in the next section or to the <a href="@drupal">online Drupal handbooks</a>.', array('@drupal' => 'http://drupal.org/documentation')), 'Help intro text correctly appears.');

Should probably be this to match the changes in the patch:

    $this->assertRaw(t('For more information, refer to the specific topics listed in the next section or to the <a href="!drupal">online Drupal handbooks</a>.', array('!drupal' => 'http://drupal.org/documentation')), 'Help intro text correctly appears.');
batigolix’s picture

Status: Needs work » Needs review
FileSize
7.68 KB

Patch:

- changes testHelp() so that it matches hook_help text

batigolix’s picture

Note to self: this needs to pass by UI review, because it contains very outdated references to pre D7 user interface

batigolix’s picture

Issue tags: +Usability

tagging

lostkangaroo’s picture

We should look at possibly merging this text with this issue #1387964: People miss the first section of the help overview page or closing either one as a duplicate

batigolix’s picture

#1387964: People miss the first section of the help overview page only introduced a new header and did not touch the body of the help text. i think we can safely continue here.

Attached patch rephrases the sentences that refers to handbooks and forums

I removed the reference to the forum alltogether.

Status: Needs review » Needs work

The last submitted patch, update-hook-help-help-2091335-10.patch, failed testing.

batigolix’s picture

new attempt

batigolix’s picture

Status: Needs work » Needs review

setting status

yingtho’s picture

The patch is missing "module_exists('node')". This is now included.

Status: Needs review » Needs work

The last submitted patch, update-hook-help-help-2091335-14.patch, failed testing.

yingtho’s picture

Status: Needs work » Needs review
FileSize
7.69 KB

Rerolled the patch from the latest from git.

batigolix’s picture

The links work. The text is the same as for Drupal 7 except for the 2 changes described in #10.

For me this is okay

jhodgdon’s picture

Status: Needs review » Needs work

Thanks!

I think this still needs a bit of work:

a) In the first section of the patch, can we replace "Next, visit the module list" with the actual name of the page, which is now "Extend" I think? Also "themes section" is actually the "Appearance page".

b) "and enable features which suit your specific needs." which -> that

c) all drupal.org links should be https

d) "View also the other support options that are available." This is not good English wording. Very awkward.

e) " the online Drupal handbooks. The handbooks" ... We do not call the on-line documentation "handbook" any more. Please change this to "documentation" or "online documentation"

f) "see the online handbook entry for the Help module.'" This also needs to be updated to fit our current template.

cosmicdreams’s picture

Whitespace.

+++ b/core/modules/help/lib/Drupal/help/Tests/HelpTest.php
@@ -70,8 +70,8 @@ public function testHelp() {
+    ¶
lostkangaroo’s picture

Status: Needs work » Needs review
FileSize
7.83 KB

I wasn't able to get #16 to apply so no interdiff sadly, but here is a summary of changes.

Text in this patch slightly formated to mimic site appearance with seven:

About

The Help module provides Help reference pages and context-sensitive advice to guide you through the use and configuration of modules. It is a starting point for Drupal.org online documentation pages that contain more extensive and up-to-date information, are annotated with user-contributed comments, and serve as the definitive reference point for all Drupal documentation. For more information, see the online documentation for the Help module.

Uses

Providing a help reference
   The Help module displays explanations for using each module listed on the main Help reference page.
Providing context-sensitive help
   The Help module displays context-sensitive advice and explanations on various pages.

Getting Started

Follow these steps to set up and start using your website:

  1. Configure your website Once logged in, visit the Administration page, where you may customize and configure all aspects of your website.
  2. Enable additional functionality Next, visit the Extend page and enable modules which suit your specific needs. You can find additional modules at the Drupal.org modules page.
  3. Customize your website design To change the "look and feel" of your website, visit the Appearance page. You may choose from one of the included themes or download additional themes from the Drupal.org themes page.
  4. Start posting content Finally, you may add new content to your website.

For more information, refer to the subjects listed in the Help Topics section or to the online documentation and support pages on drupal.org.

  1. Modified text per #18
  2. Combined the second and third sentences to combine like ideas and easier flow.
  3. Modified test to look for new text "For more information, refer to the subjects listed in the Help Topics section or to the online documentation and support pages on drupal.org."
  4. Removed Whitespace
jhodgdon’s picture

Status: Needs review » Needs work

Thanks, better!

A few things still need fixing:

a) See #18 (b) - you need to change which -> that in one sentence.
b)

+      $output .= '<p>' . t('For more information, refer to the subjects listed in the Help Topics section or to the <a href="!docs">online documentation</a> and <a href="!support">support</a> pages on <a href="!drupal">drupal.org</a>.', array('!help' => \Drupal::url('help.main'), '!docs' => 'https://drupal.org/documentation', '!support' => 'https://drupal.org/support', '!drupal' => 'https://drupal.org')) . '</p>';

You do not need the !help in the substitutions array. It is not present in the text.

Or in this line from the test:

+    $this->assertRaw(t('For more information, refer to the subjects listed in the Help Topics section or to the <a href="!docs">online documentation</a> and <a href="!support">support</a> pages on <a href="!drupal">drupal.org</a>.', array('!help' => \Drupal::url('help.main'), '!docs' => 'https://drupal.org/documentation', '!support' => 'https://drupal.org/support', '!drupal' => 'https://drupal.org')), 'Help intro text correctly appears.');
 

Other than those small changes, I think this looks good!

lostkangaroo’s picture

Status: Needs work » Needs review
Issue tags: -Usability
FileSize
4.7 KB
7.76 KB

Those must have creapt back in when I was swapping patches in and out to get #16 working for the missing interdiff. Changes from #21 added.

Status: Needs review » Needs work

The last submitted patch, drupal8.help-module.2091335-22.patch, failed testing.

jhodgdon’s picture

That's a legitimate test failure - apparently the text in the test does not match what is on the screen exactly.

lostkangaroo’s picture

Assigned: Unassigned » lostkangaroo

I saw that also. I am looking into it now.

lostkangaroo’s picture

Turns out "on" and "at" are not the same in string comparisons. The test should be fixed again.

lostkangaroo’s picture

Status: Needs work » Needs review
lostkangaroo’s picture

Assigned: lostkangaroo » Unassigned
jhodgdon’s picture

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

Assuming the tests pass this time, this seems like it is good to go! I'll move the component back to documentation at this point, so that I notice it when I'm next doing commits. I don't think there is anything the help.module maintainers particularly will need to comment on and/or change.

Thanks!

jhodgdon’s picture

Status: Reviewed & tested by the community » Fixed

Thanks again everyone! Committed to 8.x.

Status: Fixed » Closed (fixed)

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