161 posts
  • Author Level 5
  • Collector Level 1
  • 2 Years of Membership
  • Exclusive Author
themebros
says

I’m pretty sure code readability (doesn’t have to be perfect, but at least some organization in there) is one of the requirements. If not then in my opinion it should be.

Would like to hear from a reviewer about this.

158 posts
  • 6 Years of Membership
  • Affiliate Level 3
  • Author Level 3
  • Exclusive Author
+1 more
greenshady
says

You can make the code more readable, but I don’t see any reason it should’ve been rejected based on that file.

1095 posts Best-dressed man at PressNomics 2013
  • Power Elite Author
  • Author Level 12
  • Featured Author
  • Featured Item
+13 more
Parallelus
says

I know this isn’t what you were asking, but PLEASE stop naming themes “metro” whatever. Maybe the reviewers can start rejecting themes with the name “metro” in them. How many can we have? Does anyone really want to find out?

This isn’t the reviewers fault, it’s the lack of originality from authors not differentiating their items from the plethora of other “metro” style AND named themes, but you have the power to make the madness end! You could use “metro” in your description, tags, etc.. but the name… again?

Maybe you could name it “[original name] – A Metro Inspired WP Theme” or just put it in the tags, description, whatever. It’s really driving me crazy each day to see 10 new themes named “metro” show up on the home page.

Look at it this way. When someone searches for “metro” you get results for 70+ items. Now you have to compete with all of them, plus how ever many new items with that name show up each day. Wouldn’t it be smart to come up with an original name that is memorable, stands out and brands your theme uniquely?

Ok, my time on the soap box is almost up. Final plea… help me Obi Wan Reviewer, your my only hope!

3804 posts
  • Elite Author
  • Author Level 11
  • Trendsetter
  • 7 Years of Membership
+12 more
KrownThemes
says

@TheMetroGuy, separate styles in more .css files. I see that you have media queries in the same .css file which of course is a bad practice. Responsive design should be optional.

lol .. i have all the stylesheet in only one file.. Never heard that media queries in the main stylesheet is a bad practice :D

Of course they are optional, since they can be deleted :D

151 posts
  • Affiliate Level 4
  • Author Level 6
  • Beta Tester
  • Collector Level 1
+5 more
RoyalTemplates
says

There also seems to be a fair bit of inline CSS, could have been the issue.

PS, this is an easy tool to make the CSS more readable, especially if you have been moving things around etc. – http://tools.arantius.com/tabifier

815 posts
  • Affiliate Level 1
  • Author Level 5
  • Collector Level 1
  • Freebie
+3 more
rvision_
says

This is an error:

ul#portfolio

ul is overkill, element id is enough

438 posts
  • 3 Years of Membership
  • Author Level 4
  • Collector Level 3
  • Exclusive Author
Pixelous
says

This is an error:
ul#portfolio
ul is overkill, element id is enough

+1

TheMetroGuy,

Stylesheet Guidelines
  • Follow CSS coding standards when authoring your CSS.
  • Use valid CSS when possible. As an exception, use vendor-specific prefixes to take advantage of CSS3 features.
  • Minimize CSS hacks. The obvious exception is browsers-specific support, usually versions of IE. If possible, separate CSS hacks into separate sections or separate files.
  • All possible HTML elements should be styled by the Theme, both in post/page content and in comment content.
    • Tables, captions, images, lists, block quotes, et cetera.
  • Adding print-friendly styles is highly recommended.
    • You can include a print stylesheet with <tt>media=”print”</tt> or add in a print media block in your main stylesheet.

This line of code is a bad practice:

div.empty_image{width:100%; height:200px; background: #777; display: inline-block;}
.

You should use this one:

.empty_image {
    width: 100%;
    height: 200px;
    background: #777;
    display: inline-block;
}

This also would be helpful http://csscomb.com/

60 posts
  • Affiliate Level 1
  • Author Level 3
  • Beta Tester
  • Collector Level 1
+3 more
DavidGuns
says

is a bad practise giving prefix identity of css?

prefix identity like .<theme-prefix>-content { ... } #<theme-prefix>-wrapper { ... }
161 posts
  • Author Level 5
  • Collector Level 1
  • 2 Years of Membership
  • Exclusive Author
themebros
says

is a bad practise giving prefix identity of css? prefix identity like .<theme-prefix>-content { ... } #<theme-prefix>-wrapper { ... }

There’s no need to prefix classes and IDs in a theme.

For plugins, it’s a different story. In that case it’s best to prefix them to avoid clashes with other plugins or themes.

35 posts
  • Affiliate Level 1
  • Author Level 5
  • Beta Tester
  • Collector Level 2
+3 more
TheMetroGuy
says
I started re-factoring the CSS following these guidelines, https://developer.mozilla.org/en-US/docs/CSS/Writing_Efficient_CSS

billyf, Yes, that’s my first step, removing all the commented code.

rvisio_, Pixelous, Thanks friends, I guess these kind of changes are what reviewers expect, as the increase the performance.

Parallelus, Actually, when I submitted my HTML template 7 months ago, there were just 1 or 2 other metro inspired items. Since this is a WordPress version of that template, I have to keep the name. Of course, I reserved some other names for future themes :)

by
by
by
by
by
by