Problem/Motivation

#3041924: [META] Convert hook_help() module overview text to topics for the aggregator module.

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:

  1. 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().
  2. 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.

  3. 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.
  4. 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).
  5. 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).
  6. 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).
  7. 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:

  1. Apply the patch.
  2. Turn on the experimental Help Topics module in your site, as well as the module(s) listed in this issue.
  3. 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.
  4. 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].
  5. 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.

CommentFileSizeAuthor
#65 Screenshot 2019-11-12 at 6.59.16 PM.png93.72 KBanmolgoyal74
#65 Screenshot 2019-11-12 at 6.59.05 PM.png103.11 KBanmolgoyal74
#65 Screenshot 2019-11-12 at 6.58.56 PM.png134.94 KBanmolgoyal74
#64 3046957-64.patch6.34 KBjhodgdon
#62 3046957-62.patch6.32 KBjhodgdon
#62 interdiff.txt10.09 KBjhodgdon
#59 Managing feeds.png32.14 KBshimpy
#59 creating feeds.png23.79 KBshimpy
#59 Aggregator overview.png45.7 KBshimpy
#59 aggregator_help_topic_59.patch5.8 KBshimpy
#59 interdiff.txt5.87 KBshimpy
#56 aggregatoroverview1.png195.8 KBGayathri J
#56 managingaggragator.png133.77 KBGayathri J
#55 aggregator123_help_topic.patch5.79 KBGayathri J
#52 managingfeed.png128.68 KBGayathri J
#52 creatingfeed.png124.63 KBGayathri J
#52 overview.png190.66 KBGayathri J
#49 aggregator_help_topics-3046957-49.patch5.63 KBdandaman
#46 Screenshot from aggregator_affter_patch2.png81.26 KBVinodhini.E
#46 Screenshot from aggregator_after_patch1.png76.72 KBVinodhini.E
#46 Screen for aggregator123_after_apply_patch.png141.16 KBVinodhini.E
#44 aggregator123_help_topic.patch5.62 KBGayathri J
#43 screenshot_after_apply_patch .png109.27 KBVinodhini.E
#39 aggregator123_help_topic.patch6.49 KBGayathri J
#36 aggregator_660081.patch6.37 KBGayathri J
#30 3046957.patch5.57 KBshimpy
#30 interdiff.txt8.21 KBshimpy
#30 Screenshot at 2019-10-09 17-55-02.png81.79 KBshimpy
#30 Screenshot at 2019-10-09 17-50-04.png89.43 KBshimpy
#30 Screenshot at 2019-10-09 17-49-46.png111.8 KBshimpy
#30 Screenshot at 2019-10-09 17-49-56.png105.14 KBshimpy
#27 interdiff.txt6.11 KBshimpy
#27 aggregator_help_topics.patch6.31 KBshimpy
#25 Screen Shot 2019-09-20 at 11.21.56 AM.png171.03 KBalonaoneill
#25 Screen Shot 2019-09-20 at 11.20.34 AM.png145.2 KBalonaoneill
#25 Screen Shot 2019-09-20 at 11.19.14 AM.png132.42 KBalonaoneill
#25 Screen Shot 2019-09-20 at 11.15.22 AM.png158.46 KBalonaoneill
#23 aggregator-help-topics-3046957-23.patch5.24 KBMadhura BK
#22 Screen Shot 2019-09-17 at 8.16.17 AM.png69.92 KBalonaoneill
#22 Screen Shot 2019-09-17 at 8.16.10 AM.png62.19 KBalonaoneill
#22 Screen Shot 2019-09-17 at 8.16.02 AM.png50.22 KBalonaoneill
#22 Screen Shot 2019-09-17 at 8.15.53 AM.png72.73 KBalonaoneill
#20 interdiff-3046957-16-20.txt4.75 KBMadhura BK
#20 aggregator-help-topics-3046957-20.patch5.26 KBMadhura BK
#16 aggregator_help_topics-3046957-16.patch5.26 KBspitzialist
#7 aggregator_help_topics-3046957-7.patch1.7 KBcarletex
#16 aggregator_help_topics-3046957-14_applied.JPG14.65 KBspitzialist
#2 issues-3046957.patch2.63 KBchristinlepson
#14 aggregator_help_topics-3046957-14.patch5.26 KBspitzialist
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

