Problem/Motivation

The help text of the Place block module does not follow the Help text standards as described in https://www.drupal.org/help-text-standards.
Also the description on the Extend page, does not follow the standard and it should not contain the word "Drupal"

The use of Drupal::url() has been deprecated and needs to be replaced as well.
See #2868989: Replace calls of the deprecated Drupal::url() from update.module for details.

Proposed resolution

Editing the text according to the standard.

Remaining tasks

  • Edit the headers in the Uses section
  • Edit the Uses descriptions
  • Add the relevant links is the Uses descriptions
  • Edit the module description used on the Extend page.
  • Replace Drupal::url()

User interface changes

This is a UI text change

CommentFileSizeAuthor
#34 interdiff-29-34.txt2.4 KBgaurav.kapoor
#34 2830833-edit_hook_help_text_for-34.patch2.71 KBgaurav.kapoor
#29 interdiff_2830833_23-29.txt2.31 KBJacobSanford
#29 2830833-edit_hook_help_text_for-29.patch2.34 KBJacobSanford
#23 interdiff_2830833_18-23.txt2.42 KBJacobSanford
#23 2830833-edit_hook_help_text_for-23.patch2.45 KBJacobSanford
#18 interdiff_17-18.txt1.65 KBdhruveshdtripathi
#18 edit_hook_help_text_for-2830833-18.patch2.56 KBdhruveshdtripathi
#17 2830833-17-place-block-module.patch2.57 KBpepegarciag
#17 interdiff.txt1.54 KBpepegarciag
#10 2830833-10-place-block-module.patch2.39 KBprash_98
#6 2830833-edit-help-text-place-block-module-6.patch3.71 KBshailesh.bhosale
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ifrik created an issue. See original summary.

ifrik’s picture

ifrik
she/her
CreditAttribution: ifrik commented
Component: block.module » documentation
Issue tags: +Documentation, +user interface text
Feuerwagen’s picture

Feuerwagen
he/him
Primary language German
Location Bonn 🇩🇪🇪🇺
CreditAttribution: Feuerwagen as a volunteer commented
Title: Edit hook_help text for Content Moderation module » Edit hook_help text for Place Block module
Feuerwagen’s picture

Feuerwagen
he/him
Primary language German
Location Bonn 🇩🇪🇪🇺
CreditAttribution: Feuerwagen as a volunteer commented
Title: Edit hook_help text for Place Block module » Edit hook_help text for Place block module
shailesh.bhosale’s picture

shailesh.bhosale
Location Pune
CreditAttribution: shailesh.bhosale as a volunteer and at Faichi Solutions Pvt Ltd for Faichi Solutions Pvt Ltd commented
Assigned: Unassigned » shailesh.bhosale
shailesh.bhosale’s picture

shailesh.bhosale
Location Pune
CreditAttribution: shailesh.bhosale as a volunteer and at Faichi Solutions Pvt Ltd for Faichi Solutions Pvt Ltd commented
Assigned: shailesh.bhosale » Unassigned
Status: Active » Needs review
FileSize
2830833-edit-help-text-place-block-module-6.patch3.71 KB

Hi @ifrik,

I have updated the help text. Please review the patch.

Thanks.

Status: Needs review » Needs work

The last submitted patch, 6: 2830833-edit-help-text-place-block-module-6.patch, failed testing.

shailesh.bhosale’s picture

shailesh.bhosale
Location Pune
CreditAttribution: shailesh.bhosale as a volunteer and at Faichi Solutions Pvt Ltd for Faichi Solutions Pvt Ltd commented
Status: Needs work » Needs review

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

prash_98’s picture

prash_98 CreditAttribution: prash_98 commented
FileSize
2830833-10-place-block-module.patch2.39 KB

Uploading the patch for review.

prash_98’s picture

prash_98 CreditAttribution: prash_98 commented

Any updates on this issue ?

ifrik’s picture

ifrik
she/her
CreditAttribution: ifrik commented
Issue tags: +DevDays Seville
ifrik’s picture

ifrik
she/her
CreditAttribution: ifrik commented
Status: Needs review » Needs work

