Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Background:
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
Tasks:
- review / write the hook_help text according to help guidelines
Comment | File | Size | Author |
---|---|---|---|
#19 | drupal8.documentation.2091349-18.patch | 1.7 KB | ifrik |
#19 | interdiff-2091349-16-18.txt | 1.67 KB | ifrik |
#16 | drupal8.documentation.2091349-16.patch | 1.7 KB | ifrik |
#16 | interdiff-2091349-13-16.txt | 4.65 KB | ifrik |
#13 | drupal8.documentation.2091349-13.patch | 1.65 KB | ifrik |
Comments
Comment #1
radimklaskaComment #2
radimklaskaDoc in this module is pretty straight forward.
Link to the d.o documentation was linking D7 version. So I created D8 "clone" with updated screenshots and menu item names here: https://drupal.org/node/2099449
Attaching patch with updated URL.
Comment #3
jhodgdonActually, we do want the link to go to drupal.org/documentation/modules/overlay. The page will may to be updated for Drupal 8, but we do want that to be the link.
See
https://drupal.org/node/632280#url-note
for URL format though.
Comment #4
BarisW CreditAttribution: BarisW commentedSo does this mean that this issue can be closed then?
Comment #5
radimklaskaThanks for your feedback. So right now the patch is not needed because there already is correct URL in the code.
Should I do something about the new doc page I created, or should I just wait until someone redirects the "nice" URL?
Comment #6
lostkangaroo CreditAttribution: lostkangaroo commentedThe patch is still needed but the string tokens use ! rather than @
Comment #7
radimklaskaComment #8
radimklaskaAttached patch changes just the token mentioned in #6 from @ to !
Comment #9
jhodgdonThe drupal.org link also needs to be https. Thanks!
And sorry for taking so long to get this patch reviewed... I am behind from all the great work done at Prague!
Comment #10
radimklaskaNp, thanks for review. :-) New patch attached...
Comment #11
radimklaskaComment #12
ifrikThanks @radimklaska, the links are fine like that.
Now the text needs reviewing as well.
Comment #13
ifrikI tried to make the About section easy to read, and added a use: how to turn off the overlay per user.
Comment #14
ifrikComment #15
jhodgdonThanks! I am not sure I prefer the new wording completely:
In the first sentence, I find "the page" to be a bit ambiguous, which also makes "that page" a bit ambiguous in the next sentence.
The previous wording that this patch is replacing:
I think the previous wording for the first sentence is actually clearer, even though the second is shorter... Maybe you could keep that sentence and then replace "that page" with "the page you were viewing" in the second sentence?
The new Uses item is great!
Comment #16
ifrikI see what you mean. My main issue was trying to get change the first part of the sentence because I kept on reading "site display" as a noun, instead of as "makes .... display".
How about?
Comment #17
lostkangaroo CreditAttribution: lostkangaroo commentedifrik the last interdiff looks like it picked up some extra work you were doing with the text module, but tis a silly thing.
The changed wording is easy to understand and to the point which makes it effective in my book. If jhodgdon approves as well I will do a manual review this afternoon.
Comment #18
jhodgdonRE #17 - the interdiff did, but the patch didn't. :)
The patch has a typo: "... instead of replacing the page in **you** browser window" --> your
Other than that, I think this is fine... Although do we ever call a user account page a "Profile" page in the UI? If not, maybe we should say something like "by editing their user account settings" or something like that in the Uses bullet?
Comment #19
ifrikTypo fixed.
In D8, when you click on your user name, you get a menu with "view profile" and "edit profile", so I suppose we should use "Profile".
Comment #20
ifrikComment #21
jhodgdonSounds good, thanks for the clarification! I think this one is good to go.
Comment #22
jhodgdonThanks everyone! Committed to 8.x.