gringoinc created an issue. See original summary.

christinlepson’s picture

Created a top-level topic for "Enabling and managing feeds on your site" which includes information on enabling the Aggregator module and creating, editing, and deleting feeds.

Also created a module-level topic instructing users on how to add a feed they've created to their site using block layout admin page and running chron.

jhodgdon’s picture

Status: Active » Postponed

We need to postpone this until the #2920309: Add experimental module for Help Topics gets committed. No idea when/if that might happen right now...

jhodgdon’s picture

Status: Postponed » Needs review

That issue was committed, so we can un-postpone this now!

jhodgdon’s picture

Status: Needs review » Needs work

I reviewed the patch (finally! sorry for the delay) in #2. Thanks very much! I think it needs to be changed a bit:

a) The topic "Enabling and managing feeds on your site" doesn't make sense to me to include. The problem is, that topic would not be visible unless the Aggregator module is already enabled. Also, I don't think we would want to have instructions for enabling modules in multiple topics -- we would leave that to a topic on enabling/disabling modules I would think. So... I think that topic can just be deleted.

b) "Adding a feed to your site" topic -- also take out the part about enabling the module there, and just assume it is already enabled (otherwise the topic would not be visible).

c) Cron does not have an "h" in it.

d) I am confused when I read the "Adding a feed to your site" topic. What block would I enable, and why would I even want a block? I think it needs some background about what it would mean to add a feed to the site, and how it can be displayed, because I am not sure a block is the only option.

e) The files should have newlines at the end.

carletex’s picture

Status: Needs work » Needs review
FileSize
1.7 KB

Hi all!

@clepson thank you for your patch, and @jhodgdon thank you for your feedback.

I'm attaching a patch with the changes suggested by @jhodgdon. Happy to iterate over the text.

kdassing’s picture

Status: Needs review » Needs work
Issue tags: +dcasheville19

I am a novice contributor participating in the mentored first-time contrib space at Drupal Camp Asheville 2019.

I have reviewed the above patch, which applied successfully using Simplytest.me, following the steps laid out in https://www.drupal.org/project/drupal/issues/3047806#comment-13181063.

Verified the test steps and reviewed against @jhodgdon's comment:
a) Topic “Enabling and managing feeds on your site” is removed
b) "Adding a feed to your site" topic no longer mentions enabling the module
c) Cron no longer has an "h" in it
d) The "Display" section now includes the block name and link to the config page, as well as other display options, but does not including supporting information re: "why would I event want a block?" or "what it would mean to add a feed to the site"
e) Newlines are present as expected

I have reviewed and validated the topic for duplicate content, spelling, grammar, and working links. Placing issue in "Needs Work" to potentially iterate on "Display" content per bullet d.

volkswagenchick’s picture

We reviewed the steps for reviewing the patch in the UI during a mentored contribution day at dcasheville19.

1. Apply the patch via git (or use simpletest.me).
2. As a logged-in administrator navigate to Administration > Extend (admin/modules). Enable the 'Help Topics' experimental module and [target] module.
3. Navigate to Administration > Help (/admin/help).
4. Under the 'Topics' section verify [target] is listed.
5. Take a screenshot that verifies the new [target] is present on the page.
6. Select the link of the newly converted core module, the user should be directed to /admin/help/topic/[target-main]
7. Take a screenshot that verifies the main page of [target]. Please review for grammar, spelling, and usability. Review links and ensure they are correct and relevant.
8. Verify "[target-main-title]" is displayed, and the list of related topics are there too.

jhodgdon’s picture

Issue summary: View changes

Updated issue summary with better instructions/guidelines

jhodgdon’s picture

Issue summary: View changes

And one more iteration on the guidelines...

jhodgdon’s picture

Issue summary: View changes

We've migrated the help topic standards to https://www.drupal.org/docs/develop/documenting-your-project/help-topic-... so updating issue summary again.

spitzialist’s picture

Assigned: gringoinc » spitzialist
spitzialist’s picture

Assigned: spitzialist » Unassigned
Status: Needs work » Needs review
FileSize
5.26 KB

Did some adjustments to adjust to the new help topic standards and the issue summary in this issue:
- Divided the exiting topics into a top-level section and three tasks
- Added "Additional ressources" to section and tasks
- Used "Goal/Steps" approach in the tasks where possible
- Used "What is" approach in section

