This module will add a drag drop type of question in quiz module.
By using this module admin can upload images with their titles and later on all titles will become placeholder. User need to drag and drop images on the correct placeholder to get points. Currently we don't have this type of question in quiz module in drupal.

Project Page Link : http://drupal.org/sandbox/npscode/1888166
Git Repository Link : git clone --recursive --branch 7.x-1.x npscode@git.drupal.org:sandbox/npscode/1888166.git quiz_drag_drop
Non-maintainer : git clone http://git.drupal.org/sandbox/npscode/1888166.git quiz_drag_drop
This module is for Drupal 7.

Reviews of other projects:

http://drupal.org/node/1913606#comment-7051834
http://drupal.org/node/1913916#comment-7051892
http://drupal.org/node/1910316#comment-7053624

Some more manual reviews done by me:

https://drupal.org/node/1918758#comment-7072766
https://drupal.org/node/1917624#comment-7072874
https://drupal.org/node/1918832#comment-7073046

CommentFileSizeAuthor
#15 coder-results.txt1.15 KBklausi
#4 drag_drop_quiz.png32.69 KBbrunodbo
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

brunodbo’s picture

Status: Needs review » Needs work

Thanks for posting your module!

Some comments after a manual review:

  • Your .info file contains release info that automatically gets added by the packaging script on drupal.org (the version/info/timestamp info). These lines should be removed from your .info file. Have a look at an established contrib module's info file, you'll see which lines automatically get added.
  • As far as I know, you module's files should be called whatever your module folder is named. So eg. quiz_drag_drop.module instead of drag_drop.module etc.
  • Please create a 7.x-1.x branch and remove the master branch from your git repo.
  • drag_drop_config() currently does nothing. Should it be removed, or are you planning to use it in the future?
  • Noticing some code style issues, but an automatic review will also catch those (see the link below).
  • Again, AFAIK, a directory called 'theme' is usually used for template files in theme related .inc files. CSS and JS files usually have their own directory, or live in the module's main folder.

I also ran your module through PAReview, which lists all the code style issues, and a few other ones, see http://ventral.org/pareview/httpgitdrupalorgsandboxnpscode1888166git for the report. You can re-run the review after you've made changes, to verify your progress.

npscode’s picture

Hi brunodbo,

Thanks for reviewing my module.

As per your suggestion i have done all the changes required and created 7.x-1.x branch.

I have removed all the coding style related issues except few in "quiz_drag_drop.classes.inc", since my module uses quiz module's api/classes to introduce a new drag drop type of question. i am bound to follow these structure/functions otherwise my module will not work.

Other than this i have fixed every issues you pointed out.

npscode’s picture

Status: Needs work » Needs review

Kindly review the code again.. thanks..

brunodbo’s picture

Status: Needs review » Closed (fixed)
FileSize
32.69 KB

I noticed some things in theme_quiz_drag_drop_answer_form():

In quiz_drag_drop_uninstall(): would it be possible to remove quiz_drag_drop nodes when uninstalling the module?

Some typos:

  • Code comment in quiz_drag_drop_config(): 'Required' and 'Returned' should not have a capital.
  • Text strings in theme_quiz_drag_drop_response(): 'Skipped' shouldn't have a capital; neither should 'All'.

I enabled the module on a fresh D7 installation and created a quiz with a drag and drop question. It works well (and drag and drop questions are fun! :)).

  • When my image title (used as the placeholder) is longer than 2 words, it is placed outside of the 'drag' box (see attached screenshot).
  • You may want to clarify the help text in the image fieldset, eg. clarify that the image title will be used in a box that the image has to be dragged to ('placeholder' isn't that clear IMHO).

I'm getting a number of PHP warnings when I navigate to a quiz with drag and drop questions (didn't investigate where these are coming from though, they might be coming from the main quiz.module):

Warning: Invalid argument supplied for foreach() in element_children() (line 6369 of /workspace/d7/d7_git/includes/common.inc).
Warning: Illegal string offset '#children' in drupal_render() (line 5836 of /workspace/d7/d7_git/includes/common.inc).
brunodbo’s picture

Status: Closed (fixed) » Needs work

Oops, wrong status.

npscode’s picture

Status: Needs work » Needs review

