Problem/Motivation
#3041924: [META] Convert hook_help() module overview text to topics for the menu_link_content, menu_ui module(s).
Proposed resolution
Take the information that is currently in the hook_help module overview section for the module(s), and make sure the information is in one or more Twig help topic files. Steps:
- Find the hook_help() implementation function in the core/modules/MODULENAME/MODULENAME.module file(s). For example, for the core Contact module, the module files is core/modules/contact/contact.module, and the function is called contact_help().
- Locate the module overview portion of this function. This is located just after some lines that look something like this:
switch ($route_name) { case 'help.page.contact':
And ends either at the end of the function, or where you find another
case 'something':
line. - We want to end up with one or more topics about the tasks that you can do with this module, and possibly a section header topic. So, read the help and figure out a good way to logically divide it up into tasks and sections. See Standards for Help Topics for information on how to do this.
- See if some of these tasks are already documented in existing topics. Currently, all topics are in
core/modules/help_topics/help_topics
. Note that to see existing topics, you will need to enable the experimental Help Topics module (available in the latest dev versions of Drupal 8.x). - For each task or section topic that needs to be written, make a new Twig topic file (see Standards for Help Topics) in
core/modules/help_topics/help_topics
. You will need to choose the appropriate module prefix for the file name -- the module that is required for the functionality. Alternatively, if the information spans several modules or if the information should be visible before the module is installed, you can use the "core" file name prefix. For instance, it might be useful to know that to get a certain functionality, you need to turn on a certain module (so that would be in the core prefix), but then the details of how to use it should only be visible once that module is turned on (so that would be in the module prefix). - File names must be MODULENAME.TOPICNAME.html.twig -- for example, in the Action module, you could create a topic about managing actions with filename action.managing.html.twig (and "MODULENAME" can be "core" as discussed above).
- Make a patch file that adds/updates the Twig templates. The patch should not remove the text from the hook_help() implementation (that will be done separately).
Remaining tasks
a) Make a patch (see Proposed Resolution section).
b) Review the patch:
- Apply the patch.
- Turn on the experimental Help Topics module in your site, as well as the module(s) listed in this issue.
- Visit the page for each topic that is created or modified in this patch. The topics are files in the patch ending in .html.twig. If you find a file, such as core/modules/help_topics/help_topics/action.configuring.html.twig, you can view the topic at the URL
admin/help/topic/action.configuring
within your site. - Review the topic text that you can see on the page, making sure of the following aspects:
- The text is written in clear, simple, straightforward language
- No grammar/punctuation errors
- Valid HTML -- you can use http://validator.w3.org/ to check
- Links within the text work
- Instructions for tasks work
- Adheres to Standards for Help Topics [for some aspects, you will need to look at the Twig file rather than the topic page].
- Read the old "module overview" topic(s) for the module(s), at
admin/help/MODULENAME
. Verify that all the tasks described in these overview pages are covered in the topics you reviewed.
User interface changes
Help topics will be added to cover tasks currently covered in modules' hook_help() implementations.
API changes
None.
Data model changes
None.
Release notes snippet
None.
Comment | File | Size | Author |
---|---|---|---|
#32 | 3095740-32.patch | 8.53 KB | jhodgdon |
#32 | interdiff.txt | 7.73 KB | jhodgdon |
Comments
Comment #2
Rangaswini CreditAttribution: Rangaswini as a volunteer and at QED42 commentedComment #3
jhodgdon@Rangaswini -- are you still planning to work on this issue? If so, please comment here. If not, please unassign so that someone else can work on it. Thanks!
Comment #4
Rangaswini CreditAttribution: Rangaswini as a volunteer and at QED42 commentedComment #5
Rangaswini CreditAttribution: Rangaswini as a volunteer and at QED42 commentedComment #6
jhodgdonThis is a good start, thanks!
A few things that need to be done to improve the patch -- see the help topics standards page for reference: https://www.drupal.org/docs/develop/documenting-your-project/help-topic-...
a) Steps should be OL lists not UL lists on task topics.
b) A few of the topics are not following our heading structure, like missing the heading for Steps.
c) grammar proofreading in all topics. There are a lot of missing articles (a/an/the).
d) core.menus.html.twig -- how about putting the "What is a menu?" section first? Also when answering "What is a menu link?", answer that question, not "What is the Menu Link module?" (which is what the answer looks like). We don't want module overview topics.
e) Some of the navigation instructions follow our conventions, and some of them don't. See standards page.
f) menu_ui.delete.html.twig -- steps seem to be about adding a menu, not deleting it?
Comment #7
Rangaswini CreditAttribution: Rangaswini as a volunteer and at QED42 commentedComment #8
jhodgdonNeeds work is the correct status for an issue that has a patch that needs to be updated.
Comment #9
Rangaswini CreditAttribution: Rangaswini as a volunteer and at QED42 commentedComment #10
Rangaswini CreditAttribution: Rangaswini as a volunteer and at QED42 commentedComment #11
shimpyComment #12
shimpyHii I have created patch with few changes. Attaching few screenshots for easy review.
Comment #14
shimpyNew patch
Comment #16
jhodgdonI will wait to review this patch until the tests pass... Thanks!
Comment #17
shimpyHii I have recreated a patch for test to pass. Please review.
Comment #18
kuralarasanm CreditAttribution: kuralarasanm as a volunteer and at Cognizant Technology Solutions for Drupal India Association commentedHi,
I have tested the patch and looks good. Related topics also looks well and good. Please find the attached image for reference. Hence I am changing the status to RTBC.
Thanks.
Comment #19
jhodgdonJust a quick look -- this patch has some grammar problems. Also some HTML problems, such as in the top-level topic, under Additional Resources there is a UL list but the item in the list is not in an LI tag. Also not all the topics follow the templates/standards in Standards for Help Topics.
I don't have time for a full review right now and there may be other problems, so I'm just setting it back to Needs Work.
Comment #20
VladimirAusReviewed and updated.
Comment #21
benjifisherI am unassigning this issue and tagging it for MidCamp 2020. I hope we can get someone to work on it today.
Comment #22
benjifisherWe did not get to this issue at MidCamp yesterday, so I am removing the tag.
Comment #23
jhodgdonThanks for the patches! And sorry for the delay reviewing. I had some time today, so I looked at the most recent patch (#20). I did not apply the patch, just looked at the text. It still needs some work:
What is custom menu link?
This heading, if we keep it, should be written "What is a custom menu link?". However, as written, the text in this section seems to be describing the functionality of the Custom Menu Links module, not giving us information about what a custom menu link is. I am getting kind of tired of saying this in reviews, but please go read the help topic guidelines https://www.drupal.org/docs/develop/documenting-your-project/help-topic-... before making or revising patches. We do not want to have text that describes modules. We want to have text in our topics that tells you how to do things with Drupal (task-oriented), or that give overviews and background information that you would need to perform these tasks. So, this section needs a rewrite, or maybe the small amount of information that is actually needed to describe custom menu links should be added to the previous section? So that section might just have a few sentences saying that the links in the menu can either be provided by modules, or can be custom (and explain what a custom link is).
Choose <em>Menu settings</em> tab. Select the required menus.
We should word this as "Under Menu settings, select the menus that you would like to have content items placed in", or something like that.Comment #24
jhodgdonHere is a new patch. Pretty much everything changed, so I did not make an interdiff file (not useful).
Comment #26
pratik_kambleComment #27
pratik_kamble@jhodgdon, I have checked the patch in #24. I have verified the patch using the tasks mentioned for the review process against branch 9.1.
Verified the grammar for the patch and found few minor issues for punctuation and grammar.
Validated HTML, no issues found.
Links within the text redirects to appropriate pages.
Adheres to Standards for Help Topics.
The text is written in clear, simple, and in straightforward language. At two places I find changes were needed so attaching a new patch with correction.
Comment #28
pratik_kambleComment #29
jhodgdonThanks very much for the careful review! The changes you have suggested look very good to me. I think that since you have given this a careful review, we can mark the latest patch RTBC on the strength of that review with your two small changes.
Comment #30
catchReading through the help text, it looks like we use menu link and menu item interchangeably. Is that by design or should we try to standardise?
Comment #31
jhodgdonGood point, we should standardize on what it says in the UI, which I is "link". We should also verify that this is consistent in the UI, but that would be a separate issue if it's not consistent.
Comment #32
jhodgdonWell, the Menu UI module uses a mix of terms -- it usually says "menu link" or "link", but it occasionally says item. I created an issue for thatL:
#3158562: Consistently refer to "menu links" as that, not "menu items", in the UI
I will assume that should be fixed eventually, and proceed with this fix. Note that some UI text is referred to directly in the help, and so the word "item" still appears in the help. Here's a new patch.
Comment #33
Adsyy CreditAttribution: Adsyy as a volunteer commentedHi, i'm a bit new to contributing, i've setup my local environment 9.1-dev and apply your patch. I've look the topic and how to complete this "Review task", seems good for me. passing to RTBC.
Comment #35
catchCommitted f669278 and pushed to 9.1.x. Thanks!