Views Braincloud

Took a quick look at the module. Initial comments:

- Good title and project page
- No similar visualization alternatives (other than 'Cumulus') ... no real duplication issues.
- t() calls in place. Could use some tweaking, as identified in the below comments.
- I'm personally not very familiar with the various views hooks being leveraged in this module, but if it works, I'll have to assume they're being used correctly. ;)
- No real database interaction, so no concerns around common database-related security issues

Issues:
1. Remove the 'License.Txt' file. This will be added by the drupal packaging script.

2. Remove any $Id$ tags ... these are not required after the CVS to Git migration (despite Coder module suggesting otherwise.)

3. Licensing - can you identify where you got the Narkisim font?

4. Boolean constants: Drupal coding standards suggest using TRUE and FALSE instead of 0 and 1.

5. Leverage drupal_set_header() instead of calling header() directly. (Details at http://api.drupal.org/api/drupal/includes--common.inc/function/drupal_se...)

6. Not 100% sure on this one ... but I believe that taxonomy terms need to be sanitized (run through check_plain) before output. (Currently, the user-entered terms are displayed without sanitization in views_braincloud_style_plugin.inc)

7. Variable references in the form '@variablename' inside of t() functions don't need to be run through check_plain ... the @ symbol triggers this check automatically. See line 215 of views_braincloud_style_plugin.inc. (Similarily, a '!' prefix says to do no check, and a '%' prefix triggers a run through theme_placeholder().)

8. Not sure what the 'Uncaught Exception' comment at the top of your braincloud.js file is about ...

9. I believe the preference would be for any processing logic required in generating the actual braincloud output to take place in a preprocess function, as opposed to the .tpl.php file ... and the tpl.php file should contain little more than html tags and "print $variable['variablename'];" statements if possible. I realize that the braincloud rendering is a bit of a special case, and the chances of a themer trying to override the output by overriding the views-view-views-braincloud.tpl.php file are probably slim; but we should still stick to the existing precedent of seperating the logic (preprocess function) from the presentation (.tpl.php file) in order to accomodate theming over-rides. A quick look at the default views.tpl.php files will reinforce this seperation ... some of the existing views templates contain nothing more than a "print $output;" statement.

I think you could do the same, moving all of the code within views-view-views-braincloud.tpl.php into a preprocess function along with the 'drupal_add_js' and 'drupal_add_css' statements; and then have a very lightweight/minimal .tpl.php function containing just the basic 'div' tags. If needed, the 'application/processing' script could be loaded into a variable and the printed out in the .tpl.php file as well, if the drupal_add_js approach was to not work for one reason or another.

Other (less critical) suggestions:
1. There are some trivial indentation issues which could be cleaned at the bottom of views_braincloud.module (Drupal coding standards specify indenting two spaces.)

2. Consider loading the embedded classes on-demand, or leverage the 'Autoload' module, for a slight overhead improvement on pages not leveraging the views_braincloud classes.

3. Re-structure t() calls such as:
form_error($element, ('<em>'. ($element['#title'] . t('</em> must be a <em>numberic</em> value.'))));
to either include the element and a placeholder within the t() call (preferred), or remove the '/em' tag from the start of the t() function in order to provide additional context to translators.

4. Javascript scripts could also be included automatically by including a 'script' line in your module's .info file (though dynamic loading only for those pages where it's required would be preferred.)

5. I'm not as familiar with javascript in Drupal, but I don't see any of the regular patterns I would expect (using pages like http://drupal.org/node/171213 as a reference). As an example, I believe your processing.init.js optimizations could be called using the Drupal.behaviors structure, to keep things more 'Drupal-friendly' (though I openly admit that the 'Drupal-friendly' way doesn't always match up with how the vast majority of javascript developers would build things outside of the Drupal environment!).

6. I'd recommend enhancing the install instructions, to specify exactly where users can find the processing.js library, and exactly which directory they should install it in. To help facilitate upgrading the module and/or processing.js library independently, I'd suggest building in support for alternate installation locations for processing.js; with sites/all/libraries/processing as a good 'default' location.

7. In addition, the use of hook_requirements() can help determine when the processing.js library has not been installed, and reflect this issue on the administration page, the site 'status' report, and trigger a 'next steps' message during module installation. This helps to provide a better user experience for those users who fail to read the install directions (which is a LOT), and simply try to activate the module without following through with the processing.js installation.

Impressions
Code review and nitpicking aside, I do like both the premise and promise of this module ... it strikes me as a fun little piece of 'eye candy' to help add some splash to any given site. Great work, and I look forward to this module eventually being blessed for 'official' contrib status.

No activity in several months. Reopen and set the status to "needs review" if you are still pursuing this application.