Webform pager creates a pager on multi page webform's (using the default webform pagebreak component). The pager has the following 3 options:
- Simple pager ('Page x of x')
- Show percentages ('XX%')
- Page slider with usefull tooltips
The tooltips for the 'page slider' are loaded from an extra field created during module installation called 'page description' on the pagebreak add/edit form. This is shown with jQuery tooltip when hovering a page.
I believe there a no similar projects exept maybe Webform Ajax Page which provides a Ajax next/previous pager (no page names/percentages or page slider).
Project link
http://drupal.org/sandbox/strakkie/1907226
Git
git clone http://git.drupal.org/sandbox/strakkie/1907226.git webform_pager
Drupal version
- 7.x
Reviews
- I have not yet reviewed any projects, i do not yet feel confident enough about my own code to review other contributers' code. Feel free to comment me about this at any time.
Reviews of other projects
http://drupal.org/node/1904782#comment-7029930 & http://drupal.org/node/1904782#comment-7032818
http://drupal.org/node/1117996#comment-7032684
http://drupal.org/node/1905876#comment-7033134
http://drupal.org/node/1917088#comment-7065390
http://drupal.org/node/1927554#comment-7109898
http://drupal.org/node/1932796#comment-7132558
Working on more ;)
Comment | File | Size | Author |
---|---|---|---|
#19 | Screen Shot 2013-02-26 at 9.52.05 AM.png | 38.14 KB | cedewey |
#17 | coder-results.txt | 61.11 KB | klausi |
#2 | Screen Shot 2013-02-05 at 11.29.27 AM.png | 7.7 KB | Oostie |
webform_pager.png | 7.69 KB | strakkie |
Comments
Comment #1
alexmoreno CreditAttribution: alexmoreno commentedHi strakkie,
i strongly recommend you to do the review of other projects. It is an excellent way of getting self confidence, about other projects and about your own project. I have learn more doing this reviews than working on my own modules or even attending to the Drupal Sprints.
Said that, thanks a lot for your effort in contributing to Drupal.org. Let me do you some comments, so we can try to improve a bit your code.
Firstly, the link to clone your project is wrong:
Your code doesn't seems to follow the doxygen conventions. For example there:
You should specify which variables your functions gets, what does it returns, etc... Please take a look at the official doxygen documentation: http://drupal.org/node/1354
You are having a lot of errors and issues because of that, for example because of using tabs instead double spaces, as doxygen recommends: http://ventral.org/pareview/httpgitdrupalorgsandboxstrakkie1907226git
I also think that the drupal_add_js functions should be included in the drupal hook_init, not in webform_pager_show_page_slider function.
These cvs tags are also wrong, you can remove them:
You are also working in a wrong branch. You must work in 7.x-1.x for example, not in 7.x-1.x-dev
The most important, correct your issues in the comments, there are a lot juest because of not using the comment conventions:
http://ventral.org/pareview/httpgitdrupalorgsandboxstrakkie1907226git
Thank you very much again.
Comment #2
OostieThnx for this module.
I've installed the module and found a problem when you leave the description field empty.
Also there's something not corrent in your css (see screenshot) you've set a min-heigt at the tags this can lead to different heights.
Comment #3
strakkie CreditAttribution: strakkie commentedOostie,
Thank you for your comment.
The page_description row could not contain NULL, this is fixed in the webform_pager.install
The css is fixed by using height, and i added a line-height.
Thank you again.
Comment #3.0
strakkie CreditAttribution: strakkie commentedTyp error
Comment #4
strakkie CreditAttribution: strakkie commentedHello urwen,
That is a great, and awesomly fast, comment. Thank you for your time.
I will look at them and report back when i think they are fixed.
Also, thank you for pointing out that it is a good idea to review other code. I will see what i can do.
Thank you, once more.
Comment #5
alexmoreno CreditAttribution: alexmoreno commentedno problem. Think that it is a "recommended practice", but i've seen projects months before being approved, and other in just a couple of hours. Just because of the reviews.
Thank you again.
Comment #6
alexmoreno CreditAttribution: alexmoreno commentedComment #7
strakkie CreditAttribution: strakkie commentedFirstly, the link to clone your project is wrong:I also think that the drupal_add_js functions should be included in the drupal hook_init, not in webform_pager_show_page_slider function.These cvs tags are also wrong, you can remove them:You are also working in a wrong branch. You must work in 7.x-1.x for example, not in 7.x-1.x-devThe most important, correct your issues in the comments, there are a lot juest because of not using the comment conventions:http://ventral.org/pareview/httpgitdrupalorgsandboxstrakkie1907226git
I am goint to look into a few module's code files to see if i can improve my commenting. In the meantime, let me know if you find anything.
Comment #8
strakkie CreditAttribution: strakkie commentedComment #8.0
strakkie CreditAttribution: strakkie commentedIncorrect git link, corrected
Comment #8.1
strakkie CreditAttribution: strakkie commentedAdded reviews
Comment #9
strakkie CreditAttribution: strakkie commentedPAReview: review bonus
Comment #10
xenophyle CreditAttribution: xenophyle commentedIn your install file you should use hook_schema to create your tables instead of writing the query to do it. See http://api.drupal.org/api/drupal/modules%21system%21system.api.php/funct....
Make sure to indent using 2 spaces. If your editor allows it, set it to show whitespace so you can see where the indentation is inconsistent. When this is fixed the code will be a lot easier to for us to read and review.
Comment #11
xenophyle CreditAttribution: xenophyle commentedComment #12
strakkie CreditAttribution: strakkie commentedHi Xenophyle,
Thank you for your time. I am now using hook_schema to create my database table.
About the indenting, i have my code but could'nt really find any inconsistencies in my indenting so i moved all code back to the base line and indented my code again (perhaps the tabs to spaces function doesn't work as it should in my editor).
Thanks for you review.
Comment #13
a_thakur CreditAttribution: a_thakur commentedwebform_pager.module is not following Drupal Coding Standards.
Ventral is not giving any errors but when I run code on my localhost, I see a lot of indentation problems as pointed in comment #10
Comment #14
strakkie CreditAttribution: strakkie commentedHi a_thakur,
Thnx for your comment, you are right. I found out that my system used Windows line endings, i converted these to Unix format but git gave my a hard time about the line endings. I cannot test it on an other system but i think these errors are fixed (my Coder doesn't show the errors anymore).
Thanks in advance.
Comment #15
wuinfo - Bill Wu CreditAttribution: wuinfo - Bill Wu commentedIt is a nice module to have.
This module loads newer version JQuery. Can you use jquery_update module or just use the default version? Do not like a module to load 300K javascript by itself.
Comment #16
strakkie CreditAttribution: strakkie commentedHi Wuinfo,
That is indeed one of the thins i still wanted to implement. But is was having a hard time getting the default jQuery to work in my module.
I will look into it!
Comment #17
klausiThis 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:
Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.
Comment #18
strakkie CreditAttribution: strakkie commentedREADME.TXT
I renamed it to .txt on my system but git didn't change it, removed and and pushed again with .txt (lowercase).
1. Fixed, only using default Drupal jquery and a custom created .js file for showing tooltips.
2. implemented hook_node_view, thanks a lot for pointing that one out. Didn't know about this one.
3. Function removed.
4. Removed.
5. Description added
Attachments, all errors fixed. Coder no longer showing any errors, except for @file block missing on my JS file, it is there? Is this normal behaviour?
Thanks a lot for your review Klausi, i will go back to reviewing other projects when i find some time later this week.
Comment #18.0
strakkie CreditAttribution: strakkie commentedExtra review
Comment #18.1
strakkie CreditAttribution: strakkie commentedExtra review done
Comment #19
cedeweyGreat module! In fact, I just wrote up a blog post about it for Project Review Wednesday on the Aten Design blog. It'll be published tomorrow.
The one thing I noticed is that the page name option needs a little bit of styling love. Otherwise, looks good!
Comment #20
strakkie CreditAttribution: strakkie commentedHi cedewey,
Thats awesome! Thank your very much!
I've been meaning to change the complete style for the pager, i am not yet happy with the current styling. Will look into this, thanks again!
Gr,
Strakkie
Comment #21
strakkie CreditAttribution: strakkie commentedFixed styling.
Back to needs review.
Comment #22
likebtn CreditAttribution: likebtn commentedTry to keep the code well formatted all over the lines.
In webform_pager.module on line 15:
change to:
Comment #23
strakkie CreditAttribution: strakkie commentedHi likebtn,
Thank you for looking at my module. I have updated the lines.
Also, i have now separated the HTML by using two template (tpl.php) files for showing the pager.
Comment #23.0
strakkie CreditAttribution: strakkie commentedExtra review
Comment #24
strakkie CreditAttribution: strakkie commentedBonus tag
Comment #25
strakkie CreditAttribution: strakkie commentedBonus tag (duplicate, woopsie)
Comment #26
klausimanual review:
But 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 Dave Reid as he might have time to take a final look at this.
Comment #27
strakkie CreditAttribution: strakkie commentedKlausi,
Great!! Thanks again for your review! I have fixed the points you have supplied.
Once again, thanks for your time!
I'll wait for Dave to have the final look!
Comment #28
jthorson CreditAttribution: jthorson commentedThanks for your contribution, strakkie!
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.
Comment #29.0
(not verified) CreditAttribution: commentedExtra review