[D7] Icon Tabs

1) You should implement hook_uninstall() and remove all variables. Otherwise they will reside in the database after uninstalling.

2) You should make the git branch 7.x as the default if it is not already, and remove the 'master' branch.

3) You should prefix all functions and Drupal variables with 'icon_tabs', without underscores. You are creating a static variable called icon_tabs, I would call it icon_tabs_something.

4) I would try to keep lines short, I tend to use ? (ternary) conditionals only on very simple things, because otherwise I find the code is more difficult to follow (compared to an 'if/else'). When concatenating strings, for example, I would not use them.

5) I would state which browsers are supported in the README and module page.

6) You can see a more detailed an automated review of your code here: http://pareview.sh/pareview/httpgitdrupalorgsandboxfredparke2116761git
In the link above there are some links to the documentation. Unless there's a good reason to not do it, you should fix all issues reported there. You can run http://pareview.sh/ as many times as you want

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.

1) In icon_tabs_preprocess_html() you are adding the icon-contextual-links.css to every page, regardless of whether the user has access to contextual links and they actually appear on the page. You should do an additional check to see whether the css is required. At the very least, it could look like this:

function icon_tabs_preprocess_html(&$variables) {
  if (user_access('access contextual links') && variable_get('icon_tabs_contextual_links', FALSE)) {
    drupal_add_css(drupal_get_path('module', 'icon_tabs') . '/css/icon-contextual-links.css');
  }
}

2) Looks as though this module used to be called fred_tabs, there are still a few references to it. E.g. icon_tabs.module line 66

$form['contextual']['fred_contextual_links'] = array()

3) There are still Drupal coding errors (see http://pareview.sh/pareview/httpgitdrupalorgsandboxfredparke2116761git). However, these appear to be all from font-face declarations inside of css, so there is actually nothing that could be changed.

Looks good to me

pareview.sh found one error -

http://pareview.sh/pareview/httpgitdrupalorgsandboxfredparke2116761git

"./font/License.txt: ASCII English text, with no line terminators"

pareview.sh is throwing up a bunch of new errors in your CSS files.

I've installed the module and followed the instructions and all appears to work as described. Unable to find any issues with the functionality.

Cute module - I'd be interested to see where this goes!

After reviewing your module let me start by saying, awesome concept, I love icon fonts, I could definitely see myself using this module further down the line.

There are some small issues I found when I installed the module:

1. There is a typos on admin/config/user-interface/icon-tabs - vertically is misspelled as "Veritally stacked tabs"
2. "Vertically stacked tabs" does not take affect when the box is checked.

Also, I have a question...
Does the "Vertically stacked tabs" checkbox have to be dependent on the "Fred tabs" option? It would be nice to be able to stack the tabs vertically on their own.

I also found a small issue with the text boxes on content/hello-form#overlay=admin/config/user-interface/icon-tabs. right now they're set to a width of 60. this makes them overflow outside of their container when shrinking the width of the browser.

also i cannot seem to make the tabs vertically aligned no matter how much shrink my browser width (using Firefox 26.0)

on line 104 you left dpm($vertical_width) active.

also i get the following errors when trying to go to the icon tabs admin page

Notice: Undefined index: icon_tabs_admin in drupal_retrieve_form() (line 763 of path\includes\form.inc).

Warning: call_user_func_array() expects parameter 1 to be a valid callback, function 'icon_tabs_admin' not found or invalid function name in drupal_retrieve_form() (line 798 of path\includes\form.inc).

all fixed up, nice going. I think you're about there, as far as I'm concerned.

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

• Remove all .DS_Store files from your repository.
• DrupalPractice has found some issues with your code, but could be false positives.
  

  
  FILE: /home/klausi/pareview_temp/icon_tabs.admin.inc
  --------------------------------------------------------------------------------
  FOUND 0 ERROR(S) AND 1 WARNING(S) AFFECTING 1 LINE(S)
  --------------------------------------------------------------------------------
   89 | WARNING | Do not call theme functions directly, use theme('textfield',
      |         | ...) instead
  --------------------------------------------------------------------------------
  

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. icon_tabs_menu(): the permission "administer content" does not really fit ... maybe create your own?
2. "if (arg(0) == 'admin') {": use path_is_admin() instead.
3. icon_tabs_block_contents(): don't call theme() here, just return the render array. Drupal will render it later for you. Also elsewhere, avoid calling theme() early if you don't have to. Drupal is very capable of rendering nested arrays.

But otherwise looks good to me. Usually you should not RTBC your own issues, but it seems like irodriguez was about to set this anyway, so ...

Thanks for your contribution, freddybushboy!

I updated your account so you can 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 stay 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.