Open question:
- Do we want to include all tasks listed in the online documentation as tasks? https://www.drupal.org/docs/8/core/modules/aggregator/configuring-aggreg...
- Do we really always need steps on task level? In the "Displaying a feed" task, the three listed points are more options and not step-by-step tasks. As for me, it does not make sense to do an individual task for each option, I left it as a list.

volkswagenchick’s picture

Status: Needs review » Needs work

Patch #14 applies successfully.
Steps taken for testing:

  • Applied the patch
  • Enabled Help topic module
  • Enabled aggregator module
  • Navigated to Help page
  • In the Topics section, I only see one of the four sections from the patch listed, "Managing feeds"
  • Screenshot attached
  • Tested "Managing feeds" section
  • Links work as anticipated
  • Grammar looks good.
  • One spelling mistake, see code snippet below

Marking as need work due to that the other 3 sections do not appear. Creating a feed, Displaying a feed, or Managing an exisiting feed

+++ b/core/modules/aggregator/help_topics/aggregator.managing.html.twig
@@ -0,0 +1,19 @@
+<meta name="help_topic:label" content="Managing an exisiting feed"/>

Line 2: Spelling error
Managing an exisiting feed
=> Managing an existing feed

spitzialist’s picture

Thanks for reviewing @volkswagenchick.

- Regarding spelling mistake:
Thanks for the catch, fixed it and attached a new patch.

- Regarding the "missing" sections:
According to the Help Topic Standards, there should only be one top-level section and the other tasks should be related to that section. This is the case here for "Creating a feed", "Displaying a feed", and "Managing an existing feed".
patch_applied

spitzialist’s picture

Status: Needs work » Needs review
jhodgdon’s picture

Status: Needs review » Needs work

Sorry for the delay in reviewing this! Another one that fell off my radar screen by mistake. So, a few comments -- this patch is looking good!!

a) You are correct that we only want 1 top-level topic and having the other topics linked as Related.

b) The Additional info sections...
- In the top-level topic, the link given redirects to https://www.drupal.org/docs/8/core/modules/aggregator/overview and the page title is "Aggregator overview". So, we should use that URL and link text in our documentation.
- I'm also not sure if the information there is useful -- we should only include links to drupal.org if there is actually useful information there, which isn't included in the help topics the module is providing. I'm just not sure if it's all that useful? The problem is also that drupal.org will not be translated, whereas the help topics are... so providing links to information that some of our users will not be able to read is kind of problematic. I think we should only provide a link if there is definitely information there that isn't in our topics. This applies to all topics.
- Rather than provide a link to drupal.org, we should provide "Related" links to other topics. For instance, the "Creating a feed" topic says you need to configure your feed; so it needs to link to the Configuring topic, I think, rather than to the drupal.org page about configuring.
- If you need to link to topics that don't exist yet (such as documentation about Cron), you can create a "stub" topic and put it into the appropriate module's help_topics directory. Make an appropriate title and file name, but have the body be something like "Stub topic about configuring cron tasks". I think this would also apply to Views and Blocks documentation. We'll most likely have to go through these topics at the end and fix up the links... that's OK. We already have an issue for that.

c) Navigation -- we now have standards for how to describe navigation in the Help Topic Standards https://www.drupal.org/docs/develop/documenting-your-project/help-topic-... -- those standards may not have been in place when you made your patch (sorry! It has been an evolving process, but I think the standards are now fairly stable). So, the navigation description in all topics (telling how to get to particular admin pages) needs to be updated. (We adopted a standard similar to the User Guide.)

d) Goal -- in the sample on the Help Topic Standards page, we are using verbs like "Configure" not "Configuring", so we should do that in the Goal sections of these pages too.

e) Wondering if "Managing an existing feed" should be called "Managing and configuring an existing feed"? Also it says just "Select a feed to configure and change the configuration in the next step." but doesn't seem to have a step about the configuration? I think we need to describe that too?

Madhura BK’s picture

Assigned: Unassigned » Madhura BK
Madhura BK’s picture

Made changes suggested in #18. Please review.

Madhura BK’s picture

Assigned: Madhura BK » Unassigned
Status: Needs work » Needs review
alonaoneill’s picture

It looks like this patch doesn't follow suggestions in comment #18.
Also, I found a typo and in word "resources" in "Managing feeds" and provided screenshots.
Thanks

