[D7] Sidr Responsive Menu

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxSwarad2092357git

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

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

PAReview: 3rd party code

jquery.sidr.min.js appears to be 3rd party code. 3rd party code is not generally allowed on Drupal.org and should be deleted. This policy is described in the getting involved handbook. It also appears in the terms and conditions you agreed to when you signed up for Git access, which you may want to re-read, to be sure you're not violating other terms.

The Libraries API module is a recommended method for adding 3rd party dependencies without directly including the code on Drupal.org.

Hi swarad07,
1. I have followed your README and have done as you have guided.
But not able to catch the functionality of your module, so can you please upload a screenshot showing the functionality?

2. Secondly, in the .info file, you need not explicitly declare the js and css files. Instead declare them in your .module file, See drupal_add_js() and drupal_add_css()

3.

function theme_sidr_menu() {

}

in your sidr_responsive_menu.module is empty. You should remove it if not required.

4. Before initializing $sidr_js, you should keep a check as:

if(module_exists('libraries') && function_exists('libraries_get_path')) {
  $sidr_js = libraries_get_path('sidr');
}

Comment me if I am wrong

Hi swarad07,
Generally the declaration of js and css are not encouraged in .info file as you donot have any control as to when the js and css should be invoked. If its solves your purpose with least complexity, then its alright.
However, I came across the function process_page function
See whether it can be of some use for this purpose. I would be happy to explore new stuffs with your module.

Removed review links that are not manual reviews of source code.

Review of the 7.x-1.x branch (commit dbb0631):

• Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
  

  FILE: /home/klausi/pareview_temp/sidr_responsive_menu.js
  ----------------------------------------------------------------------
  FOUND 4 ERRORS AFFECTING 4 LINES
  ----------------------------------------------------------------------
    2 | ERROR | [x] Expected 1 space(s) before asterisk; 0 found
    3 | ERROR | [x] Expected 1 space(s) before asterisk; 0 found
    4 | ERROR | [x] Expected 1 space(s) before asterisk; 0 found
   19 | ERROR | [x] Whitespace found at end of line
  ----------------------------------------------------------------------
  PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
  ----------------------------------------------------------------------
  

• ESLint has found some issues with your code (please check the JavaScript coding standards).

sidr_responsive_menu.js: line 6, col 2, Error - Use the function form of "use strict". (strict)
sidr_responsive_menu.js: line 19, col 7, Error - Trailing spaces not allowed. (no-trailing-spaces)
sidr_responsive_menu.js: line 20, col 32, Error - Missing space before function parentheses. (space-before-function-paren)
sidr_responsive_menu.js: line 20, col 34, Error - Missing space before opening brace. (space-before-blocks)

4 problems

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. project page is a bot short, see https://www.drupal.org/node/997024 for improving it.
2. sidr_responsive_menu_libraries_info(): the version callback is optional so you don't have to provide one and sidr_responsive_menu_bypass_version_check() can be removed.
3. sidr_responsive_menu_page_alter(): since you are not changing/altering anything in the page render array you should use hook_page_build() instead.
4. sidr_responsive_menu_page_alter(): do not call theme() or render() here, just build add to the nested render array. Drupal core will render it later for you. That way it is easier for other modules to alter the render array. See https://www.drupal.org/node/930760
5. "$url = $base_url . '/' . drupal_get_path('module', 'sidr_responsive_menu') . '/menu-icon.png';": do not create a URL like that, use url() instead.
6. "'access arguments' => array('administer sidr configuration'),": that permission is not defined with hook_permission()? Which means that currently only user 1 can ever edit your stuff on your admin page.
7. sidr_responsive_menu_page_alter(): this is vulnerable to XSS exploits. The link text is user provided text and needs to be sanitized before printing to HTML. Make sure to read https://www.drupal.org/node/28984 again. This is currently not a security issue because only the trusted user 1 can ever edit the link text in the first place, but this should be fixed anyway.

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

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

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