Provides the Guiders-JS library to Drupal.

Guiders are a user experience design pattern for introducing users to a web application.
Guiders are a great way to improve the user experience of your web application.

This is the first module to integrate the Guiders-JS 3rd party library.
I have talked to the library author and got his "approval" for using his library in this module.

The module allows an admin to create a guider's pack and in it a set of guiders that are calling each other one by one.
Each guider's pack is active on a specific path.

you could see the library in action in this url: http://jeffpickhardt.com/guiders/
and you can see the module in action by installing it and creating your first guider's pack and guiders.

The module: http://drupal.org/sandbox/udisun/1227788

CommentFileSizeAuthor
#1 guiders_js-1234626-1.patch3.37 KBtim.plunkett

Comments

tim.plunkett’s picture

Status: Needs review » Needs work
StatusFileSize
new3.37 KB

Here's a patch with some basic changes, this isn't everything.

There is a lot of code commented out, so please don't set this back to "needs review" until that is finished.

In addition, while the hook implementations are documented correctly, all of your custom functions are not. Please see http://drupal.org/node/1354#functions for reference.

basik.drupal’s picture

Status: Needs work » Needs review

First of all I wanted to thank you for taking the time and reviewing my module.
I know how tedious it can be and I appreciate it very much.

I implemented your patch and followed you're guidelines by making changes to all of the module files.
I added a lot of needed documentation and made sure to follow the doxygen and drupal documentation guidelines.
I fixed all of the indentation problems.

Run it through coder again and fixed all of the problems.

In addition I added a cache implementation to the module to make sure there would not be any performance hits.

After spending a lot of time testing and making sure everything is up to the drupal standards,
I think the module is ready for another review and I hope it could be approved for the community.

Thanks again.

tim.plunkett’s picture

Priority: Normal » Major

I'm going to let someone else take another look at this, that way when they RTBC it, I can fix it in good conscience.

itangalo’s picture

Disclaimer: I don't have the power to deny/approve project applications, nor do I have very much coding experience.

This being said, I have browsed through the code, and it seems neat and tidy. Not every thing adheres to Drupal coding standards (I saw and array being assigned on multiple lines without each line ending with a comma, and some other thing), but nothing that should cause this project to be denied.

If a more experienced coder would like to review, I recommend looking closer at guiders_js_page_build (in guiders_js.module), where the real magic is.

bfroehle’s picture

In a brief look (10 minutes or so), the code quality looks good. The author is clearly familiar with using database, form, and cache APIs. All user input seems to be properly sanitized before display.

basik.drupal’s picture

thank you for taking the time and reviewing the module.

I's it in a place to be RTBC?, do I need to change the status or is that something a reviewer must do?

itangalo’s picture

Making it RTBC is probably not something you should do.
Other reviewers could do that, but more likely is that someone with authority will change it to 'fixed' directly.

basik.drupal’s picture

Tim, the other reviewers say this is ok to be published to the community but haven't changed the status to RTBC.
what now?

tim.plunkett’s picture

Status: Needs review » Fixed

I'm not sure why I was so worried about guiders_js_page_build(), it looks fine.

I've granted you full project creation rights. Use this ability carefully!

Thanks for your patience.

bfroehle’s picture

Yay! Can't wait to use Guiders-JS on a new site! :) Enjoy Ehud.

basik.drupal’s picture

Thank you all very much for your time in making sure everything was right.

Status: Fixed » Closed (fixed)

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

avpaderno’s picture

Issue summary: View changes
Priority: Major » Normal
Status: Closed (fixed) » Fixed

I am giving credits to the users who participated in this issue.

Status: Fixed » Closed (fixed)

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