Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
git repository
git clone http://git.drupal.org/sandbox/themestar/1838172.git drupal_professional_theme
Comments
Comment #1
ARUN AK CreditAttribution: ARUN AK commentedHi themestar,
to review your project please provide you project's sandbox link and git repository link here.
Comment #2
themestar CreditAttribution: themestar commentedi have added sandbox link and git repository link
Comment #3
themestar CreditAttribution: themestar commentedNeed review
Comment #4
indydas CreditAttribution: indydas commentedHi,
Nice theme, very intriguing to see the framework behind this.
It appears you are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.
Also this is the results of your master branch from a code review.
http://ventral.org/pareview/httpgitdrupalorgsandboxthemestar1838172git
I have checked out your code now. Could you explain a bit more as to how the theme will be in HTML5? Will users be presented with markup in the templates or will they need to do the markup themselves. Also are you looking to use media queries and breakpoints in the stylesheets so that the theme is responsive to any device?
Thanks!
Comment #4.0
indydas CreditAttribution: indydas commentedgit repository
Comment #5
themestar CreditAttribution: themestar commentedThanks for your feedback,
Fixed all http://ventral.org/ reviewed errors
Created "7.x-1.x" branch
Drupal Professional Theme is not HTML5 and Responsive. FIxed width ( 1000px ). sorry i was mistaken in issue summery
Comment #6
odegard CreditAttribution: odegard commentedTheme looks nice.
The coder module reports issues in the info-file, but they are as far as I can see wrong (they report the same problems with Bartik!).
According to the examples module on js_example the script.js should be changed from this:
to this:
Other than that, looks good to me!
Comment #7
themestar CreditAttribution: themestar commentedThanks odegard,
Fixed script.js issue. please check
Comment #8
odegard CreditAttribution: odegard commentedConfirmed that the script is updated.
However, I just noticed that the main body background image body.png is 1.8MB!
Comment #9
themestar CreditAttribution: themestar commentedi will compress the image size
Comment #10
themestar CreditAttribution: themestar commentedFixed,
Removed heavy image files.
Comment #11
odegard CreditAttribution: odegard commentedConfirmed body.png is removed.
Comment #12
zymphonies-dev CreditAttribution: zymphonies-dev commentedHI,
Manual Review:
please fix these following UI bugs
1) Give button style cursor:pointer and hover effect.
2) Blog: give little more space between each links.
(attached screenshot 1,2 points)
Thanks
Comment #13
zymphonies-dev CreditAttribution: zymphonies-dev commentedchanging status to "need work"
Comment #14
themestar CreditAttribution: themestar commentedThanks shanidkv,
Fixed CSS issue please check
Comment #15
zymphonies-dev CreditAttribution: zymphonies-dev commentedconfirmed,
theme looks good now
Comment #16
bryanbraun CreditAttribution: bryanbraun commentedEverything seemed to pass the automated tests I ran. I looked through your Code manually and found a few things:
The theme looks nice... it will be a great addition to the theme directory. Keep up the good work.
Comment #17
themestar CreditAttribution: themestar commentedthanks for your feedback
i will fix these issues
Comment #18
gillisig CreditAttribution: gillisig commentedYou might want to consider another name. There is already a theme called Professional Theme (http://drupal.org/project/professional_theme)
Also your project page is not very detailed, it would be helpful if you gave a quick brief of what is different about your theme compared to the others, even if it involves counting up its visual features.
Comment #19
themestar CreditAttribution: themestar commented@bryanbraun
i have fixed all issues. please check.
@s2sz
can i use name "Max Professional Theme". just i named as "Max"
Comment #20
gillisig CreditAttribution: gillisig commentedYeah probably you can use Max Professional Theme. although I am no expert since I am in the process of releasing my first theme here as well so maybe others want to pitch in whether that is ok or not :)
Comment #21
themestar CreditAttribution: themestar commentedif i change project name. i have to create new sandbox project and upload files. maybe better to keep this same name ( "Drupal Professional Theme" ). and i have fixed all issues. please tell me your suggestion about naming. or shall i change to "need review" status
Comment #22
Danny EnglanderHi themestar, that's only the case for full projects as far as renaming goes and it's really the alias that counts. You can rename your project when it's in the sandbox state by simply editing your project title and there is nothing dependent on the URL / alias right now. Once your project is promoted to full, you then determine an alias for the project i.e. max_professional_theme or whatever. I just went through that and I actually renamed my sandbox a few times and did not need to create a new project. Once my project got approved and I had full project promotion rights, I then had to choose the final resting place of my theme by then selecting a specific unchangeable alias.
Comment #22.0
Danny EnglanderDescription changes
Comment #23
themestar CreditAttribution: themestar commentedI have changed theme name to "Max Professional Theme".
please check
Comment #24
peterx CreditAttribution: peterx commentedHello themestar,
Nice use of red, white, and black.
Does your theme pass any accessibility standards, like WAI? If it does, it is worth mentioning on your project page.
Do you have a demo page?
Comment #25
themestar CreditAttribution: themestar commentedYes,
i followed all drupal standerds and compiled in http://ventral.org/.
presently i dont have any domain & server. so i will try to upload in my friends server
Comment #26
monymirzaHi,
please fix errors/warnings listed here:
http://ventral.org/pareview/httpgitdrupalorgsandboxthemestar1838172git
Comment #27
themestar CreditAttribution: themestar commentedFixed, please check
Comment #28
Danny EnglanderHi themestar here are a few items for feedback. If you are changing the name of the theme, you should probably edit the info file.
Right now it says:
drupal_professional_theme.info
...should it be:
max_professional_theme.info
-- You may also want to update your default logo and the theme description in the info file. You will probably want to change the name of your theme folder to match this as well.I am also getting a Javascript error relating to Superfish. I am not sure why you have the call to Superfish in your theme. The Readme says to install the Superfish module. That module takes care of any calls to Superfish other than downloading the library itself so I think your code in script.js is superflous.
The Readme also mentions:
I did not see any theme setting or otherwise for a slideshow after I installed your theme.
I also think your project page lacks any detail, description or depth. You might want to take a look at these:
Comment #29
Danny EnglanderNote, I am not sure if this has been mentioned but you may want to consider:
#1410826: [META] Review bonus - by reviewing other's project applications, it will help with your own and it's also a good learning experience.
Comment #30
themestar CreditAttribution: themestar commentedok thanks,
i will review other's project applications
Comment #30.0
themestar CreditAttribution: themestar commentedtheme name changes
Comment #30.1
LaelMoon CreditAttribution: LaelMoon commentedCreate a new issue in the Project Applications queue.
Comment #31
PA robot CreditAttribution: PA robot commentedClosing due to lack of activity. Feel free to reopen if you are still working on this application.
I'm a robot and this is an automated message from Project Applications Scraper.
Comment #31.0
PA robot CreditAttribution: PA robot commentedi edit this nod, By my mistake. revert.