Problem/Motivation

#3041924: [META] Convert hook_help() module overview text to topics for the block and block_content modules.

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
#68 Screen Shot 2019-10-30 at 6.23.22 PM.png297.98 KBalonaoneill
#68 Screen Shot 2019-10-30 at 6.22.46 PM.png376.03 KBalonaoneill
#68 Screen Shot 2019-10-30 at 6.21.41 PM.png205.49 KBalonaoneill
#68 Screen Shot 2019-10-30 at 6.20.52 PM.png234.92 KBalonaoneill
#68 Screen Shot 2019-10-30 at 6.20.26 PM.png206.76 KBalonaoneill
#68 Screen Shot 2019-10-30 at 6.20.00 PM.png173.25 KBalonaoneill
#68 Screen Shot 2019-10-30 at 6.19.29 PM.png315.23 KBalonaoneill
#68 Screen Shot 2019-10-30 at 6.17.34 PM.png67.89 KBalonaoneill
#68 Screen Shot 2019-10-30 at 6.17.17 PM.png273.55 KBalonaoneill
#67 interdiff.txt511 bytesjhodgdon
#67 3047709-67.patch12.1 KBjhodgdon
#65 interdiff.txt4.35 KBjhodgdon
#65 3047709-65.patch12.12 KBjhodgdon
#62 3047709-62.patch8.35 KBjhodgdon
#59 block123_help_topic.patch7.7 KBGayathri J
#56 book_help_topic135.patch7.71 KBGayathri J
#51 placingblock.png82.66 KBGayathri J
#51 managinblocks.png77.88 KBGayathri J
#51 configuring.png86.16 KBGayathri J
#51 creating blocktype.png55.67 KBGayathri J
#51 creatingblock.png60.86 KBGayathri J
#51 block_help_topic.35.patch7.61 KBGayathri J
#50 block_help_topic-12345.35.patch7.57 KBGayathri J
#47 block12345_help_topic.patch16.13 KBGayathri J
#40 Screen Shot 2019-09-23 at 9.50.14 AM.png182.99 KBalonaoneill
#40 Screen Shot 2019-09-23 at 9.48.59 AM.png188.53 KBalonaoneill
#40 Screen Shot 2019-09-23 at 9.45.38 AM.png200.18 KBalonaoneill
#40 Screen Shot 2019-09-23 at 9.44.43 AM.png224.88 KBalonaoneill
#39 block_help_topic.patch5.84 KBGayathri J
#37 book_help_topic.patch5.84 KBGayathri J
#33 interdiff_29_33.patch4.93 KBanmolgoyal74
#33 block-topic-3047709-33.patch8.7 KBanmolgoyal74
#29 interdiff-3047709-21-29.txt5.52 KBMadhura BK
#29 block-topic-3047709-29.patch7.62 KBMadhura BK
#24 Screen Shot 2019-08-20 at 4.44.50 PM.png229.47 KBalonaoneill
#24 Screen Shot 2019-08-20 at 4.43.54 PM.png52.19 KBalonaoneill
#24 Screen Shot 2019-08-20 at 4.40.57 PM.png224.32 KBalonaoneill
#24 Screen Shot 2019-08-20 at 4.40.31 PM.png260.16 KBalonaoneill
#24 Screen Shot 2019-08-20 at 4.40.18 PM.png182.46 KBalonaoneill
#24 Screen Shot 2019-08-20 at 4.40.09 PM.png222.81 KBalonaoneill
#24 Screen Shot 2019-08-20 at 5.01.31 PM.png97.86 KBalonaoneill
#23 interdiff_17-21.txt10.52 KBspitzialist
#21 block-topic-3047709-21.patch7.5 KBspitzialist
#18 Screen Shot 2019-08-08 at 2.01.02 PM.png81.44 KBalonaoneill
#18 Screen Shot 2019-08-08 at 8.46.01 AM.png95.39 KBalonaoneill
#18 Screen Shot 2019-08-08 at 8.45.49 AM.png123.21 KBalonaoneill
#18 Screen Shot 2019-08-08 at 8.45.29 AM.png68.83 KBalonaoneill
#18 Screen Shot 2019-08-08 at 8.45.15 AM.png74.7 KBalonaoneill
#17 block-topic-3047709-17.patch6.16 KBspitzialist
#16 Screen Shot 2019-08-06 at 7.48.09 PM.png50.95 KBalonaoneill
#16 Screen Shot 2019-08-06 at 7.35.19 PM.png161.83 KBalonaoneill
#16 Screen Shot 2019-08-06 at 7.34.57 PM.png88.77 KBalonaoneill
#16 Screen Shot 2019-08-06 at 7.32.54 PM.png29.25 KBalonaoneill
#15 block-topic-3047709-15.patch4.73 KBspitzialist
#15 interdiff_2-15.txt3.54 KBspitzialist
#2 3047709.patch2.66 KBpetedussin
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

