Simple Pin Map

Welcome,

please get a review bonus first. Then try to fix issues raised by automated review tools: http://ventral.org/pareview/httpgitdrupalorgsandboxsergiodrupal1784066git

1. We have installed the module, added a content, uploaded the map image file, but the section "Click on the Map to place the pin" does not show the image (see the attached screenshot). There is only a small grey area.
2. After uninstalling the module the content type "Simple Pin Map" is not deleted from the menu "Add content".

it's good module and i look forward to using it.
I installed it and it works very well for my responsive theme. Fill out the to-do mentioned for administration interface for the image setup which is hardcoded now and this module should be in drupal contrib list.

It will be very useful module. But... ;)
1. Please change module package in .info file. I think "Blocks" or "User Interface" will be much better, instead of "logicdesign.be". Or you can simply remove it from .info file, and your module will be displayed in "Other" package.
2. Don't use jQuery(document).ready. You need to change JavaScript behaviors. Use

(function ($) {
  Drupal.behaviors.initSimplePinMap = {
    attach: function (context) {
    ...
    }
  }
})(jQuery);

3. Also, you can place module settings on block configure page. See hook_block_configure()

Some minor bugs:
1. In "simplepinmap-edit.js" needs to add click event for #simplepinmap-marker. If i want to move the pin on a few pixels to the side, then it will not work. Or you can add ui-draggable library this will allow to drag the pin. Add this to form:

    '#attached' => array(
      'library' => array(
        array('system', 'ui.core'),
        array('system', 'ui.draggable'),
      ),
    );

and use $('#simplepinmap-marker').draggable({ ... });
2. On configure page i would like to be able to select or upload icons for the pin.

forgot

I think 3 or 4 different pin pairs will be enough.

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. You have to get a review bonus to get a review from me.

manual review:

1. I image there are a lot of existing module out there that alredy do similar stuff. Please add the differences to those module to your project page, so that users can make educated decision when choosing modules.
2. simple_pin_map_install(): wrong doc block, this is a hook and should be documented as such. See http://drupal.org/node/1354#hookimpl . Same for simple_pin_map_uninstall().
3. simple_pin_map_install(): No need to set variables upon installation, you can use default values with variable_get() anyway.
4. _simple_pin_map_installed_fields(): doc block is wrong, this function does not install anything by itself? See http://drupal.org/node/1354#functions on how to document functions.
5. Does your module only work for europe? Should that be added to the project page?
6. _simple_pin_map_get_content(): the node query does not take node access grants into account, so node access modules might have no effect. When adding a node listing to your module, be sure to use a dynamic query created by db_select() and add a tag of "node_access". This will allow modules dealing with node access to ensure only nodes to which the user has access are retrieved, through the use of hook_query_TAG_alter(). http://api.drupal.org/api/drupal/modules!node!node.module/group/node_acc...
7. _simple_pin_map_get_content(): you could use node_load_multiple() instead of singe node_load()s.
8. _simple_pin_map_get_content(): this is vulnerable to XSS exploits. The node title is user provided input and needs to be sanitized before printing into HTML. See http://drupal.org/node/28984 . Please also check the other variables used in this function and whether the field content has been sanitized already or not.
9. _simple_pin_map_get_valid_markers(): do not create image markup yourself, use theme('image', ...) instead.
10. _simple_pin_map_get_valid_markers(): $base_path exists as global variable, why do you have to access $_SERVER['DOCUMENT_ROOT']?
11. simple_pin_map_admin_submit(): do not use the raw, unsanitized $form_state['input'], always use $form_state['values'] where possible.

Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

There are a whole bunch of tilde backup files under revision control. These should be deleted.

You should double check the CSS3 properties to see if you also need/want vendor prefixes to support people who may not have totally current browsers.

In you JS, you are using a closure like

(function ($) {
})(jQuery);

but you are then using jQuery everywhere instead of $.

Your JS anonymous objects, (eg simplepinmap-edit.js:5) have trailing commas after the final element. Older versions of IE don't like this.

You also need to do a pass and clean up some minor formatting in the JS.

It looks like you are creating five variables in the hook_install() but only deleting two in the hook_uninstall().

simple_pin_map.form.inc has a no-op function that should be removed.

The alt and titles being added to the image are unstranslated strings.

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. You have to get a review bonus to get a review from me.

manual review:

1. some functions do not use the correct "/**" style doc block, they use the inline "/*" (that is what pareview.sh complains about).
2. simple_pin_map_hook_info_alter(): why is this hook needed? Please add a comment.
3. simple_pin_map_form_alter(): empty function, so remove it.
4. "'title' => 'Edition marker',": all user facing text must run through t() for translation. Please check all your strings.
5. simple_pin_map_admin(): doc block is wrong, this is not a hook. See http://drupal.org/node/1354#forms on how to document forms.

Not hard blockers, so I guess this is RTBC. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Hi,

it seems that you are double-check-plaining the "simple plan map item content".
I get this as text when clicking on a pin in the block:

test
<p>tetstasdasd</p>

Sometimes, when editing the node, I was not able to set the pins position by clicking.
It just didn't move. I'm using chrome, no js errors occurred. Not sure what could be wrong here.
After reloading the page it worked again, maybe an issue with adding the js asset.

in urls I personally prefer dashes rather than underscores
admin/simple-pin-map/customize

It's okay to break long conditions, but some indenting would make it easier to read this code.

if ($delta == 'board_block'
 334   && isset($edit['simple_pin_map_marker_block_config'])
 335   && is_numeric($edit['simple_pin_map_marker_block_config'])
 336   && file_exists(

You usually do not set variables on installation. It's not an error but not the way it's designed for, best practices advice to use the default value.

The only real issue I see here is that your double-check-plaining the nodes contents on the block when clicking on a pin.

Beside this, thanks for your help in the application queue!

Thanks for your contribution!

I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.