The release notes for Drupal 8.1.0 indicates an " Improved site administration experience: * Improved admin/help page to be more flexible and list tours on it."
The Help text standard (for core and contrib) gives a detailed account of how to do this the 'drupal way'.
I've been working on this for a few other modules and should be able to make quick work of getting this in for Video Embed Field, so I've assigned this issue to myself to work on. The way I've done it elsewhere is to implement each aspect (1-4) as a separate patch/issue. We'll see how that approach works here.
This is a followup to #2716941: Add README file which is a small stepping stone to improving HELP documentation for this module.
Comment | File | Size | Author |
---|---|---|---|
#21 | 2716967-hook-help-21.patch | 1.59 KB | Sam152 |
#18 | 2716967-Add_Video_Embed_Field_HELP-18.patch | 2.41 KB | dbt102 |
#15 | Add_Video_Embed_Field_HELP-2716967-4.patch | 3.71 KB | Sam152 |
#4 | Add_Video_Embed_Field_HELP-2716967-4.patch | 3.71 KB | dbt102 |
Comments
Comment #2
dbt102 CreditAttribution: dbt102 commentedComment #3
dbt102 CreditAttribution: dbt102 commentedSo, I mentioned above I'd work thru items 1-4. I'll just list them here and comment on each as kind of my review checklist before submitting the patch...
<1. Short description of what the module does. It is displayed on the Extend or Modules page (in Drupal 8 or 7). It is the only texts users will see if the module is not enabled yet.
^-- N/A ... this is OK, the description already exists and looks good to me
2. Description on links are displayed with the links on the Configuration and Structure pages and invite users to do something.^-- N/A ...there is really no Configuration and Structure pages per say, for this module, so this section does not apply.
<3. Explanations on the administration pages. Ideally this should not be needed, but if they do they are short and do not duplicate the help page.^-- N/A ... The existing explaination/descriptions on the permisions admin pages at /admin/people/permissions#module-video_embed_field look good to me.
4. Help page displayed by the Help module with three sections: What does the module do, what can users do with it, and a link to the online documentation here on drupal.org. This hook_help() text is in the my_module.module file.^-- This is the scope of the patch #4 included below. Also, I added an "about" type documentation page as part of this update, just to kind of orient new users to what this module does before diving into the more detailed config page that follows
This checklist reviews the new HELP work added for the main Video Embed Field module. The two additional sub-modules (Video Embed Media and Video Embed WYSIWYG
) will be addressed as separate issues.
NOTE: Once these HELP components are in place we anticipate adding on TOUR functionality as a followup issue.
Comment #4
dbt102 CreditAttribution: dbt102 commentedComment #5
dbt102 CreditAttribution: dbt102 commentedComment #9
Sam152 CreditAttribution: Sam152 at PreviousNext commentedThese fails are likely in HEAD. media entity got a new release which likely has failed the upgrade path test.
Comment #11
Sam152 CreditAttribution: Sam152 at PreviousNext commentedNot sure about copying the providers into the hook help? A link to the project page seems like enough? I don't fancy the idea of duplicating information between the README, project page and hook help. Lets keep hook help entirely non-technical. I don't see it as a tool for developers.
Comment #12
dbt102 CreditAttribution: dbt102 commentedThanks @sam152,
For what its worth, I tested and retested to make sure the patch applied correctly at least on my local.
Thanks for the work on a great module!
Comment #13
dbt102 CreditAttribution: dbt102 commentedYou are right of course, but we have a couple customers that will use this HELP a lot. So I thought I'd throw it in.
Just trying to make it easy for the very novice user to see that there are many options they can come back to dev's to get implemented.
Comment #14
dbt102 CreditAttribution: dbt102 commentedUnassigning the task from myself so others won't shy away from testing this patch
Comment #15
Sam152 CreditAttribution: Sam152 at PreviousNext commentedComment #16
Sam152 CreditAttribution: Sam152 at PreviousNext commentedUnrelated change?
Where is this closed?
This doesn't seem like it adds much value?
I don't feel like maintaining this twice is sane.
Video Embed Field case inconsistency.
Comment #17
dbt102 CreditAttribution: dbt102 commentedThanks for the great feedback sam152
assigning this back to myself to work on
Comment #18
dbt102 CreditAttribution: dbt102 commentednotes:
1. Unrelated change? --> yep, changed back
2. Where is this closed? --> oops ... added closing statement
3. This doesn't seem like it adds much value? --> agreed
4. I don't feel like maintaining this twice is sane. --> no problem ... removed
5. Video Embed Field case inconsistency. --> yep, the proper noun is "Video Embed Field" ... fixed
Comment #19
dbt102 CreditAttribution: dbt102 commentedComment #20
dbt102 CreditAttribution: dbt102 commentedjust checking in to see if there are any other changes you want to see made for this. At this point, for D8 modules, I think any HELP is better than none at all.
Comment #21
Sam152 CreditAttribution: Sam152 at PreviousNext commentedSorry for the hold up. I've made some necasary changes to this:
Comment #23
Sam152 CreditAttribution: Sam152 at PreviousNext commented