petedussin created an issue. See original summary.

petedussin’s picture

Created twig template, very closely matching current help, but more question/answer format.

petedussin’s picture

Status: Postponed » Needs review

set to needs review.

benjifisher’s picture

There is a project-management challenge here:

+{% set blockcontent_help_url = render_var(path('help.page', {'name': 'block_content'})) %}
...
+<p>{% trans %}You can add custom blocks, if the <em>Custom Block</em> module is installed. For more information, see the <a href="{{ blockcontent_help_url }}">Custom Block help page</a>.{% endtrans %}</p>

That creates a link to /admin/help/block_content. When we deprecate hook_help(), that page will go away. But we cannot update that link until we have written the new Help Topics for the block_content module.

Maybe all we need is a new child issue of #3031642: Deprecate hook_help() and combine with Topics to look for all links to the old help pages (render_var(path('help.page', ...)) and change them to point to Help Topics.

jhodgdon’s picture

Status: Needs review » 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

Title: Convert block module hook_help() to topic(s) » Convert block, block_content, and block_place module hook_help() to topic(s)
Issue summary: View changes

We have decided to group modules into fewer issues, instead of having one issue per core module.

So, this issue needs to be expanded to also cover the block_content and block_place modules.

jhodgdon’s picture

Issue summary: View changes

Updating issue summary to latest template.

jhodgdon’s picture

Status: Needs review » Needs work

Needs help added for additional modules. Please review the issue summary about making it topic-based. Thanks!

spitzialist’s picture

Assigned: Unassigned » spitzialist

I start working on this issue.
-- update: Have to postpone the work due to problems with local dev env.

spitzialist’s picture

Assigned: spitzialist » Unassigned
spitzialist’s picture

Assigned: Unassigned » spitzialist

Starting working on this task.

jhodgdon’s picture

Issue summary: View changes

Updated issue summary with better instructions/guidelines

jhodgdon’s picture

Issue summary: View changes

And some even better guidelines -- check out the new issue summary.

spitzialist’s picture

Assigned: spitzialist » Unassigned
Status: Needs work » Needs review
FileSize
3.54 KB
4.73 KB

Worked on this issue in the last days.

Things I did:

  • Applied and tested existing patch as described in the issue summary (-> /admin/help/topic/block.overview).
  • Integrated block_content, and block_place module help in Q/A-style.
  • Adjusted integrated links.
  • Cleaned up a little bit so fewer links between the integrated module helps are present which would refer to content which is now on the topic site.

Thanks in advance for your feedback!

alonaoneill’s picture

Patch applied. Help topics shows up on "Help" page. (Screen provided)
This patch need some work:
- Please review new better guidelines (Standards for Help Topics). We don't want all those h3 heading to be in top-level topic, so I would suggest to create Reated topics.
- The name of the Help topic is "Block", we don't want topics called things like that. I would call it something like "Managing blocks"? Maybe?
- We definitely do not want links to the old-style help page for the Field/Field UI modules in this patch.
- "...see the online documentation for the Custom Block module." link doesn't work (screen added).

I also added screenshots.
Marking needs work.
Thanks

spitzialist’s picture

Status: Needs work » Needs review
FileSize
6.16 KB

Thanks alonaoneill for the feedback.

With the attached patch, I try to resolve the mentioned issues:
- Created related topics and linked to overview section.
- Adjusted overview topic naming and changed content according to guidelines.
- Changed links to online ressources.
- Fixed broken link.

For now, I am not sure about the "Step" section of the related topics. In my opinion, those are more options within a task and not a specific order that need to be followed (as suggested in the guidelines). For now, I left the different parts as separate paragraphs.