Madhura BK’s picture

@ alonaoneill, had forgotten to git add files before creating patch ,so changes were not reflected,have uploaded the right one now.Thanks for reviewing the previous patch.

Have fixed the typo as well.Thanks.

Madhura BK’s picture

Status: Needs work » Needs review
alonaoneill’s picture

I see you followed all suggestion in #18 and fixed typo.
I uploaded screenshots for faster review since jhodgdon gave all suggestions.

alonaoneill’s picture

Status: Needs review » Needs work
shimpy’s picture

Status: Needs work » Needs review
FileSize
6.31 KB
6.11 KB

Hii team,
I have tried working on the suggested changes and come up with few changes in the steps and added links to some config pages. Please review.

volkswagenchick’s picture

Issue tags: +badcamp2019

Tagging for badcamp2019, thanks! (October 2-5)

jhodgdon’s picture

Status: Needs review » Needs work

This is looking better!

Some comments -- some of them reference the Help topic standards page linked in the issue summary:

a) aggregator.creating.html.twig
- The line at the end about what OPML is seems like background information (at least partly). See Standards page for where this should be. Probably the part about what OPML is should be treated as background, and the part about practicalities (link to the import OPML) could stay in Steps. However, note that rather than linking to the admin page, our standard is to provide navigation instead.
- Also I'm not sure the link in Additional Resources has enough information in it that it should be included?

b) aggregator.displaying.html.twig
- Remembering that these topics are pages for people who are not familiar with the Drupal admin interface, we generally want to link to other topics where possible. So for instance rather than providing links/navigation to Blocks or Views admin pages, we should link to help topics about placing blocks and creating views. If these pages do not yet exist, you can add them as "stub" pages (with titles but no content) in this patch.
- Also rather than linking to admin pages (if it's steps related directly to Aggregator and not topics about Blocks, Views, etc.), use the standards for navigation to the admin pages.
- I think we need some background information added about what "displaying a feed" means, or what the differences are between the options for displaying a feed. Again, see the standards for how to add background information. It might belong in this topic or in an overview topic?
- Navigating to the canonical feed URL ... I don't think this would be necessarily comprehensible to less sophisticated users -- what exactly is aggregator_feed}? That needs to be explained better.

c) aggregator.managing.html.twig
- Goal: "Manage an existing feed and changing its configuration." -- needs to have parallel verbs, either "managing" and "changing" or "manage" and "change". Check the standards.
- Leave out the line saying "This will take you to...", or better yet, include it in the previous line. It is not a step.

shimpy’s picture

@jhodgdon
In reference to #29 I have tried creating a new patch. Please review.

shimpy’s picture

Status: Needs work » Needs review
jhodgdon’s picture

Status: Needs review » Needs work

The meta-data section at the top of the help topic files has recently changed format, so this patch is going to fail testing (we now also have tests for correct formatting of help topics). See the help topic standards page for the new format. https://www.drupal.org/docs/develop/documenting-your-project/help-topic-...

So, the patch will need to be reworked for that.

I reviewed the text in the screenshots though -- it's looking much better!

A few thoughts, and some "nitpicks" for spelling/punctuation/etc.:

a) In the Overview topic, how about making two separate "What is" sections? One for "What is a feed" and another for "What is OPML". That way it is easier for someone to scan and see that there is info there about both subjects.

b) Also in the Overview topic, there should be a section called "Overview of aggregation" and that is where the sentence about what the Aggregator module should go. See help topic standards, down at the bottom where it talks about the structure of overview topics.

c) I'm still not sure we need those online resources that are listed in Additional Resources. We should only put in links to pages that actually have information that is useful but not in the topics... it is better if we instead get the useful information into the topics, where it will be instantly available and also translated, rather than linking to drupal.org (which is untranslated).

d) Managing an existing topic: Do not capitalize "Administration Page" in "which will take you to...".

e) Managing an existing topic: nitpicks:
- The second step should say something like "Choose a feed to configure and click ___ to open the configuration page." Not say "in the next step".
- The third step is a "comma splice" sentence. Fix that please. https://en.wikipedia.org/wiki/Comma_splice
- Third step should still be written in 2nd-person imperative. Like "Select the ... " or "Choose the ...". not "Administrators can".
- There is no step saying to change the configuration.
- Where do you go to update the feed? It seems like we're still in the configuration page, but I don't think it is there?

