[D7] Clientside hierarchical select

Hi,

I took a look at the module. PAreview is nice and clean, but I did find a few minor things to tidy up (Fixing some grammar in the documentation, and removing unneeded stuff in the CSS files - patch attached).

There was one issue I found when I tried the module on my test site. Here's what I found:

• I have a four level deep taxonomy (Great-grandfather, grandfather, father, boy)
• I enabled cshs for the field on a content type, and enabled "Force selection of deepest level" for the field
• I went to the node/add screen for my content type, and chose "Great-grandfather", "Grandfather", "Father" - ie 3 levels out of the 4
• I re-set the third level in the cshs widget back to "-Please choose-" - so that only two levels were selected
• I didn't enter any other fields (Specifically, I left the title field blank)
• I submitted the form.

Drupal reloaded the form (As expected), and flagged an error that "Title field is required.". It did *not* flag an error for the cshs field, and in fact, it reset all selections in that field, e.g. I only had one box, and it was set to "-Please choose-". It should have flagged an error for the cshs field, and maintained the levels that had been selected [What it does if you don't do the 4th step in the list above].

Hi,

I pulled down the latest version and there is still an issue with validation. Compare these two scenarios with the same setup as before:

Scenario 1:

• I went to the node/add screen for my content type, and chose "Great-grandfather", "Grandfather", "Father" - ie 3 levels out of the 4
• I didn't enter any other fields (Specifically, I left the title field blank)
• I submitted the form.
• Form re-loads with errors shown on cshs field as expected. My existing choices of Great-grandfather, grandfather, and father are preserved

Scenario 2

• I went to the node/add screen for my content type, and chose "Great-grandfather", "Grandfather", "Father" - ie 3 levels out of the 4
• I re-set the third level in the cshs widget back to "-Please choose-" - so that only two levels were selected
• I didn't enter any other fields (Specifically, I left the title field blank)
• I submitted the form.
• Form re-loads with errors shown on cshs field as expected. However, my existing choices of Great-grandfather, grandfather, and father have been lost and I have to start again from the top level of the hierarchy.

Hi - that looks great - I can confirm it works as expected for me - updated to RTBC :)

I started my review of this, so I am assigning to me.

My work/home commitments the last few days have prevented me from completing my full review, but...

The README does not follow the guidelines for in-project documentation and/or the README.txt Template. It really needs to describe how a user would use this module.

Did you contact the maintainers of either of the two modules that you mention above? As this is essentially just a field widget, it seems like it could be a pretty clean/easy patch to shs. This would consolidate some functionality, and give end users the ability to choose which best fits their needs and also change between them w/o needing new modules.

Hm, its quite difficult for me to decide whether "cshs" could be integrated into "shs". While the resulting widget looks similar, the main concept is sure another.
I would suggest leaving this as a separate project for now and we could talk about a possible integration (and if its possible at all) during the next camp (maybe DrupalCamping ... ;) ).

Automated Review

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

• Coder 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.

False positives w/ the CSS and JS.

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Addressed above.

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements

3rd party code

Yes: Follows the guidelines for 3rd party code.

README.txt

Yes: Follows the guidelines for in-project documentation and the README.txt Template.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Yes.

Coding style & Drupal API usage

Place code review here using this format:

The JS doesn't use 2 spaces per level of indentation, which is the Drupal standard.

The module passed some explicit XSS tests.

The use of $key on line 252 troubled me, but this mirrors the usage in form_select_options(). I don't know why form_select_options doesn't use
drupal_attributes, but you may want to think about it here.

The !name placeholder in cshs_widget_validate also troubled me, but I tested this fairly extensively with XSS. I actually don't think the logic
here is correct, as I can't get $value to be '_none'. It also looks like this is escaped already, as I tested bypassing the test and passing
the t() to drupal_set_messasge.

I went through this a few times and didn't see any blocking issues. Seeting to RTBC and assining to @heddn for a second look if he has time.

Not blockers:

  Location      file [cshs] - .../js/libs/jquery.simpler-select.js   Problem synopsis      Variable $select implicitly declared at line 120
  Location      file [cshs] - .../js/libs/jquery.simpler-select.js   Problem synopsis      Variable $option implicitly declared at line 143
  Location      file [cshs] - .../js/libs/jquery.simpler-select.js   Problem synopsis      Parameter parent is not described in JSDoc (at line 130)

See as there are no blockers... Thanks for your contribution, valderama!

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.