alonaoneill’s picture

- Patch applied. Help topics enabled and "Managing blocks" shows up on Help page in Topics.
- "Managing blocks" is top_level topic, that has Related topics (Screenshots added).
- All links work! Broken link was fixed.
- Links to online ressources were changed as suggested.

@jhodgdon Do you have any thoughts/suggestions about "Step" section?

jhodgdon’s picture

Status: Needs review » Needs work

Thanks for the patch and the review and screenshots!
My thoughts (only looking at the screenshots, since Alona checked the links):

  • Managing blocks topic: looks great! And good that there is only one top-level topic.
  • Configuring block settings:
    -- Typo -- spelling -- individual [in Goal section]
    -- Steps section -- please change to a numbered list. Such as: step 1 would tell you how to navigate to the block placement page. Step 2 would tell you to click on the theme whose blocks you are configuring. step 3 would tell you to use the preview link to see where the regions are in your theme. step 4 would tell you to find the region, locate the block, and click edit. step 5 would tell you to edit the settings, and maybe have a bullet list explaining the ones that are common to all blocks (block title, explain why it's different from the block name, and the page/content type restrictions). step 6 would tell you to save the block settings.
  • The other two topics -- also convert to steps in a similar manner
  • It looks like maybe there are 2 tasks in the Placing blocks section: one on placing, and the other on moving. And you might link to Configuring as Related?
  • It looks like maybe there are two tasks with custom blocks -- so maybe there should be one on creating a custom block type, and a second one on creating a custom block?
  • Given that... it might be good to explain on the overview (Managing) topic, another heading called "What is a custom block?" that would explain custom block types, and the difference between blocks provided by modules and custom blocks that you edit.
  • Rather than linking to the online documentation for the field module, let's have it link to the Fields overview [which doesn't yet exist]. This patch could create a stub topic for that and put it in core/modules/fields, so that the link can go somewhere.
  • When you talk about placing blocks in the Custom blocks topic, how about making the Placing blocks topic "Related" instead of linking to the Block layout admin page?
  • I'm thinking that the standards need an update: let's add a section at the end for "Additional information" that would link to online resources, rather than having all the "For more information" things in the middle... I'll move that discussion to #3041928: ISSUE TEMPLATE -- Do not edit -- template for making child issues after I'm done commenting here.
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. New standards have a section called "Additional resources".

spitzialist’s picture

Status: Needs work » Needs review
FileSize
7.5 KB

Thanks for the feedback.

I am attaching a new patch adressing your feedback:
- Corrected spelling mistake in "Configuring block settings".
- Used numbered list in tasks.
- Linked "Configuring" in "Placing" as related.
- Did not divided "Placing blocks" as it seems an overkill to have "Moving blocks" as a separate topic. But I rearranged "Placing blocks" to be mores strcutured.
- Did also not divided "Creating custom blocks" as this is one topic with very similiar and linked information in my optionen. I created two sub-lists within the task list to have a better structure.
- Added "What is a custom block" to the overview.
- Created a stub topic for the fields help topic and linked to it.
- Linked "Placing" in "Custom" as related.
- Added "Additional resources" where needed.

joy29’s picture

Assigned: Unassigned » joy29

@spitzialist when you attaching new patch w.r.t your others patch please upload indiff file too..

Thanks

spitzialist’s picture

Assigned: joy29 » Unassigned
FileSize
10.52 KB

Attached interdiff between #17 and #21.

alonaoneill’s picture

-"Managing blocks" looks good.
-"Configuring block settings" I see that typo was corrected and list in tasks numbered.
- "Placing blocks" has a bunch of typos I underlined those in a screenshot.
-"Creating custom blocks" in "Steps" maybe "2.Use the Blocks link to configure and add custom blocks:" and "3.Use the Block types link to configure custom block types:" use a bulleted list instead of numbered?

@jhodgdon any thoughts?
- "Additional resources" Should it be "Online documentation for Custom Block module" or "Working with Custom Blocks" same as the name of the page?

Thanks for working on this module!

jhodgdon’s picture

In general, I think links to pages on drupal.org should use the page name as the link title.

volkswagenchick’s picture

Status: Needs review » Needs work

I am bumping this back to Needs Work status as per the comments made in #24 by @alonaoneill

jhodgdon’s picture