Thanks for all the suggestions. Did all the changes mentioned by you..

  1. Used Drupal API to generate unformatted list. Reset button is also now created by using form api.
  2. Deleted "field_dragdrop_image" in HOOK_UNINSTALL. As per quiz module standards i am not deleting quiz_drag_drop nodes as other question type modules also work in the same way.
  3. Fixed all the typos mistakes.
  4. Added a help text in image fieldset regarding placeholder title. If user still inserts a title more than 18 characters than in the placeholder, it will be trimmed to 18 characters only.

PHP warnings are coming from quiz module itself.. :)

Kindly do the review..

klausi’s picture

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

vladimir-m’s picture

Status: Needs review » Needs work

Hello

Thank you for great module.

Manual review:

1. Please add git link corresponding to non-maintainer like "git clone http://git.drupal.org/sandbox/npscode/1888166.git quiz_drag_drop"
2. In file: quiz_drag_drop.install at line: 19 variable "node_options_quiz_drag_drop" is set, for what you set this variable? If variable is set you have to delete them in hook_uninstall().

npscode’s picture

Hi vladimir-m,

I will take care of the second point mentioned by you but can you please explain first point. should i add git link "git clone http://git.drupal.org/sandbox/npscode/1888166.git quiz_drag_drop" in my project issue summary??

Thanks..

npscode’s picture

Status: Needs work » Needs review

Hi vladimir-m,

I have done all the changes mentioned by you. Kindly review the code again.

Thanks.

jurcello’s picture

Status: Needs review » Needs work

Functionality review:

I created a quiz question containing 4 images. On Chrome for OSX I was unable to drag a portrait picture to the matching box. It turned out that the image was to big for the box. Setting the tolerance option on line 19 of the javascript from fit to intersect solved the problem. Is there a reason that you used fit here?

Code review:
The code looks ok at first sight. I got some coding standard issues regarding whitespaces found at end of line, so maybe it is a good idea to check again, but this should not hold promotion.

This is a nice little module!

npscode’s picture

Hi jurcello,

I deliberately used fit there as i want user to drag and drop the image completely on the placeholder.

Let me know if you are not able to drop the image on the placeholder?

npscode’s picture

Status: Needs work » Needs review
npscode’s picture

Issue summary: View changes

Added non-maintainer like "git clone http://git.drupal.org/sandbox/npscode/1888166.git quiz_drag_drop" as per suggestion..

npscode’s picture

Priority: Normal » Major
Issue tags: +PAreview: review bonus

Added other projects reviews done. Kindly review the module now.

klausi’s picture

Priority: Major » Normal
Status: Needs review » Needs work
Issue tags: -PAreview: review bonus
FileSize
1.15 KB

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

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:

  1. project page is too short, see http://drupal.org/node/997024
  2. quiz_drag_drop_enable(): do not juggle with module weights, this is unreliable. Use hook_module_implements_alter() if you need to run before/after a specific hook.
  3. file doc blocks: do not include sponsoring or credits here, that can be done in the README.txt file.
  4. quiz_drag_drop.js: you should use Drupal.behaviors, see http://drupal.org/node/756722
  5. "$this->question->field_dragdrop_image['und']": do not use "und", use LANGUAGE_NONE instead.
  6. "public function isValid() {": the filter_xss() is useless there as you are not printing anthing to the user? Make sure to read http://drupal.org/node/28984 again. Also elsewhere.

Otherwise looks pretty ready, pushing back to "needs work" as you need to know when to sanitize user input and what it is good for. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

klausi’s picture

Issue summary: View changes

Added reviews of other projects done.

npscode’s picture

Status: Needs work » Needs review
Issue tags: +PAreview: review bonus

Hi klausi,

I have worked on all the points that you mentioned and fixed all the issues.

I have done another 3 reviews of other projects. so adding "PAReview: review bonus" tag again. Kindly do the review now.

I hope we can make it a full project now. :-)

Thanks..

npscode’s picture

I have done another 3 reviews of other projects. so adding "PAReview: review bonus" tag.

klausi’s picture

Assigned: Unassigned » cubeinspire
Status: Needs review » Reviewed & tested by the community
Issue tags: -PAreview: review bonus

manual review:

  • quiz_drag_drop_init(): why do you need hook_init()? You should only add your CSS if it is actually needed on the page, not on every single page response.

Otherwise looks RTBC to me. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Assigning to cubeinspire as he might have time to finally approve this.

jthorson’s picture

Status: Reviewed & tested by the community » Fixed

Thanks for your contribution, npscode!

I updated your account to let you 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 get 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.

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

Issue summary: View changes

added review links.