Problem/Motivation

While the README file is fairly minimal, it does exist (with a link to the online documentation), but this module is missing an implementation of hook_help().

Proposed resolution

Until something better can be provided, at least provide an implementation of hook_help() using the existing README file as the source.

Remaining tasks

  1. Fix the issue
  2. Create a patch
  3. Review the patch
  4. Commit the patch

User interface changes

There will be an entry for this module on Drupal's Help page (/admin/help).

API changes

None.

Data model changes

None.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

oadaeh created an issue. See original summary.

oadaeh’s picture

Assigned: oadaeh » Unassigned
Issue summary: View changes
Status: Active » Needs review
FileSize
1.54 KB

Attached is a patch that addresses this issue.

dani3lr0se’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
162.21 KB

The patch applies cleanly in my local dev site. There was an issue with simplytest.me. This appears to be working as intended. In the code it shows "markdown" but it appears to be plain text? See screenshot. Otherwise, it's working and looks fine to me. Thanks for the patch.

oadaeh’s picture

Issue summary: View changes
moghingold’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
1.11 KB

The patch in #2 goes about this in an interesting way, but a more succinct help summary that links to the project page for more info, doesn't access the file system, and allows the paragraph to be styled by the site's administration theme might be preferable. I have attached a less complex patch for review.

dani3lr0se’s picture

Status: Needs review » Reviewed & tested by the community

Patch applies cleanly. I like this one as well. I agree that this is more succinct and adding a t() also makes it translatable, which is nice to have.

joelpittet’s picture

Status: Reviewed & tested by the community » Needs work

Thank you for the patch, I've a few things that need to be taken care of before this is ready.

  1. +++ b/views_bulk_operations.module
    @@ -11,6 +11,22 @@ define('VBO_ACCESS_OP_UPDATE',    0x02);
    +  // Main module help for the block module
    

    Nit: needs a period at the end but maybe just remove this comment alltogether.

  2. +++ b/views_bulk_operations.module
    @@ -11,6 +11,22 @@ define('VBO_ACCESS_OP_UPDATE',    0x02);
    +  case 'admin/help#views_bulk_operations':
    

    This is weird but I guess this is how it works, should be indented though.

  3. +++ b/views_bulk_operations.module
    @@ -11,6 +11,22 @@ define('VBO_ACCESS_OP_UPDATE',    0x02);
    +    $help_text = 'This module augments Views by allowing bulk operations to be executed on the displayed rows. It does so by showing a checkbox in front of each node, and adding a select box containing operations that can be applied. Drupal Core or Rules actions can be used.';
    +    $output .= t($help_text) . '</p><p>';
    +    $help_text = 'For more information, please visit the <a href="https://www.drupal.org/project/views_bulk_operations">official project page on Drupal.org</a>';
    +    $output .= t($help_text) . '</p>';
    

    Variables shouldn't be added to the t() function. The translation tools parse the PHP looking for t() function calls and rip out the string in them.

    https://api.drupal.org/api/drupal/includes!bootstrap.inc/function/t/7.x#...

    Also the URL would be better passed as a token like @url because it's not really translatable and wouldn't want a path change to trigger translators to rewrite the text.

dani3lr0se’s picture

Ah, dang. Sorry, I didn't realize that about translating variables. Thanks for the helpful link as well @joelpittet.

joelpittet’s picture

No worries, thanks for reviewing this @daniel_rose, glad to see a mentor in my queue!