Thanks for the new patch and review and screenshots! The topics mostly look pretty good. I reviewed again just by looking at the screenshots -- one thing I didn't check was whether the link text and button text described in the Steps is the same as what you see in the UI... Anyway, here are my notes:

a) Managing blocks
-- Typo at the end of the Custom blocks section -- the paragraph ends in a , instead of a .

b) Configuring block settings
-- Goal: I think I would take the word "available" out?
-- Step 1 -- Navigation should be described in words. See https://www.drupal.org/docs/develop/documenting-your-project/help-topic-... in section where it describes "Steps".
-- Step 2 -- I don't think there is a link called "Configure" for the theme. I think the links just say the theme name, like "Bartik"?
-- I think there needs to be some background information telling the difference between the block name and the block title.

c) Creating custom blocks
-- Step 1 -- see note above about navigation
-- Seems like there are two topics: one about creating a custom block, and one about creating a block type. Let's split them up.
-- Somewhere we need some background information about what a block type is.

d) Placing blocks
-- I'll just note here that the part about demonstrate block regions has different wording than what is in Configuring block settings, and describes different UI text. They need to be unified.

Madhura BK’s picture

Assigned: Unassigned » Madhura BK
Madhura BK’s picture

Adding patch which contains the changes suggested in #27. Please review.

Madhura BK’s picture

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

Assigned: Madhura BK » Unassigned
jhodgdon’s picture

Status: Needs review » Needs work

Looking much better, thanks!

I still think that the block.custom.html.twig file needs to be split up into two separate topic files. One would be on creating custom block types, and the other would be about creating custom blocks. (They should link to each other as Related.)

Another thing we should consider is that we have different install profiles. We should not assume that someone started with the Standard install profile when we write help. So, in the topic about creating custom blocks, we should not assume that there is already a custom block type that is defined -- I am pretty sure that if they installed with the Minimal install profile, there would not be a custom block type already there (that is provided by the Standard install profile). So, in the topic about creating custom blocks, we will need to say something about creating a block type if there isn't already one defined... we need to review what it looks like if there are no types. I think there's a message on the screen saying you need to create a block type first, but I'm not sure?

Let's also be careful about words like "click" and "select". In the User Guide, we had standards for that:

For clicking on a link or button, use the phrase "Click (link or button text)", rather than "click on", "select", or other terms. For a select list or radio button, say "Select (choice)". For a checkbox, say "Check (choice)". For a drop-down list of operations links, say "Click (choice)". Make sure that the actual UI text you are selecting or clicking on is in italics.

So we shouldn't say "Select" unless it is a select list or radio button. So in the custom block text, one line says

<li>{% trans %}Select the desired custom block type.{% endtrans %}</li>

but I am pretty sure that is clicking a link, so we should use the word "Click" not "Select".

Thanks for all the work on this -- it is definitely getting closer!!

anmolgoyal74’s picture

I have split the file. For creating a new block type, I think it should be a part of the block content module. I have just created one file for block content.

jhodgdon’s picture

Creating a block type and creating a custom block are both functions of the Block Content module, actually. I don't have time to review this patch for about a week, sorry! Just adding this quick note... hopefully someone else can take a look.

jhodgdon’s picture

Also, thanks for making an interdiff! Next time, if you use the .txt extension on the interdiff instead of .patch, then the automatic tests will not try to run on that patch, and you won't get the "Patch failed to apply" message that you see in #33. :)

jhodgdon’s picture

Status: Needs review » Needs work

I had time to review the patch in more detail today. I was looking at the patch file -- didn't actually try to apply it -- so that still will need to be reviewed/tested eventually. Also all topics need to be reviewed/tested to see if the steps match what you see in the Drupal administrative user interface, which I didn't do in this review.

So... A few thoughts -- thanks again for the patch(es)! :

a) Configuring block settings topic:
- This from comment #27:
I think there needs to be some background information telling the difference between the block name and the block title.
That should be in this topic.
- Also there are more settings that every block has, not just the title and visibility. Let's be more comprehensive. For instance, there is a checkbox about whether or not to display the title, and there is also the block name field, and there is the region as well, right?

