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
- review on line docs at https://drupal.org/documentation/modules/block

In D8, the administration of blocks is split over two different configuration pages. This should be clearer in the help text.
The text should be checked whether other possible changes from D7 are adequately reflected.

Files: 
CommentFileSizeAuthor
#12 block-help-text-2029732-11.patch5.38 KBdrupaldrop
PASSED: [[SimpleTest]]: [MySQL] 59,142 pass(es).
[ View ]
#9 block-help-text-2029732-9.patch5.36 KBifrik
PASSED: [[SimpleTest]]: [MySQL] 58,408 pass(es).
[ View ]

Comments

xjm’s picture

ifrik’s picture

Status:Active» Closed (duplicate)
jhodgdon’s picture

I think it's probably better to have two separate issues as they are two separate modules, but it's OK if you want to combine them... I've put a note there and changed the issue title.

ifrik’s picture

Status:Closed (duplicate)» Active

Re-opened after agreeing that #2062761: Update hook_help() for custom_block modules should only deal with custom blocks.
Some bit of help text can be taken from there.

ifrik’s picture

I'm working on this today during the contribution sprint.

ifrik’s picture

Status:Active» Needs review
StatusFileSize
new5.68 KB
PASSED: [[SimpleTest]]: [MySQL] 58,563 pass(es).
[ View ]

I've rewritten the blocks help page to reflect the changes in D8. It mentions the option to create custom blocks and then links to its help page.

jhodgdon’s picture

Status:Needs review» Needs work

Thanks -- this looks really good! A few small things I noticed:

a) "Toggling between different theme" [Uses heading] --> themes

b) Our Drupal content style guidelines require the use of a serial comma before and/or in a list. So:
"...restricting it to specific pages, content types and/or roles." ==>
restricting it to specific pages, content types, and/or roles

c) Maybe the generic item in Uses about configuring settings should come before visibility (which is a specific setting)? Then the Visibility item would not have to repeat the instructions about how to get into the configuration?

d) link on the  <a href="!blocks">... There is an extra space before the A tag.

e) I do not think we should provide any details about how to use the Custom Blocks module here. Just say you can add custom blocks if you have that module enabled, and link to its help.

ifrik’s picture

Status:Needs work» Needs review
StatusFileSize
new2.13 MB
new5.36 KB
PASSED: [[SimpleTest]]: [MySQL] 58,408 pass(es).
[ View ]

Thanks for the comment. All taken up.

I copied the link to the Extend page from another module. I hope it's correct like that:
\Drupal::url('system.modules_list')

jhodgdon’s picture

Issue tags:+Needs manual testing

Thanks! This looks good to me. We need someone to give it a manual test:
- Install the patch.
- Click all the links and verify they go to the right places.
- Verify that the text in the help matches what you see on the Blocks administration page (page titles, link text, etc. that is mentioned in the text should match exactly what you see).

jhodgdon’s picture

Issue summary:View changes

added link back to the meta issue

nitvirus’s picture

Tried applying the patch on fresh drupal8 install, but got errors :
warning: core/modules/block/block.module has type 100755, expected 100644
error: patch failed: core/modules/block/block.module:37
error: core/modules/block/block.module: patch does not apply

I thinks the patch needs to be made again.

drupaldrop’s picture

Issue summary:View changes
StatusFileSize
new5.38 KB
PASSED: [[SimpleTest]]: [MySQL] 59,142 pass(es).
[ View ]

Adding a new patch ... Needs Review

Mukeysh’s picture

12: block-help-text-2029732-11.patch queued for re-testing.

ifrik’s picture

@nitvirus: It looks like your D8 installation has different file permissions: 755 instead of 644 and that therefore the patch doesn't apply in for you.
@drupaldrop: What are the changes you made? Could you make an interdiff?

drupaldrop’s picture

@ifrik below are the steps i did
I downloaded a patch tried it and it doesn't apply i found the line number changes in the patch . i am not sure if this could be the reason so i updated a existing patch and i retried, it works after that

my mistake i should had wrote update exisiting patch instead of adding new patch in previous comment.

jhodgdon’s picture

Yeah, it looks like the new patch is a simple reroll of the old patch. Thanks for doing that drupaldrop!

The patch still needs manual testing. See #10.

ekes’s picture

Status:Needs review» Reviewed & tested by the community

✓ Applies (just used simpletest.me)
✓ Links all go to the appropriate pages.
✓ All text in italics is identical to the text on the block page. Other terms used, usually previously in italics, plus additional terms mentioned in 'Controlling visibility' are identical to text used on the block page.

jhodgdon’s picture

Thanks for the review! I'm going to click "retest" on the latest patch, just to make sure all the tests still pass since the last test was quite a while ago. Then it should be ready to commit.

jhodgdon’s picture

12: block-help-text-2029732-11.patch queued for re-testing.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 12: block-help-text-2029732-11.patch, failed testing.

jhodgdon’s picture

Status:Needs work» Needs review

12: block-help-text-2029732-11.patch queued for re-testing.

jhodgdon’s picture

Status:Needs review» Reviewed & tested by the community

Test bot fail... assuming this time it works, back to RTBC.

jhodgdon’s picture

Status:Reviewed & tested by the community» Fixed

Thanks again all! Committed to 8.x.

Status:Fixed» Closed (fixed)

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