Prash_98,

please provide an interdiff for review: https://www.drupal.org/documentation/git/interdiff

And please stop renaming patches. If your work is based on somebody elses initial patch, then please keep the patch name so that others know what's going on. Otherwise it looks like you are discarding the work previously done.

prash_98’s picture

prash_98 CreditAttribution: prash_98 commented

@ifrik
Will keep that in mind and like should I provide the interdiff for this one or is there some more work needed on it?

DuaelFr’s picture

DuaelFr
he/him
Primary language French
Location Montpellier, France
CreditAttribution: DuaelFr as a volunteer and at Happyculture commented
Issue tags: -DevDays Seville +DevDaysSeville

Fixing event tag :)

pepegarciag’s picture

pepegarciag CreditAttribution: pepegarciag at La Drupalera by Emergya commented

There is something to work here? Or are we just waiting to the interdiff to review it?

pepegarciag’s picture

pepegarciag CreditAttribution: pepegarciag at La Drupalera by Emergya commented
FileSize
interdiff.txt1.54 KB
2830833-17-place-block-module.patch2.57 KB

Rework the Uses section with a description list.
Also I have removed some phrase used twice, and leave it only in the description section because it seems to fit there.

dhruveshdtripathi’s picture

dhruveshdtripathi CreditAttribution: dhruveshdtripathi at DevsAdda for OpenSense Labs commented
Status: Needs work » Needs review
FileSize
edit_hook_help_text_for-2830833-18.patch2.56 KB
interdiff_17-18.txt1.65 KB

WHile applying the last patch its was giving a whitespace warning, so removed the whitespace. Also changed '>>' to '>' in the path. Interdiff added.

Aanal.addweb’s picture

Aanal.addweb CreditAttribution: Aanal.addweb at AddWeb Solution Pvt. Ltd. commented
Status: Needs review » Needs work

Hi @dhruveshdtripathi, Thanks for the patch but still some changes were needed for this file
1) Change "To add a block : Administer > Structure > Blocks > Add."
to Administration > Structure > Blocks > Add
2) link of " online documentation for the Place Blocks module." is not working i think it should redirect us on https://www.drupal.org/docs/8/core/modules/block-place/overview

ifrik’s picture

ifrik
she/her
CreditAttribution: ifrik commented

I think you are looking at the wrong module here. This is should be the help text for the Place Block module, not the Block module.

prash_98’s picture

prash_98 CreditAttribution: prash_98 at Google Summer of Code commented

So does the above patch require any work?

ifrik’s picture

ifrik
she/her
CreditAttribution: ifrik commented

Yes, it needs work because the text is for the wrong module.

JacobSanford’s picture

JacobSanford
Primary language English
Location Fredericton 🇨🇦
CreditAttribution: JacobSanford commented
FileSize
2830833-edit_hook_help_text_for-23.patch2.45 KB
interdiff_2830833_18-23.txt2.42 KB

Updated text and removed any description and references that describe block module instead of block_place.

JacobSanford’s picture

JacobSanford
Primary language English
Location Fredericton 🇨🇦
CreditAttribution: JacobSanford commented
Status: Needs work » Needs review

The last submitted patch, 17: 2830833-17-place-block-module.patch, failed testing.

Aanal.addweb’s picture

Aanal.addweb CreditAttribution: Aanal.addweb at AddWeb Solution Pvt. Ltd. commented
Status: Needs review » Reviewed & tested by the community

@JacobSanford, Thanks for the patch it works well for me, the link & content change were displayed correctly.

ifrik’s picture

ifrik
she/her
CreditAttribution: ifrik commented
Status: Reviewed & tested by the community » Needs work

Sorry, but this text does not follow the help text standards as outlined, and the other remaining tasks as described in the issue summary are also still open.

dhwani.addweb, before you RTBC any issue please check the issue summary. This is not only about whether a patch works but whether it does what has been outlined as needs doing.

JacobSanford’s picture

JacobSanford
Primary language English
Location Fredericton 🇨🇦
CreditAttribution: JacobSanford commented