b) Creating custom blocks topic:
- The topic seems to have some text left over from when it was split into two topics. That needs to be cleaned up. For instance, we don't need two levels of numbered list, and we don't need a heading called "Creating custom blocks" inside this topic's steps.
- These things "Click (button text)" and "Select (link text") are not right. That was a misunderstanding of something I said in #27 I guess, sorry! What I meant was you should say, for example, "Click Add custom block..." or "Select the desired block type". You should not have the literal text "button text" or "link text" in the text of the topic.
- This topic should be in the block_content module, not in the block module.

c) Managing blocks topic:
- There should not be a newline in the middle of a paragraph. There is one now just before "See the related topics listed below". So, that should be joined with the previous line.
- The link https://www.drupal.org/documentation/modules/block/ ... So for one thing, it redirects to https://www.drupal.org/docs/8/core/modules/block/overview and for another thing, the page title there is "Working with blocks". So, we should put the new URL in there, and the new page title.

d) Placing blocks topic:
- typo: "exisiting" (spelling error)
- Navigation needs to be fixed. It should match the navigation in the "Configuring block settings" topic, which I think is correct. Also the step of clicking the theme name is missing.
- This topic is called "Placing blocks". It probably should be titled something like "Adding a block to a region", because people really don't know what "Place block" means. Also, I guess this topic should be split up into two topics -- this one about placing/adding a block, and a separate topic about moving a block to a different region. Or... actually we should probably talk about the region settings in the Configuring topic? But if someone was trying to find information about how to move a block to a different region, I don't think they would know to go to the topic about adding/placing a block. So we need a different topic with documentation of how to change where a block is placed.
- The Place Blocks module is currently Experimental, so for now I do not think we should add information about it to this topic.

e) Creating custom blocks types topic:
- The title should be "Creating custom block types" not "blocks types".
- The About section does not belong in this topic. Please read the help topic guidelines (see link in issue summary).
- See comments in (b) about text left over from splitting this topic into two topics, and about the "button text" stuff.
- I do not think we should have documentation about the Fields module in Additional Information. Instead, we have the (currently not written) Fields topic as a Related topic. So, please remove that link.
- This page that is linked in Additional Information https://www.drupal.org/documentation/modules/block_content/ seems totally useless. Let's only make links that take us to actually useful pages.

Gayathri J’s picture

FileSize
5.84 KB

#36 Hi @jhodgdon I have created patch for help_topic block module as per the above mentioned requirements.
Please review

Gayathri J’s picture

Gayathri J’s picture

Status: Needs work » Needs review
FileSize
5.84 KB
alonaoneill’s picture

- "Adding a block to a region" has few typos.
- Adding screenshots for @jhodgdon to review suggestions in comment #36

volkswagenchick’s picture

Issue tags: +badcamp2019

Tagging this for badcamp2019. (October 2-5)

jhodgdon’s picture

Status: Needs review » Needs work

I started to review this patch, but just looking at the first topic in the patch block.configure.html.twig -- it doesn't look like the items I mentioned in comment #36 have been addressed, so I stopped reviewing.

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.

Gayathri J’s picture

Gayathri J’s picture

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.

Gayathri J’s picture

Updated as per #46. Please review.

Gayathri J’s picture

Please ignore #47 as there was some trailing whitespace line error.

jhodgdon’s picture

OK, I will wait for you to set the issue to "needs review" and then review your patch. Thanks!

Gayathri J’s picture

Gayathri J’s picture

Hi jhodgdon
Please ignore #50, i re-created patch for block help topics based on help topic standers and based on #36 requirements.
In placing blocks topic mention is create new page for how to move blocks in different region for this i mention point in placing blocks page. i think that page is not required as per your suggestions i will going to create page i will in update next patch. for reference i added screenshots as well.

Please review the patch thanks!

jhodgdon’s picture

Status: Needs work » Needs review

Setting issue status to "Needs review" to indicate this needs review, and to launch the automated tests. I will review once the tests come back green. Thanks!

The last submitted patch, 50: block_help_topic-12345.35.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 51: block_help_topic.35.patch, failed testing. View results

jhodgdon’s picture

The test failure says:

Topic block.overview Twig file has all of its text translated

Looks like the problem is the "Additional resources" string, which indeed is missing the Twig translation tags.

Gayathri J’s picture

#55 i re-created patch please review.

Gayathri J’s picture

Status: Needs work » Needs review
jhodgdon’s picture

