This module integrates the wow.js library into Drupal 7. WOW is a Javascript library which works nicely with the Animate CSS library to create great cross browser CSS3-based animations in your Drupal sites.

More information on the WOW JS' project page.

Git clone command:

git clone --branch 7.x-1.x http://git.drupal.org/sandbox/GoddamnNoise/2419147.git wow_js
cd wow_js

PAReview results: http://pareview.sh/pareview/httpgitdrupalorgsandboxgoddamnnoise2419147git

Manual reviews of other projects

Comments

GoddamnNoise’s picture

GoddamnNoise CreditAttribution: GoddamnNoise commented
Issue summary: View changes
GoddamnNoise’s picture

GoddamnNoise CreditAttribution: GoddamnNoise commented
Issue summary: View changes
Issue tags: +PAreview: review bonus

Added three manual reviews of other project applications to get the review bonus status.

yuriy.kostin’s picture

yuriy.kostin CreditAttribution: yuriy.kostin commented
Status: Needs review » Needs work

Hi GoddamnNoise,

Automated Review
⊠ Git default branch is not set, see the documentation on setting a default branch. See https://www.drupal.org/node/1659588

Individual user account
☑ Follows the guidelines for individual user accounts.

No duplication
☑ Does not cause module duplication and/or fragmentation.

Master Branch
☑ Default branch is set up.

README.txt/README.md
☑ The content is ok.

Installation / Usage
⊠ After installation module please set message that need to install wow plugin in libraries.
⊠ Better if you add string urls on lines 17, 37, 38 to define function in wow_js.module.
⊠ function wow_js_help() is not working. If go to admin/help#wow I will see standard help content.

Project Description
⊠ You should provide the pareview link in the issue page to make the review process easier. http://pareview.sh/pareview/httpgitdrupalorgsandboxgoddamnnoise2419147git

My wish:
Better if you do drush comand to install wow plugin

Nice work!

GoddamnNoise’s picture

GoddamnNoise CreditAttribution: GoddamnNoise commented
Issue summary: View changes

Hi yuriy.kostin,

Thanks a lot for your review!. About those problems you've found in my project application:

  • branch git issue: fixed.
  • After installation module please set message that need to install wow plugin in libraries: Fixed. I've added a warning message when the wow.js library is not installed in the libraries folder.
  • Better if you add string urls on lines 17, 37, 38 to define function in wow_js.module: Fixed.
  • function wow_js_help() is not working. If go to admin/help#wow I will standard help content: Fixed. It was a bug. If you go to admin/help#wow_js you'll see a "WOW JS" link in the help topics.
  • You should provide the pareview link in the issue page to make the review process easier: fixed.

One final note about your whish. I don't know how to do that, but if you could point me in the right direction i'd be glad to do that (it could be a good feature for the next release).

GoddamnNoise’s picture

GoddamnNoise CreditAttribution: GoddamnNoise commented
Status: Needs work » Needs review
yuriy.kostin’s picture

yuriy.kostin CreditAttribution: yuriy.kostin commented

Hi GoddamnNoise,

I add to you example to use drush comand for download js plugin to folder "libraries". (In example used colorbox module)
http://cgit.drupalcode.org/colorbox/tree/drush/colorbox.drush.inc

If you have any questions, I will help you.

GoddamnNoise’s picture

GoddamnNoise CreditAttribution: GoddamnNoise commented

Hi yuriy.kostin,

I'm improving this project's help documentation right now. If it's promoted to full drupal project, i'll try to add the drush comand for the next release and i'll ask for your help if i find any problems.

Thanks a lot again!.

GoddamnNoise’s picture

GoddamnNoise CreditAttribution: GoddamnNoise commented

Hi again, yuriy.kostin,

I've just made your wish true. I've added a drush command to the module, so you'll be able to download and install the wow.js library with the "drush wow-js" command.

criz’s picture

criz
he/him
Location Vienna/Austria
CreditAttribution: criz commented
Status: Needs review » Reviewed & tested by the community

Automated Review

No issues found. http://pareview.sh/pareview/httpgitdrupalorgsandboxgoddamnnoise2419147git

Manual Review

Individual user account
Yes: Follows the guidelines for individual user accounts.
No duplication
Yes: Does not cause module duplication and/or fragmentation.
Master Branch
Yes: Follows the guidelines for master branch.
Licensing
Yes: Follows the licensing requirements.
3rd party assets/code
Yes: Follows the guidelines for 3rd party assets/code.
README.txt/README.md
Yes: Follows the guidelines for in-project documentation and/or the README Template.
Code long/complex enough for review
Yes: Follows the guidelines for project length and complexity.
Secure code
Yes: Meets the security requirements.
Coding style & Drupal API usage and more
         
  1. Just a minor thing: Maybe it would be better to detect the version using the wow.js file itself, and not bower.js (as it is not really needed and some people might prefer to have only the js/css files in the libraries folder).
  2. Minor: I am not sure why you are checking if the wow.min.js file exists when the libraries isn't available in hook_requirements, as the library module is a dependency.
  3. Suggestion: I would prefer to have a custom javascript provided by the module that initializes the library. Maybe also with a small settings form for the javascript settings.
  4. Suggestion: In my opinion it is good that you have no dependency to https://www.drupal.org/project/animate_css, because this way you are free to use for example the animate.scss version. Alternatively you could also just load the animate.css version packaged in the wow.js library (what could be also a module setting by the way). But some documentation about these options in the readme would be nice to have.

    5.    