f) Managing an existing topic -- we need to link to a *topic* about running cron, not online docs. You don't have to write this topic right now, but instead you can make a "stub" topic with a body that says something like "This topic needs to be written".

g) Displaying a feed -- write it in 2nd person imperative.

h) Displaying a feed -- The View help link also needs to be a stub topic. As written, the link doesn't go anywhere.

i) Creating a feed -- might need more detail about how to configure the feed?

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

jhodgdon’s picture

Issue summary: View changes

We just found out that all topic Twig files currently need to go into core/modules/help_topics/help_topics (with their finalized module-based file names), for the time being until the Help Topics module is stable. Updating issue summary. Patch will need to be updated too.

mradcliffe’s picture

Issue tags: +Amsterdam2019

Tagging for Drupal Con Amsterdam 2019.

Gayathri J’s picture

@jhodgdon
I have tried creating a new patch based on # 32 requirements.

jhodgdon’s picture

Status: Needs work » Needs review

Please remember to set issues to Needs Review after uploading a patch. That will automatically make the tests run (you will not have to trigger the tests manually). Also signals to human reviewers that there is something to review. :) I'll wait for the test bot to finish...

jhodgdon’s picture

Status: Needs review » Needs work

Thanks for the patch! Can you make another try? A few things to look for:

a) Navigation. The patch has things like:

+  <li>{% trans %}In the Manage administrative menu, navigate to Configuration > Web Services > Aggregator.{% endtrans %}</li>

This has a few problems:
1. Everything you see in the Drupal UI text should be in italics, so for example <em>Manage</em>. Same with Configuration, Web services, etc. This also applies to things like buttons/links to click, field names, etc.
2. Check the capitalization. I think it is probably "Web services" not "Web Services" in the UI.
3. The > signs should be using HTML entities &gt;
4. The last piece of the navigation should be turned into a link to the admin page in question. There are examples of how to do this in other recent patches I've made for help topics, such as #3067617: Convert contact module hook_help() to topic(s)
5. Don't add text (or worse yet a new step) that says something like "which will take you to the Aggregator administration page."

b) Spell checking -- please run everything through a spell checker and fix spelling errors. Also, acronyms like "url" need to be all-caps URL.

c) Make sure that everything exactly matches what you see in the Drupal administration interface. For instance, I don't think there is a button that says "OPML-Import". That doesn't seem right.

d) Do not put background information into steps. For instance, in the Adding topic, you have a "step" that says "OPML is an XML-based file format used to share outline-structured information such as a list of RSS feeds. Feeds can also be imported via an OPML file." That is not a step. We have information in the Help Topic Standards for what to do with background information. Besides which, step 2 above that has importing via OPML file right there.

e) Do not use an OL list for things that are not really numbered items, like in the Displaying topic. Really, those aren't steps anyway, they are options for how to display a feed... this is not really a Task topic, or it is multiple task topics... Again, these aren't steps.

Gayathri J’s picture

#38 I re-created patch please review.

Gayathri J’s picture

Status: Needs work » Needs review
Madhura BK’s picture

jhodgdon’s picture

Status: Needs review » Needs work

This is looking way way better, thanks! A few more notes:

a) There is still a lot of text that I think needs to be put in italics. *Everything* in the UI should be in italics. So for example in this line in the Creating topic:

Through <em>Import OPML</em>, choose OPML file and enter OPML Remote URL, set the update interval time and click <em>Import</em> button.

If there is a field called "OPML file" that should be in italics; if the field has another name, that should be in the step and it should be in italics. Same for the remote URL and update interval fields. Be specific.

Also "In the Manage administrative menu" --> Manage should be in italics too.

b) I would replace the word "through" in that previous line with

If you clicked <em>Import OPML</em>, ". "Through" doesn't make any sense to me.

c) In the Displaying topic, check the HTML syntax. It seems there are a bunch of LI items, but no UL or OL heading. Probably needs to be an OL... but it isn't really "steps" per se. Since it doesn't really have steps, it seems like it is not a Task topic. So let's just add it to the overview topic instead.

d) In topics for the Aggregator module, we cannot link to pages that are provided by other modules if they are not dependencies. So for instance
<code>
+{% set block_admin_url = render_var(url('block.admin_display')) %}