Apologies for the drive-by patch help. I had a few moments of free time last night and made an incorrect assumption that the review changes in #19 were all that was needed. I will resubmit.

JacobSanford’s picture

JacobSanford
Primary language English
Location Fredericton 🇨🇦
CreditAttribution: JacobSanford commented
Status: Needs work » Needs review
FileSize
2830833-edit_hook_help_text_for-29.patch2.34 KB
interdiff_2830833_23-29.txt2.31 KB

The enclosed addresses the "Remaining tasks" items, aside from:

  • Add the relevant links is the Uses descriptions

As there are IMHO no relevant links - the single case I created in the 'Uses' section only references the administration toolbar and contextual placement of the 'plus symbol', not routes or external links.

I've rewritten quite a bit of the help text to align with brevity and standards.

ifrik’s picture

ifrik
she/her
CreditAttribution: ifrik commented
Issue summary: View changes

The use of Drupal::url() has been deprecated and needs to be replaced as well.
See #2868989: Replace calls of the deprecated Drupal::url() from update.module for details.

ifrik’s picture

ifrik
she/her
CreditAttribution: ifrik commented
Issue summary: View changes
ifrik’s picture

ifrik
she/her
CreditAttribution: ifrik commented
Issue summary: View changes
ifrik’s picture

ifrik
she/her
CreditAttribution: ifrik commented
Status: Needs review » Needs work

Thanks JacobSandford,

this looks pretty good.

(a) The "from" can be a bit misleading, because you can also access the Block Layout page from any page.
(b) The note that you can't place a block on an admin page is good, but it should be plural. Maybe The Place Blocks module allows you to insert blocks directly on any page, except on administration pages.
In this case, the description in the info file needs to be changed as well.

(c) A nitpick: "by clicking the Place Block link": Labels should always be exactly as the user sees them so block should be with a small "b"

(d) We can tighten up the text a bit by leaving out the "will": "All theme regions then show..." and "Clicking a plus sign button in a region opens..."

(e) I think we need another Use about rearranging the order of blocks. I'm not sure whether it is the desired behaviour that after adding a block the user then is send to the Block layout page, where they can arrange the order of blocks before returning to the page. Something like Arranging the order: After adding a block, you see the Block layout page, where -if required - you can reorder the blocks in a region and save this new order. You can return to your original page by clicking on the "Back to site" link in the admin tool bar.
In this case, the Block layout page would need a link.

gaurav.kapoor’s picture

gaurav.kapoor CreditAttribution: gaurav.kapoor at OpenSense Labs commented
Status: Needs work » Needs review
FileSize
2830833-edit_hook_help_text_for-34.patch2.71 KB
interdiff-29-34.txt2.4 KB
prash_98’s picture

prash_98 CreditAttribution: prash_98 at Google Summer of Code commented
Status: Needs review » Reviewed & tested by the community

This patch applies well and also I guess it is apt now. So changing to RTBC.

ifrik’s picture

ifrik
she/her
CreditAttribution: ifrik commented
Status: Reviewed & tested by the community » Needs work

Sorry, this patch is not yet ready for RTBC.

@guarav.kapoor: if you only address some of the points raised in a comment please say so when you post a patch, because it clearly needed more work.

@prash_98: The issue summary clearly lists the remaining tasks which are still open. RTBC is not about whether a patch applies - that's already covered by the automated tests. It is about whether the issue raised in the issue summary is actually fixed.

@jacobSandford: If you want to continue working on this issue, then taking it from the patch in #29, would be fine.

gaurav.kapoor’s picture

gaurav.kapoor CreditAttribution: gaurav.kapoor at OpenSense Labs commented

My bad , should have mentioned what i have fixed. It's a 3 months old patch , I'll check again and upload a new patch.
@ifrik What do you think is still left in it.

prash_98’s picture

prash_98 CreditAttribution: prash_98 at Google Summer of Code commented

@ifrik I thought all the things were fixed. Sorry and Will keep it in mind :)

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

ifrik’s picture

ifrik
she/her
CreditAttribution: ifrik commented
Status: Needs work » Closed (outdated)

The Place Block module got depreciated so therefore we don't need a help text.