Image CircleSlider
provides the user the flexibility to present their images in a more attractive and modern way in front of the users. It just modifies the views of the images in place of a list to a circular slider which the user can slide upon to have a nice view of the images. The slider can be used with as many images one likes but gives the best result when the picture size is greater or equal to 350pixels * 350 pixels.
For more information, visit the site
http://baijs.com/tinycircleslider/
to see what this slider does.
Link to the project : https://www.drupal.org/sandbox/ajalan065/2500887
Git source : git clone --branch 7.x-1.x http://git.drupal.org/sandbox/ajalan065/2500887.git image_circleslider
Pa-review : http://pareview.sh/pareview/httpgitdrupalorgsandboxajalan0652500887git
[In Pa-review, some points which would be useful to the reviewers: jquery.tinycircleslider.js is 3rd party plugin javascript, so cannot make changes. And regarding names of classes, which on changing are creating problems. I have followed the naming conventions as per the module views_slideshow]
Review of other projects:
1. [D7]Date Multiselect- https://www.drupal.org/node/2417425#comment-10009861
https://www.drupal.org/node/2417425#comment-10013183
2. [D7]Menu Auto Display- https://www.drupal.org/node/2432943#comment-10013609
3 [D7]oFeatures Customer Service- https://www.drupal.org/node/2504823#comment-10016703
https://www.drupal.org/node/2504823#comment-10019807
4.[D7]Commerce DragonPay- https://www.drupal.org/node/2427159#comment-10017465
5.[D7]Semantic UI Views- https://www.drupal.org/node/2431411#comment-10017585
6.[D7]Wet4 Slider- https://www.drupal.org/node/2479471#comment-10019841
Comments
Comment #1
PA robot CreditAttribution: PA robot commentedThere are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxajalan0652500887git
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.
Comment #2
dillix CreditAttribution: dillix commentedHi, ajalan065
First of all you should set normal title in this issue in format: [D7] Your module name
Also add to issue body link to git for your project.
Also there are problems with default branch of git:
There is still a master branch, make sure to set the correct default branch: https://www.drupal.org/node/1659588 . Then remove the master branch, see also step 6 and 7 in https://www.drupal.org/node/1127732
You should remove working files from git repo: ./Readme.txt~
Also move this code to function which call slider in your module:
Why you load 2 versions of jquery in imagecircleslider-view.tpl.php? Consider to use jquery_update module depency.
Also please fix Drupal code standards: https://www.drupal.org/node/318
You can check coding standards with this service: http://pareview.sh
Comment #3
ajalan065 CreditAttribution: ajalan065 commentedComment #4
ajalan065 CreditAttribution: ajalan065 commentedComment #5
ajalan065 CreditAttribution: ajalan065 commentedTnaks Dillix for your support and guidance. I went through your points and the links provided by you and have done the changes.
Default branch corrected to 7.x-1.x and removed master.
Removed working files Readme.txt~
Made changes to imagecircleslider.module
Made changes to imagecircleslider-view.tpl.php
Corrected the code according to Drupal Coding Standards
Comment #6
ajalan065 CreditAttribution: ajalan065 commentedComment #7
dillix CreditAttribution: dillix commentedYou should post link to your project git in first message.
Comment #8
ajalan065 CreditAttribution: ajalan065 commentedComment #9
ajalan065 CreditAttribution: ajalan065 commentedComment #10
ajalan065 CreditAttribution: ajalan065 commentedComment #11
ajalan065 CreditAttribution: ajalan065 commentedComment #12
ajalan065 CreditAttribution: ajalan065 commentedComment #13
ajalan065 CreditAttribution: ajalan065 as a volunteer and at Innoraft commentedComment #14
ajalan065 CreditAttribution: ajalan065 as a volunteer and at Innoraft commentedComment #15
ajalan065 CreditAttribution: ajalan065 as a volunteer and at Innoraft commentedComment #16
gaurav.goyal CreditAttribution: gaurav.goyal at Innoraft commentedHey ajalan065,
Great work. Thanks for your contribution.
I havent gone through the functionality of the project. I have just reviewed your code and below are my findings:
Please fix the above issues.
Comment #17
gaurav.goyal CreditAttribution: gaurav.goyal at Innoraft commentedComment #18
ajalan065 CreditAttribution: ajalan065 as a volunteer and at Innoraft commentedComment #19
ajalan065 CreditAttribution: ajalan065 as a volunteer and at Innoraft commentedHi gaurav.goyal,
I have followed through your steps and made the changes.
hooks_library() was added as because of convention, which I saw in most of the modules.
I have removed the imagecircleslider.tpl.php and have removed the duplicacy.
Thanks for reviewing my module.
Comment #20
ajalan065 CreditAttribution: ajalan065 as a volunteer and at Innoraft commentedComment #21
ajalan065 CreditAttribution: ajalan065 as a volunteer and at Innoraft commentedComment #22
ajalan065 CreditAttribution: ajalan065 as a volunteer and at Innoraft commentedComment #23
ajalan065 CreditAttribution: ajalan065 as a volunteer and at Innoraft commentedComment #24
ajalan065 CreditAttribution: ajalan065 as a volunteer and at Innoraft commentedComment #25
babusaheb.vikas CreditAttribution: babusaheb.vikas commentedHi ajalan065,
Take a look on imagecircleslider-field.tpl.php file.
There is wrong closing of
</section>
.In line 27 it should
</div>
and</section>
would be in line 28.You readme file name should be README.txt instead of Readme.txt
Comment #26
ajalan065 CreditAttribution: ajalan065 as a volunteer and at Innoraft commentedThanks babusaheb.vikas for your comments.
I have the respective changes.
Please review the module and comment on any further changes.
Thanks a lot.
Comment #27
BR0kENdrupal_get_path('module', 'imagecircleslider')
into constance and use it in all places where the value is needed. For example:libraries_get_path('library')
instead of hardcoded path.array_key_exists()
byisset()
here.$items
here. Just put the$variables['item']
as first argument toforeeach
.Comment #28
klausiThat look like nice but minor improvements and are not application blockers, anything else that you found or should this be RTBC instead?
Comment #29
BR0kENI think 4 item is not a minor. Library can be placed anywhere, not only in "sites/all".
Comment #30
klausiThat would be a standard bug report, no security issue, no licensing violation, no heavy API misuse - so should not hold up this application.
Comment #31
ajalan065 CreditAttribution: ajalan065 as a volunteer and at Innoraft commentedThanks BROkEN and klausi for contributing your valuable time to review my module.
I have made all the changes as pointed out by you.
Please comment if it need any more changes.
Comment #32
gaurav.goyal CreditAttribution: gaurav.goyal at Innoraft commentedHi ajalan065,
Module looks good to me. Few minor change as an extension of what BROKEN has pointed out in https://www.drupal.org/node/2502333#comment-10057474.
and you are trying to get "imagecirclesliderlib" in the above code. Please fix.
Comment #33
ajalan065 CreditAttribution: ajalan065 as a volunteer and at Innoraft commentedHi gaurav.goyal,
Thanks for contributing your time for the review.
I have looked upon these issues and fixed them.
Comment #34
ajalan065 CreditAttribution: ajalan065 as a volunteer and at Innoraft commentedComment #35
gaurav.goyal CreditAttribution: gaurav.goyal at Innoraft commentedLooks good to me. I think we can now move it to RTBC.
Comment #36
cweagansThanks for your contribution!
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.
Comment #37
PA robot CreditAttribution: PA robot commentedProject 1: https://www.drupal.org/node/2535206
Project 2: https://www.drupal.org/node/2502333
As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).
If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.
I'm a robot and this is an automated message from Project Applications Scraper.
Comment #38
cweagansComment #39
ajalan065 CreditAttribution: ajalan065 as a volunteer and at Innoraft commentedThanks cweagans for your contributing your time in reviewing.