That will make the whole topic have an error if the Block module is not installed.

e) Also we should make sure that the block.place topic is linked as Related in any topics that talk about placing blocks.

f) I know we do not have a cron topic yet, but we should have one sometime. So we should have a link in Additional Information linking to online documentation about cron. We should just leave this out and eventually we will link to the cron topic when it is written.

g) I have mentioned this before, but many of your patches have this at the end:

diff --git a/git b/git
new file mode 100644
index 0000000000..e69de29bb2

Please do not include this empty "git" file in your patches. Always do a "git status" command before you make the patch, and make sure you are only including the files you want to include in your patch.

Vinodhini.E’s picture

@J.Gayathri, I applied patch #39, works for me now, thank you!

Gayathri J’s picture

#42 Hi jhodgdon
Thanks for the review i re-created patch please review.
f) I know we do not have a cron topic yet, but we should have one sometime. So we should have a link in Additional Information linking to online documentation about cron. We should just leave this out and eventually we will link to the cron topic when it is written: I am not sure what are you suggesting i think already we have cron online documentation then why said leave it anyway i removed that in additional information.
Thank you!

Gayathri J’s picture

Status: Needs work » Needs review
Vinodhini.E’s picture

@J.Gayathri, I applied patch, everything is fine, Thanks for patch.

Gayathri J’s picture

Hi Vinodhini , Thanks for the review.

dandaman’s picture

Status: Needs review » Needs work

I applied this patch and verified that I see the same that @Vinodhini.E has in his screenshots.

+++ b/core/modules/help_topics/help_topics/aggregator.managing.html.twig
@@ -0,0 +1,19 @@
+  <li>{% trans %}Choose a feed to configure and click <em>add feed</em> button to open the configuration page.{% endtrans %}</li>

Says "add feed" when it should say "Edit" button here. Maybe edit button to the right or something would be helpful? (No, probably not - it could maybe be flipped in some languages?)

I'll try to see if I can roll an updated patch shortly.

dandaman’s picture

Status: Needs work » Needs review
FileSize
5.63 KB

Trying to work on this a bit today at DrupalCorn Camp today.

It's been a while since I've done a patch for core, but I think this is attached with the code updated to include the "edit" link on the "manage" page of the Help Topics.

jhodgdon’s picture

RE #44 -- We do not want links to drupal.org unless we really need them. Just because the Cron topic is not currently written is not a good reason to link to drupal.org. This module Help Topics is EXPERIMENTAL. That means that the topics are not complete yet. When I wrote the Block Place topic, I went back and made links in every topic that was already committed, and every patch that was in progress, to the new topic. But I didn't have to take anything out. The same should apply here for Cron. I guess someone already took that out...

For everyone: it is very helpful if you make an interdiff file when you make a new patch, so we can see what changed. That way I can review just the changes quickly. There were no interdiffs made for the last few patches... I will review the whole patch sometime soon, but in the meantime, please go take out those references to drupal.org cron documentation. Thanks!

Gayathri J’s picture

Hi jhodgdon
What ever you mention in comment #42 I have tried creating a new patch. Added screenshots as well please review. form next patch onwords i will create interdiff file.
Thank you!

Gayathri J’s picture

FileSize
190.66 KB
124.63 KB
128.68 KB
jhodgdon’s picture

This is looking better! I took a look at the screenshots, and I have a few suggestions:

(1) Overview topic
(a) OPML section -- the last sentence isn't a sentence... and it is hard to tell why I should care about OPML in the context of feeds. Maybe that last sentence was trying to say that OPML is a commonly-used format for a list of feeds? Please make that clearer.The

(b) Overview section -- it seems like it has 4 sentences basically saying the same thing. Please reduce it so there is not so much repetition. Also check the topic standards -- it should end with a standard sentence about "See the related topics below...".

(c) Displaying a feed section
-- header needs to start with "What is" or "What are"
-- it should be above the Overview section.
-- Take out the navigation (this is not a task topic). Instead, link to the block.place topic for the two block items, something like "See Placing a block for more information.". Leave the navigation out for views too, and when the views topic is finished, we'll link to that there too.

(2) Creating a feed looks good! Needs some minor grammar/wording proofreading but nothing major.