Disclaimer: I didn't check the lately introduced drush integration.

This review uses the Project Application Review Template.

GoddamnNoise’s picture

GoddamnNoise CreditAttribution: GoddamnNoise commented

Hi criz,

Thanks a lot for your review. About your comments:

  1. Yeah. I thought that too, but the version of the wow.js library doesn't appear in the wow.js file. It only appears in the wow.min.js, but it appears as part of a comment, so I thought that checking the version in the bower.js file would be safer.
  2. You're right. I could remove that checking when the libraries module is requirement (libraries was not a requirement at first). I'll remove that check in the next commit.
  3. Ok. I've written down your suggestion. I'll try to add those features for the next release. Could I ask you for advice if this module is turned into a full Drupal project?.
  4. I've written down this suggestion too. It seems they are good features to have for the next release.
criz’s picture

criz
he/him
Location Vienna/Austria
CreditAttribution: criz commented

Hi GoddamnNoise,

Great, sure you can ask back for advice.
Looking forward to use this module!

mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
CreditAttribution: mpdonadio commented
Assigned: Unassigned » er.pushpinderrana

Automated Review

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

  • No automated test cases were found, did you consider writing Simpletests or PHPUnit tests? This is not a requirement but encouraged for professional software development.

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

Individual user account
Yes the guidelines for individual user accounts.
No duplication
Yes: Does not cause module duplication and/or fragmentation.
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/README.md
Yes: Follows the guidelines for in-project documentation and/or the README Template.
Code long/complex enough for review
Yes: Follows the guidelines for project length and complexity.
Secure code
Yes: Meets the security requirements.
Coding style & Drupal API usage

You don't need to feature detect for libraries in code if you have it as a dependency in your .info

Don't break translated strings across lines to appease the pareview script. It makes them harder to translate.

Code can be longer than 80 chars. You have some artificial breaks in places that make it harder to read as a result.

(+) The hook_init is bad. Users should be able to declare when and where they need the library.

(+) Don't pass built links to t(). Add the link markup to the string, and pass in the URL as a paramater. See https://www.drupal.org/node/322774

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

No blocking issues. Assigning to er.pushpinderrana for a second look, if he has time.

GoddamnNoise’s picture

GoddamnNoise CreditAttribution: GoddamnNoise commented

Hi mpdonadio,

Thanks for you nice review. I've just fixed all the issues you've detected in your review:

  • I've removed the libraries detection from code (it's a dependency in the module's info file).
  • I've arranged the previously broken translated strings to make its translation easier.
  • I've moved the library load from hook_init() to hook_page_build() (see this tip from Dave Reid ).
  • I've read the documentation about t() function and links and added the link markup to the strings to provide context for translators.
  • About what you have told about the users should be able to declare when and where they need the library: i've fixed that too. By default the module will load the wow.js library in every non-administrative page. If the user want to load the wow.js library in a few pages only, they will be able to do it from the theme's settings. I've added a field to that settings page where the user can provide a list of pages where the wow.js library has to be loaded.

I think I haven't forgotten anything. Thanks again for your review and your good tips to make a better module.

GoddamnNoise’s picture

GoddamnNoise CreditAttribution: GoddamnNoise commented

Hi again,

I've improved the interface to select on which pages the wow.js library should be loaded. I've provided an interface like the one provided in Drupal's block configuration forms. So, the user can go to the theme settings page to set on which pages the wow.js library should be loaded (or not loaded). I've made an integration with Drupal's core PHP filter module (the user can configure the wow.js library to be loaded on those pages where certain PHP expression retuns TRUE) too.

I've updated the module's project page, the README.txt file and the module's help page accordingly.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented
Assigned: er.pushpinderrana » Unassigned
Status: Reviewed & tested by the community » Fixed

manual review:

  1. it seems the global $custom_theme variable does not exist in Drupal 7. Why do you even need this function? global $theme will always contain the current theme after drupal_theme_initialize() has been called in the Drupal bootstrap.
  2. It would be interesting to see how this module works with a Views listing for example, you could add an example module or Views export for demonstration.

But otherwise looks good to me, so ...

Thanks for your contribution, GoddamnNoise!

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:

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.

GoddamnNoise’s picture

GoddamnNoise CreditAttribution: GoddamnNoise commented

Hi klausi,

Thanks a lot for your review and for making possible for this module to be a "full" project. Thanks for your comments too. About those comments:

  1. I've read that is the right way to check which is the current theme in Drupal. I can't find where i saw that code, maybe an outdated post. I'll fix that using the global $theme variable instead.
  2. Yeah. I'm working on a new module which will integrate the Animate CSS and WOW JS libraries with Drupal's Block system. I'll link the new module from this module's project page.

Thanks for your guidance and for all the resources you've linked to. I'll read all that info.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.