reddream is a simple drupal 7 theme. Basically colour of black and dark.
sandbox :
http://drupal.org/sandbox/ketelagoreng/1624618
Features:
Superfish drop-down menu (2, 3 or more nested levels)
Built-in image banner
Tableless
XHTML 1.0 and CSS 2.1 validated
Primary links and secondary links
Using 3rd party :
jQuery Cycle Plugin by M. Alsup
jQuery Superfish Plugin by Joel Birch
jQuery hoverIntent Plugin by Brian Cherne
Note:
Doesn't support for IE 6
these are some review I did :
http://drupal.org/node/1788704#comment-6513498
http://drupal.org/node/1683924#comment-6513464
http://drupal.org/node/1397718#comment-6513414
Comment | File | Size | Author |
---|---|---|---|
#28 | search.png | 9.33 KB | zymphonies-dev |
#28 | RHS navigation.png | 14.19 KB | zymphonies-dev |
#28 | comment.png | 41.6 KB | zymphonies-dev |
#28 | error.png | 53.55 KB | zymphonies-dev |
#27 | drupalcs-result.txt | 1.05 KB | klausi |
Comments
Comment #1
iqbalnurhakim CreditAttribution: iqbalnurhakim commentedwelcome,
As installation and usage instructions are quite important for us to review, please take a moment to make your project page follow the tips for a great project page. Also make sure your README.txt follows the guidelines for in-project documentation.
while waiting for an in-depth review of your module you can start out fixing some coding style issues detected by automated tools:
http://ventral.org/pareview/httpgitdrupalorgsandboxketelagoreng1624618git.
Manual Review
The Libraries API module is a recommended method for adding 3rd party dependencies without directly including the code on Drupal.org. Or you can use the existing module to integration with superfish, etc
Comment #2
ketelagoreng CreditAttribution: ketelagoreng commentedComment #3
ketelagoreng CreditAttribution: ketelagoreng commentedthanks for your review...
I just fix some coding style issues detected by automated tools and push it :D
Comment #3.0
ketelagoreng CreditAttribution: ketelagoreng commentedupdate description
Comment #4
ketelagoreng CreditAttribution: ketelagoreng commentedcompleted, bug from ventral :)
Comment #5
hamburgers CreditAttribution: hamburgers commentedHello,
I found a few issues for you to look at. First of all you still have files other than README.txt in the master branch
There also appears to be an issue with your folder structure. I attempted to download and use the theme but you have two .info files and a second level reddream folder which seems to contain the theme.
I did a manual review of some of these files and there appears to be some tabbing issues in some of your tpl files. Ventral should have picked that up but I'm wondering if the folder structure caused it to miss those issues. As an example I'm looking at lines 19 & 20 in node.tpl.php
I'm also not certain about the 3rd party JS that you have included. I couldn't find any specific license for that code so I'm not sure if it can be included or not.
Comment #6
hamburgers CreditAttribution: hamburgers commentedComment #7
D4Ko CreditAttribution: D4Ko commentedTested in FireFox 13 and Opera 12 and found a small bug with the style css. See attachment. In my opinion you should insert margin: 10px in ul.primary instead of margin: 5px
Comment #8
ketelagoreng CreditAttribution: ketelagoreng commentedhello hamburgers, thanks for review reddream :D
yes, there is readme in branch master and I just remove it, thanks.
the folder reddream in the 1624760, I just delete the folder and take all files out.
then I just fix the node.tpl.php line 19 and 20, I hope it's fixed.
and about gpl, I sure that the 3rd partys I using are gpl but I'm not sure about the images in the slideshow is gpl. I'm still trying to find images with gpl license. :D
hello dako, thanks for your review :)
I just fix bug in tabs as your advice, using margin: 10px thanks. I hope it fixed.
Comment #9
ketelagoreng CreditAttribution: ketelagoreng commentedComment #10
igoen CreditAttribution: igoen commentedManual Review:
1. page.tpl.php: $user->name: Do not print the username like that, as it is user provided input. Use theme('username', ...) and it will generate the link markup for you.
2. page.tpl.php: all user facing text ("logged in as") must run through t() for translation. And don't forget to use placeholders for dynamic values like the username.
3. page.tpl.php & maintenance-page.tpl.php: all style information should be defined in CSS files.
Comment #11
ketelagoreng CreditAttribution: ketelagoreng commented- double post -
Comment #12
ketelagoreng CreditAttribution: ketelagoreng commentedhi igoen, nice to meet you
thanks for review reddream
I just fix the bugs, I hope it's fixed.
and thanks for your bluejems :p
it can solve my problems.
sorry if I see bluejems without your permission.
Comment #13
igoen CreditAttribution: igoen commentedGood.
Just for suggest, put the template files (ie: *.tpl.php) in a separate folder called “templates” and put the css files in a separate folder called “styles”. By doing so, you need to change the path in the info file too. It is not a good idea to put template files and css files in the same place. :D
Comment #14
ketelagoreng CreditAttribution: ketelagoreng commentedhi igoen, thanks for reviewing me again. As you suggestion I made them in folders templates and styles. thanks, and review me again if you find a bug in reddream :)
Comment #15
igoen CreditAttribution: igoen commentedManual review:
1. Rss feed colapsed in all IE. Just see screenshot.
2. Opera too.
3. In safari the search button is collapsed.
4. And see "text error.txt" , for suggest, i think you take too much color. And that is different color, is not good looking. Just change it into soft color.
5. And your secondary menu is broken, maybe you must give some margin of it. See in "text error.txt".
Good luck.
Comment #16
igoen CreditAttribution: igoen commentedComment #17
ketelagoreng CreditAttribution: ketelagoreng commentedyap, I just fix the bug in search and rss, but I still don't understand what's wrong with my secondary menu, can you give me some details in that bug?
Comment #18
igoen CreditAttribution: igoen commentedI think still there bugs on your theme.
Manual review:
1. On chrome your button is collapsed, see screenshot.
2. In IE and Opera, the RSS feed still overlapping, see screenshot.
3. Overlapping content, see screenshot.
4. In module page, your text is unreadable, please changes your font color.
5. In secondary menu still error, see screenshot, when I took cursor in whitespot, hover placed in false link.
6. Superfish menu is error, when I took my cursor in whitespot number 1, and slowly move to whitespot number 2, the dropdown menu disappear before I get my cursor to whitespot number 2. But it didn't happen when I do fast move.
Good luck!
Comment #19
igoen CreditAttribution: igoen commentedComment #20
ketelagoreng CreditAttribution: ketelagoreng commentedhi igoen, thanks again for reviewing me.
I just fix some bugs as you said.
1. I hope it fixed
2. same here, fixed
3. I just make sure that you should use space " "
4. font changed into white
5. fixed
6. fixed
thanks, need review again :D
Comment #21
igoen CreditAttribution: igoen commentedManual review:
1. Chrome still collapsed.
2. Different font size in menu bar.
3. IE 7 broken secondary links.
4. Just suggest, in box comment maybe you should pick a good color, its too different.
see screenshot.
good luck.
Comment #22
igoen CreditAttribution: igoen commentedManual review:
1. Chrome still collapsed.
2. Different font size in menu bar.
3. IE 7 broken secondary links.
4. Just suggest, in box comment maybe you should pick a good color, its too different.
see screenshot.
good luck.
Comment #23
igoen CreditAttribution: igoen commentedSorry my connection is errors.
Comment #24
ketelagoreng CreditAttribution: ketelagoreng commented1. Fixed
2. Sorry, but I still don't know what's wrong with it
3. Fixed
thanks
Comment #24.0
ketelagoreng CreditAttribution: ketelagoreng commentedupdate fix some bugs
Comment #25
ketelagoreng CreditAttribution: ketelagoreng commentedadded some reviews I did, please review reddream again. thanks
Comment #26
ketelagoreng CreditAttribution: ketelagoreng commentedPAReview: review bonus
Comment #27
klausiThere is still a master branch, make sure to set the correct default branch: http://drupal.org/node/1659588 . Then remove the master branch, see also step 6 and 7 in http://drupal.org/node/1127732
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:
Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.
Comment #28
zymphonies-dev CreditAttribution: zymphonies-dev commentedHi,
Manual Review
Few UI issues
1) Please middle align search button with text field
2) Right side block style is breaking
3) Add comment wrapper background color ( #fff ) not matching with site color ( please make background: transparent; )
4) Please fix Drupal Code Sniffer issues
(Attached screenshot for all points)
thanks
shanidkv
Comment #29
klausiClosing due to lack of activity. Feel free to reopen if you are still working on this application.
If you reopen this please keep in mind that 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 :-)
Comment #29.0
klausiadd some review