(3) Managing and configuring topic:
- Someone else mentioned this in a previous comment -- if you are managing an existing feed, you should not be clicking "Add feed" in step 2.
- Step 3 should start with navigation.
- Step 4 seems to be back on the first navigated page, so probably it should be moved ahead of step 3?
- Eventually for cron we want to ***link to the cron topic*** not to the cron page. So please take that link out because we will want to change it later and it is easier to add a link than to take out stuff we don't want and rewrite the sentence.

jhodgdon’s picture

Status: Needs review » Needs work
Gayathri J’s picture

Status: Needs work » Needs review
FileSize
5.79 KB
Gayathri J’s picture

Hi, re-created patch please review.
(c) Displaying a feed section
-- header needs to start with "What is" or "What are"->Already we mention what are feeds in above section so here added how to display only for this reason not changed title.

1) I added additional resource for cron topic, ***link to the cron topic*** not to the cron page means in 5th step given link for cron page need to remove this link?

2) I think so we need to add block module topic in related topic? i added block.place link only in displaying feed topic.

Thank you

Vinodhini.E’s picture

jhodgdon’s picture

Status: Needs review » Needs work

Please take a look at the patch for #3087562: Convert datetime, datetime_range, field, field_ui, link, options, telephone, text modules hook_help() to topic(s). The overview topic there is an illustration of having multiple "What is/are" sections. Our standard is that they should all start with "What" so that they are easily scannable as background information. It could be "What are the options for displaying feeds?" for example.

Please also take a look at the patch for #3047723: Convert views, views_ui module hook_help() to topic(s). The last step in views_ui.add_display topic has an example of linking to another topic rather than repeating information in the steps. We will want to do the same for the cron eventually, although right now that cron topic is not there.

Please also take a look at the topic help.help_topic_search that is already committed. That has an example of telling someone to run cron in the 4th step.

I hope these examples will clarify what I have been trying to say... sorry that I haven't been clear... I'm setting the issue back to Needs Work so that you can try to follow these examples. Thanks!

ravi.shankar’s picture

Status: Needs work » Needs review
jhodgdon’s picture

Assigned: Unassigned » jhodgdon

In the future, please do not add so many tests. It is not necessary, and each test takes time on a test server in the cloud that the Drupal Association has to pay for. Just the standard test that runs when you set the issue to Needs Review is sufficient. Thanks!

Anyway, I took a look at the latest patch -- thank you! However:
- It needs proofreading for grammar and punctuation.
- Please read my comments #53 and #58 again ****carefully**** because many of the things I suggested were not done. And please again review the help topic standards.

Actually, I will just assign this to myself and make the next patch.

jhodgdon’s picture

FileSize
10.09 KB
6.32 KB

Here's a new patch. A few notes (beyond my previous comments on this issue):

1. I found out that the OPML paragraph was copied word-for-word from Wikipedia. That is not OK without a citation. I rewrote it.
2. I took out the links to the online documentation in Additional Resources. It just didn't have any useful information that we didn't cover in topics.

Status: Needs review » Needs work

The last submitted patch, 62: 3046957-62.patch, failed testing. View results

jhodgdon’s picture

Status: Needs work » Needs review
FileSize
6.34 KB

Whoops, I left some text untranslated in the aggregator.overview topic. Here's a new patch. The line changed is

+<p>Feeds are <em>updated</em> (new items imported) each time your site's cron task runs, if the selected <em>Update interval</em> for the feed has passed. You can also update feeds manually; see <a href="{{ config_topic }}">Managing and configuring an existing feed</a> for details.</p>

(added Twig trans tags)

anmolgoyal74’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
134.94 KB
103.11 KB
93.72 KB

The patch looks fine to me. Marking it RTBC.

Gayathri J’s picture

The patch looking fine.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed 4b5713b0a2 to 9.1.x and b144a708ab to 9.0.x. Thanks!

  • alexpott committed 4b5713b on 9.1.x
    Issue #3046957 by Gayathri J, shimpy, jhodgdon, Madhura BK, spitzialist...

  • alexpott committed b144a70 on 9.0.x
    Issue #3046957 by Gayathri J, shimpy, jhodgdon, Madhura BK, spitzialist...
alexpott’s picture

Version: 9.1.x-dev » 9.0.x-dev

Status: Fixed » Closed (fixed)

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