[D7] Exit Modal

Hi rob.costello,
Few tweaks are here fix the automated pareview issues too and the issues are listed in the given below url.
Upload the sanbox url from your project
Change the version to work from "Default Branch"
Remove "version" from the ./exit_modal.info file
Remove the license.txt in the module it will updated only by Drupal community

Automated Review

Fix all the Pareview issue listed in the given url http://pareview.sh/pareview/httpsgitdrupalorgsandboxrobcostello2775329git

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

No: Does not follow 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

No: Does not follow this module is approval for one time approval guidelines for project length

Secure code

Yes: Meets the security requirements

Coding style & Drupal API usage

[List of identified issues in no particular order. Use (*) and (+) to indicate an issue importance. Replace the text below by the issues themselves:

1. (*) Variables which are created for this module should be deleted while uninstalling the module.
2. (+) It would be good, if you implement hook_help in your module.
3. Just a recommendation: - Can this module will merged with the module which you have specified(Bounce Convert)

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.

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.

I installed this module. But I couldn't make it work as defined in project page. Please explain more like how and where I can change configurations. Also I am getting following warning after install this module.

Strict warning: Only variables should be passed by reference in exit_modal_content() (line 139 of \sites\all\modules\exit_modal\exit_modal.module).

Also if I am assigning any block which has no scope to the current page(eg: Author Pane) then I am getting below warning message.

Notice: Trying to get property of non-object in exit_modal_content() (line 140 of \sites\all\modules\exit_modal\exit_modal.module).

Thanks,
ARUN AK

This functionality is already available in Bounce Convert module. I am feeling this module only replicating the minimal functionality of Bounce Convert module.

Thanks,
ARUN AK

Hi rob.costello,

That sounds great!!! If this module can add user registration and fivestar rating blocks in to exit popup then it will be an additional functionality to the Bounce Convert.

I have done one more round of review and found couple of things.

1. You should pass default value to the variable_get() function to use value if variable is not set. eg: use variable_get('exit_modal_type', 'text') instead of empty(variable_get('exit_modal_type')) ? 'text' : variable_get('exit_modal_type') in line 48 in exit_modal.module. Fix in all places. Refer variable_get().
2. Use #attached attribute to include css and js with block.
   

   $block['content'] = array(
           '#markup' => exit_modal_content(),
           '#attached' => array(
             'css' => array(
               drupal_get_path('module', 'exit_modal') . '/exit_modal.css',
             ),
             'js' => array(
               drupal_get_path('module', 'exit_modal') . '/exit_modal.js',
             ),
           ),
         );

3. I think we can assign Exit Modal block in to any region to make it work. If that is the case then can we have any alternate method to include exit module popup content in to the page. So that we can avoid the step users to assign Exit Modal block in to some region.

Thanks,
ARUN AK

Some nitpicking:
README.txt: manually wrap text around 80 characters.

exit_modal.js: include a block comment (/** */) for your behavior defining what the attach function does.

exit_modal.module: inline comments (//) should end with a period.
exit_modal.module: It's better to wrap strings containing apostrophes in double quotes rather than single quotes and having to escape the ', especially in t() strings. (ex. line 99)
exit_modal.module Lines 148, 150, 167: You're forgetting to pass in a default value to variable_get().

Otherwise, good work.

Hi rob.costello,

I reviewed it again and it seems like you have fixed the issues mentioned in comments. Exit modal functionality is working fine as defined.

I have one suggestion that you can consider in future development, currently the exit modal is activating every time user loads the page. Sometimes it feels annoying to the user. It would be better to have a configuration to setup how many times need to display exit modal popup for a user or a settings to define certain time interval to display exit modal popup.

Otherwise looks good to me and giving RTBC.

Thanks,
ARUN AK

I will update 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!

Thank you, 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 go the dedicated reviewer(s) as well.