Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Background:
This issue is part of the task to update the hook_help texts of the modules for Drupal 8:
#1908570: [meta] Update or create hook_help() texts for D8 core modules
Task:
The links in hook_help need to be changed.
See https://drupal.org/node/632280#url-note
Comment | File | Size | Author |
---|---|---|---|
#26 | drupal8.help-module.2091335-26.patch | 7.76 KB | lostkangaroo |
#26 | interdiff.txt | 1.52 KB | lostkangaroo |
#22 | drupal8.help-module.2091335-22.patch | 7.76 KB | lostkangaroo |
#22 | interdiff.txt | 4.7 KB | lostkangaroo |
#20 | drupal8.help-module.2091335-20.patch | 7.83 KB | lostkangaroo |
Comments
Comment #1
batigolixPatch:
- 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
Comment #3
lostkangaroo CreditAttribution: lostkangaroo commented#1: update-hook-help-help-2091335-1.patch queued for re-testing.
Comment #5
neclimdulHelpTest as the following code:
Should probably be this to match the changes in the patch:
Comment #6
batigolixPatch:
- changes testHelp() so that it matches hook_help text
Comment #7
batigolixNote to self: this needs to pass by UI review, because it contains very outdated references to pre D7 user interface
Comment #8
batigolixtagging
Comment #9
lostkangaroo CreditAttribution: lostkangaroo commentedWe 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
Comment #10
batigolix#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.
Comment #12
batigolixnew attempt
Comment #13
batigolixsetting status
Comment #14
yingtho CreditAttribution: yingtho commentedThe patch is missing "module_exists('node')". This is now included.
Comment #16
yingtho CreditAttribution: yingtho commentedRerolled the patch from the latest from git.
Comment #17
batigolixThe links work. The text is the same as for Drupal 7 except for the 2 changes described in #10.
For me this is okay
Comment #18
jhodgdonThanks!
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.
Comment #19
cosmicdreams CreditAttribution: cosmicdreams commentedWhitespace.
Comment #20
lostkangaroo CreditAttribution: lostkangaroo commentedI 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:
Comment #21
jhodgdonThanks, better!
A few things still need fixing:
a) See #18 (b) - you need to change which -> that in one sentence.
b)
You do not need the !help in the substitutions array. It is not present in the text.
Or in this line from the test:
Other than those small changes, I think this looks good!
Comment #22
lostkangaroo CreditAttribution: lostkangaroo commentedThose 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.
Comment #24
jhodgdonThat's a legitimate test failure - apparently the text in the test does not match what is on the screen exactly.
Comment #25
lostkangaroo CreditAttribution: lostkangaroo commentedI saw that also. I am looking into it now.
Comment #26
lostkangaroo CreditAttribution: lostkangaroo commentedTurns out "on" and "at" are not the same in string comparisons. The test should be fixed again.
Comment #27
lostkangaroo CreditAttribution: lostkangaroo commentedComment #28
lostkangaroo CreditAttribution: lostkangaroo commentedComment #29
jhodgdonAssuming 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!
Comment #30
jhodgdonThanks again everyone! Committed to 8.x.