Status: Needs review » Needs work

This is getting better! Thanks for all the patches.

Here are a few thoughts for improvements -- see the Help topic standards for reference:

0. All files: If you click the link to the patch in #56 you can see it says:

\ No newline at end of file

at the end of each files. They all need to have a newline at the end of the file.

1. block.configure.html.twig file:

a)

+<h2>{% trans %}Goal{% endtrans %}</h2>
+<p>{% trans %}Block name is the identification name for blocks, choose blocks based on block names. After placing the blocks here we can change Block title it will show in interface.{% endtrans %}</p>
+<p>{% trans %}Configure the settings of an individual block.{% endtrans %}</p>

This section seems to be a mixture of the Goal (the second paragraph) and some background information. Background information should go in a *separate* H2 section, with "What is..." or "What are..." title.

b) That first paragraph under Goal (once it is moved into a What Is/Are section) also needs some grammatical attention. The second paragraph looks good.

c) In the Steps section, in the UL list of settings that you can change, we need to make sure that the text shown in the topic matches the text the user would see in the Drupal user interface. For instance, I don't think that it says "Block description name".

d) That UL list would also be a good place to put the descriptions of what the settings are... so maybe in (a) just take that first paragraph out, and put the information in this list? For instance, it could say "Block name: The name of the block that is shown in the administrative user interface" [I haven't checked the UI to verify that "Block name" is actually what that setting is called, you still need to check that, but that is an example of what I think would be useful here].

f) Last step says "Save the block settings.". Instead of that, say something like "Click Save to save the block settings" [you will need to check the UI text and see if the button actually says "Save" or "Save configuration" or something else, and put that into the topic.]

g) This line near the top

+{% set block_layout_url = render_var(url('block.admin_display')) %}

I don't think this is used in the file. Please remove.

2. block.custom.html.twig file:

a) I think that this file belongs to the block_content module, not the block module? So the file name should start with block_content.

b) Topic title is "Creating custom blocks" so the goal should not say anything about "Managing".

c) I don't know what this step means: "Use the Blocks link to configure and add custom blocks:". Am I supposed to click a link? If so, say "Click the Blocks link" not "Use the ...". But it seems wrong... Please verify that the steps work if you follow them exactly.

d) This step "Configure the basic settings and click Save." seems a bit vague. You are writing text for a block, right? It should be two steps, and say what you need to put in the title and body fields, and other fields that you see on the page.

e) This step "Once created, custom blocks ..." should be made into a link to another topic. Something like "Follow the steps in (link to the Place Blocks topic) to place the block. You don't need the "Once created" text -- if someone follows these steps, they have already created a block. Then we don't need this line:

+{% set block_layout_url = render_var(url('block.admin_display')) %}

3. block.overview.html.twig file: This is mostly looking excellent! Just a couple of small comments here:

a) "...can be displayed in region on your page" -- I would say "on pages of your site" rather than "on your page".

b) "The Block module allows you to place blocks in regions of your installed themes, and configure block settings. Using the Custom Block module, you can create a library of custom block types to create custom blocks.". These two sentences should have parallel construction. So the second one should start with "The Custom Block module allows you to".

c) "Online documentation for the Managing Blocks." ==> remove the word "the" here.

4. block.place.html.twig file:

a) This topic has too many goals. It should just have one goal, placing a block. At the end you could point people to the configuring blocks topic for information on how to change where a block is placed. Or we could have a separate topic about changing the placement of a block, but I think that configuring topic already covers how to change the region. That should replace the last two steps.

b) wihtin, Confiugure (spelling errors)

c) Navigation is incorrect in the first step (it takes you to the custom block library, not the block layout page).

d) Take a look at the patch file. The LI items do not line up nicely.

e) I don't think we need this line

+{% set block_layout_url = render_var(url('block.admin_display')) %}

5. block_content.custom.html.twig file:

a) We don't need this line

+{% set block_library_url = render_var(url('entity.block_content.collection')) %}

b) The goal is to create a custom block type, not "Manage and create" I think?

c) The last two steps need a lot of expansion.

d) The HTML syntax seems off.. not sure why there are so many closing OL tags at the end?

Gayathri J’s picture

