Horizontal Accordion

welcome,

You are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.

As installation and usage instructions are quite important for us to review, please take a moment to make your project page follow the tips for a great project page. Also create a README.txt that follows the guidelines for in-project documentation.

An automated review of your project has found some issues with your code; As coding standards make sure projects are coded in a consistent style we please you to have a look at the report and try to fix them. Anyway, note that issues found are possibly false positives and fixing all issues is not a requirement for getting through the application process.
Report: http://ventral.org/pareview/httpgitdrupalorgsandboxukan1700574git

There are about 100 other applications waiting for review and only a hand full of active reviewers.
We do really need more hands in the application queue and highly recommend to get a review bonus so we can come back to your application sooner.

regards

Hi Quentin,

This is an interesting module, I think when a few adjustments are made, it will be very useful!

Critical License Issue

1. The liteAccordion code is not compatible with Drupal's GPL license (the liteaccordion library uses an MIT license). So you cannot bundle the code with your module, instead it must be downloaded separately into a libraries directory and you can use the Libraries API to include it. Here's a module developer's guide to using the Libraries API:
http://drupal.org/node/735160

Functionality:

2. The form for adding an element to the horizontal accordion was confusing. There is no label on the first textfield, I had no idea what that field would be used for. I would suggest adding: "title" or something as the field label.

3. The image functionality didn't work, I attached a screen shot of the error message. There was no file upload form element on the "new element" form. Perhaps there is another module that is required for this functionality to work?

Thanks,
Bart

Your reviews of other projects were quite short. When finishing your review comment also set the issue status either to "needs work" (you found some problems with the project) or "reviewed & tested by the community" (you found no flaws).

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:

• 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. project page is too short, see http://drupal.org/node/997024
2. there are a lot of existing accordion modules: Views Horizontal Slider, views_accordion, block_accordion, menu_accordion etc. Please document exactly what the differences to that modules are on you project page.
3. Maybe you should rename your module to liteaccordion?
4. I think the library path should be liteaccordion, not littleaccordion.
5. "'access arguments' => array(''),": this should be a valid permission name like "administer site configuration".
6. "'description' => 'Configuration de Horizontal Accordion',": all user facing text must be in English in the source code.
7. "'access callback' => TRUE,": security: anyone can edit the configuration?

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

Looks like a nice module :)

Here are some remarks:

1) I found a lot of lines exceed the the advised limit of 80 characters per line.

2) Typo in horizontal_accordion.info, line 5:
libraires should be: 'libraries'

3) horizontal_accordion.admin.inc, lines: 256, 364, 407:
$form_state is missing &

4) horizontal_accordion.module, line 84:
Suggestion for a more simple/readable code:

function horizontal_accordion_get_option($option_name) {
  $options = variable_get(
    'horizontal_accordion_options',
    array(
      'activateOn' => 'click',
      'autoPlay' => false,
      'easing' => 'swing',
      'enumerateSlides' => false,
      'firstSlide' => 'first',
      'headerWidth' => '48',
      'height' => 180,
      'sizingType' => 'tab_size',
      'theme' => 'basic',
      'width' => 600,  
    )
  );

  if (isset($options[$option_name])) {
    return $options[$option_name];
  }

  drupal_set_message(
    t(
      'You requested the value of a variable that doesn\'t exist : @option_name',
      array('@option_name' => $option_name)
    ),
    'warning'
  );
  return FALSE;

}

5) horizontal_accordion.module, line 141:
I think it better to avoid inline js if possible. It gives a cleaner HTML (search engines like that) and improves performance.
I would create a js file and store the js variables in the Drupal.settings namespace.

6) horizontal_accordion.module, line 117:
In the horizontal_accordion_block_view you are mixing code and HTML. I suggest creating theme function(s) to get a better overview.

7) horizontal_accordion.module, line 172:
Instead concatenate a string into an ordered list I suggest you build up an array of rows and render it through theme_item_list.

Adding reviews

Sorry for the delay. I think you forgot to add the "PAReview: review bonus" tag as explained in http://drupal.org/node/1410826 , so you didn't show up on my high priority list.

manual review:

1. "'description' => 'Modifier l\'accordéon horizontal',": all text in the source code should be English. Please check all your code.
2. horizontal_accordion_edit_specific_element(): never use the raw $form_state['input'], use $form_state['values'] instead.
3. horizontal_accordion_edit_specific_element(): do not use arg(4), the URL part is passed as parameter to the function and you should use that.
4. horizontal_accordion_menu(): admin/config/media/horizontal_accordion and admin/structure/horizontal_accordion are redundant, onw should be removed?
5. Notice: Undefined index: #elements in horizontal_accordion_edit_elements_form_validate() (line 199 of horizontal_accordion.admin.inc).
6. Warning: Invalid argument supplied for foreach() in horizontal_accordion_edit_elements_form_validate() (line 199 of horizontal_accordion.admin.inc).
7. horizontal_accordion_edit_elements_form(): by using system_settings_form() you are polluting the variable table with an "options_fieldset" variable. You should re-name your form elements accordingly so that system_settings_form() works correctly.
8. horizontal_accordion_edit_elements_form_validate(): do not save stuff in validation functions. That's what submit callbacks are for.
9. horizontal_accordion_edit_elements_form_validate(): why do you need the foreach loop? You could just use array_values()?
10. horizontal_accordion_save_options(): why do you need the global variable? You should avoid global variables where possible.
11. horizontal_accordion_get_content(): why is this function needed if it is just one line using variable_get()?
12. horizontal_accordion_get_option(): variable_get() is statically cached anyway, so no need for the global variable.
13. The variable horizontal_accordion_elements is always empty for me, how do I get the elements to show up above the advanced settings on the configuration page.

Review bonus tag is removed already, you can add it again if you have done another 3 reviews of other projects.

In the .css file margin-left and right are used. Drupal supports conditional loading of CSS files with specific override rules for right-to-left languages. See http://drupal.org/node/302199.

Good job, happy coding.

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

doesnt work for me. there no site to add tabs or somthing. only twice config sites

Adding a review