Custom Add Block

http://ventral.org/pareview/httpgitdrupalorgsandboxwillisiw1846140git

There is still a master branch, make sure to set the correct default branch: http://drupal.org/node/1659588 . Then remove the master branch, see also step 6 and 7 in http://drupal.org/node/1127732
Review of the 7.x-1.x branch:

Hi willisiw,

The automated review picked up the following.

Line 47: String concatenation should be formatted with a space separating the operators (dot .) and the surrounding terms
      '#suffix' => '</td><td>' .  $previewimg . '</td></tr></table>',

Your configure link does not link to the correct location, admin/config/user-interface//custom_add_block instead of admin/config/user-interface/custom_add_block.

$imglink does not return the correct path for me. Would it be more appropriate for CSS to handle this?

Cheers,

Hi willisiw ,

Manual Review:

1) Use t() function in line no : 16, 17, 18, in admin.inc

2) There is no hook_uninstall to remove your custom variables from database like: custom_add_block_node_weight_, custom_add_block_node_icon_, etc. in admin.inc

3) There are some incomplete information in Readme.txt like: ' TROUBLESHOOTING' & ' FAQ '. If there is no info available till date, you can remove the headings.

Manual review

README.txt
> change text "To use the slidedown block, the site must have jQuery."
drupal has jquery in core. please rewrite if a specific jQuery version needed or remove this line.

> also change "* After enabling the module, navigate to /?q=admin/config/user-interface/custom_add_block"
you may write this way. "Congfiguration > User interface > Custom add block"

custom_add_block.module
it will more helpfull to add hook_help() with detailed description "How to use"

custom_add_block.css
remove extra spaces after each group.

custom_add_block.js
please follow Behavior handling.

Hi willisiw,

Alright, I'm going to try to give you a little help on this.

First of all, in general, since you're working with Open Source code, I recommend you try to always look at how other modules and coders do it.
I'm sure you have been using some very popular modules such as Nice Menus, Administration menu, or any of the core modules, such as the Block module (on which custom_add_block module builds its functionalities), etc...
All of the modules mentioned above have JS files with the code and type of syntax you are looking for.

So your code would look more like:

(function ($) {

/**
 * TODO: This is an example:
 * Provide the toggle features for the custom_add_block_jquery
 */
Drupal.behaviors.customAddBlockJquery = {

  attach: function(context, settings) {
    $('#block-custom-add-block-custom-add-block-jquery').once('custom_add_block', function () {
      // When site loaded, hide the Popupbox First
    [...The rest of your JS code and logic should be here...]
  }
}

})(jQuery);

You will have to try to figure out what to do with your functions, how they could be declared and how you could rework slightly your JS code, but I guess this would simply require a little bit more testing.

I think if you have written this module and got as far already in the validation process and development of it, you should certainly be able to take down this last part for the JS code, which really doesn't need that much work or time. It is important that you understand how these files work because you may also be brought to using other modules that implement the same kind of code.

One last validation related advice would be for you to run the coding standards validation (PAReview) again, after you have committed your changes, to make sure you didn't introduce any new coding standards errors when you made the changes. After that, feel free to change the status back to needs review, that will save you one not so useful review from another user who would have simply told you about the coding errors mistakes (more back and forth = more time). This is something very common that happens to all of us (and just for one whitespace at the end of the line... the status is changed back to needs work again).

And it's exactly the case currently with your code, if you go and look at the Automatic Review (You've exactly got one whitespace over :-) ).


Automated Review: http://ventral.org/pareview/httpgitdrupalorgsandboxwillisiw1846140git

Review of the 7.x-1.x branch:

Drupal Code Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.
This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project.

FILE: ...view/sites/all/modules/pareview_temp/test_candidate/custom_add_block.js
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
35 | ERROR | Whitespace found at end of line
--------------------------------------------------------------------------------

Source: http://ventral.org/pareview - PAReview.sh online service

Lastly, if you really wanted to do things the best way, I would recommend you copy paste some of the different items I listed/detailed above and make them tickets in your sandbox project tracker. Then, when you commit you will be able to include a ticket number in your log message, as it's being done with most standard contributed modules (Views, Panels, Search API, etc...).
This is not needed for the validation of the module, but think about its future and potential community of developpers that would greatly appreciate going through your documented work if they came to use this module. When the module is approved, the commits will be visible to users along with the issues and all that, which would really get your module to look like a real/serious one.

Well done, nice work and I sincerely hope these comments help you improving this very nice module.

Cheers!

Hi willisiw,

Great! That's it! You got the custom_add_block.js figured out and everything seems to look fine with this part of the code now.

I haven't had the time to test it again on a sandbox though, but looking at the code, everything seems to be as expected, based on #7.

Automated code validation (PAReview) seems to be fine, as well.

If you have tested it again and everything is fine, then I would recommend you change the status to needs review and work on reviewing others' applications to get a #1410826: [META] Review bonus, since it seems you didn't get any so far.
That would allow adding the tag PAReview: review bonus to your application, which would get you on the fast lane for validation until you get the attention of official reviewers.

Feel free to let me know if you would have any more questions, comments or issues on any aspects of the code or previously discussed items, I would certainly be glad to help.

Thanks again for your comments and feedbacks.
Cheers!

Hi,

Some issues need to fix. see here:
http://ventral.org/pareview/httpgitdrupalorgsandboxwillisiw1846140git

Hi willisw,

I tested your module and reviewed the code.

Please use the t() function in .module for:

$block['subject'] = 'Add New Content'; in lines 75 and 106
$block['content'] = 'Custom Text';
$block['subject'] = 'Join Now';
$block['subject'] = 'Start Now';

and for the "Add New Content" string in:

$block['subject'] = '<div id="custom-add-block-load"><img src="' . base_path() . drupal_get_path('module', 'custom_add_block') . '/add.png' . '">Add New Content</div><div id="custom-add-block-close"><img src="' . base_path() . drupal_get_path('module', 'custom_add_block') . '/delete.png' . '"></div>';

Your module depends on the Block module. It must be enabled, and this must be stated in the .info file. Please add this dependency in the .info file. It must contain a directive like:
dependencies[] = block

The configuration instructions in the README.txt do not mention that there are 2 new blocks, which need to be activated. Please extend the instructions accordingly.

When I place the blocks in the left sidebar, they look like in the attached picture. Something with the CSS seems to be wrong.

Feel free to ask me, if you need details about my configuration.

Cheers
Martin

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

I'm a robot and this is an automated message from Project Applications Scraper.

Drupal 7