Thanks for reviewing the patch i re-created patch as per your suggestions.
2. block.custom.html.twig file:
a) I think that this file belongs to the block_content module, not the block module? So the file name should start with block_content: Its not under block_content module its block module only so i didn't change file name.

Gayathri J’s picture

Status: Needs work » Needs review
jhodgdon’s picture

Title: Convert block, block_content, and block_place module hook_help() to topic(s) » Convert block and block_content module hook_help() to topic(s)
Issue summary: View changes

I will review your patch soon -- thanks for making it! Meanwhile, I found out that the block_place module is "Hidden" and also it is probably going away soon, so I am removing it from this issue. I think the existing topics are OK though as they were not using this hidden module.

jhodgdon’s picture

FileSize
8.35 KB

Thanks again for the patches! I have reviewed the latest patch, which is looking pretty good.... but I thought it needed some updates for grammar, clarity, adherence to the help topic standards, and more concise wording... So I've put my review in the form of a new patch (I didn't make an interdiff file, because nearly every line of the patch changed). Some notes on specific changes:

General: see https://www.drupal.org/docs/develop/documenting-your-project/help-topic-...

a) Managing blocks overview topic:
- Added the final sentence that is supposed to be in the Overview section
- Changed the links in Additional Resources to point to the Blocks chapter of the User Guide instead of the resources that were linked there.

b) Configuring topic:
- Decided the background information was useful for more than one of the task topics, so I moved it to the overview (also edited slightly as it was not quite right grammar/wording). Also the UI for blocks uses "block description" not "block name" now, so updated that in the headings/text for the background information.

c) Placing topic:
- I decided the last step was better off in the configuring topic (the step about changing the region), so I moved it there (reworded a bit).

d) Custom block topic
- Belongs in the block_content module, so I changed the file name... was confusing with the other block_custom topic, so I changed both of the file names actually

Status: Needs review » Needs work

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

Gayathri J’s picture

Hii
@jhodgdon thank you so much for reviewing patch.

jhodgdon’s picture

Status: Needs work » Needs review
FileSize
12.12 KB
4.35 KB

Here's a new patch to fix the validation test failure from #62. Also I added a couple of links from existing topics to the Block Overview topic, now that it exists, and in one case, rewrote a few steps in a topic so it referenced the block place topic.

Status: Needs review » Needs work

The last submitted patch, 65: 3047709-65.patch, failed testing. View results

jhodgdon’s picture

Status: Needs work » Needs review
FileSize
12.1 KB
511 bytes

Fixing test fail (wrong Related topic key, actually decided that didn't need that related topic anyway).

alonaoneill’s picture

1. Patch applied. Help Topics module and Book module were enabled.
2. Reviewed Help Topics for grammar and spelling.
3. All links work!
4. Link to documentation page works! But have a different name than on the documentation page. "Blocks chapter of the User Guide" vs "Chapter 8. Blocks", not sure if should be changed.
5. Provided screenshots with all Help topics created!

All looks great to me!
Thanks!

Sutharsan’s picture

Status: Needs review » Reviewed & tested by the community

I have reviewed the text to see whether it is clear and understandable. It is to me.

Regarding @alonaoneill's comment ""Blocks chapter of the User Guide" vs "Chapter 8. Blocks", not sure if should be changed."
I agree with the new name. It describes properly where the user will go to. It creates the right expectation with the user. The title of the target page has a number in it and that may change in the future. Keeping them in sync would be a maintenance burden we want to avoid.

  • larowlan committed bae8faf on 9.0.x
    Issue #3047709 by J.Gayathri, jhodgdon, spitzialist, anmolgoyal74,...
larowlan’s picture

Version: 8.9.x-dev » 8.8.x-dev
Status: Reviewed & tested by the community » Fixed

Committed bae8faf and pushed to 9.0.x. Thanks!

c/p to 8.9 and 8.8

thanks all

  • larowlan committed 428b2bb on 8.8.x
    Issue #3047709 by J.Gayathri, jhodgdon, spitzialist, anmolgoyal74,...
  • larowlan committed ee89209 on 8.9.x
    Issue #3047709 by J.Gayathri, jhodgdon, spitzialist, anmolgoyal74,...
Madhura BK’s picture

anmolgoyal74’s picture

It's good to see this feature moving forward.
Thanks all.

Status: Fixed » Closed (fixed)

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