Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Background:
This issue is part of the task to update the hook_help texts of the Drupal 8 modules:
#1908570: [meta] Update or create hook_help() texts for D8 core modules
Tasks:
- review / write the hook_help text according to help guidelines
- 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.
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | block-help-text-2029732-11.patch | 5.38 KB | drupaldrop |
| #9 | block-help-text-2029732-9.patch | 5.36 KB | ifrik |
Comments
Comment #1
xjmSee also: #2062761: Update hook_help() for custom_block modules
Comment #2
xjmNote that #2055853: [meta] Improve the place block UX; Separate interaction from the create block UX; Improve the existing blocks-by-theme layout and child issue #2058321: Move the 'place block' UI into the block listing remove one of the new screens.
Comment #3
ifrikThis is already dealt with in #2062761: Update hook_help() for custom_block modules
Comment #4
jhodgdonI 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.
Comment #5
ifrikRe-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.
Comment #6
ifrikI'm working on this today during the contribution sprint.
Comment #7
ifrikI'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.
Comment #8
jhodgdonThanks -- 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.
Comment #9
ifrikThanks 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')Comment #10
jhodgdonThanks! 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).
Comment #10.0
jhodgdonadded link back to the meta issue
Comment #11
nitvirus CreditAttribution: nitvirus commentedTried 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.
Comment #12
drupaldrop CreditAttribution: drupaldrop commentedAdding a new patch ... Needs Review
Comment #13
Mukeysh CreditAttribution: Mukeysh commented12: block-help-text-2029732-11.patch queued for re-testing.
Comment #14
ifrik@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?
Comment #15
drupaldrop CreditAttribution: drupaldrop commented@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.
Comment #16
jhodgdonYeah, 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.
Comment #17
ekes CreditAttribution: ekes commented✓ 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.
Comment #18
jhodgdonThanks 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.
Comment #19
jhodgdon12: block-help-text-2029732-11.patch queued for re-testing.
Comment #21
jhodgdon12: block-help-text-2029732-11.patch queued for re-testing.
Comment #22
jhodgdonTest bot fail... assuming this time it works, back to RTBC.
Comment #23
jhodgdonThanks again all! Committed to 8.x.