Opened 7 months ago

Closed 7 months ago

Last modified 6 months ago

#9974 closed theme (not-approved)

THEME: Esquire - 1.1

Reported by: automattic Owned by: kwight
Priority: major Keywords: theme-esquire
Cc: themes@…

Description

Esquire - 1.1

An elegant and bold theme featuring multiple, visually distinct, Post Formats. Designed by Matthew Buchanan and inspired by the art direction of Esquire magazine.

Theme URL - http://theme.wordpress.com/themes/esquire
Author URL - http://matthewbuchanan.name/

SVN - http://themes.svn.wordpress.org/esquire/1.1
ZIP - http://wordpress.org/extend/themes/download/esquire.1.1.zip?nostats=1

Diff with previous version: http://themes.trac.wordpress.org/changeset?old_path=/esquire/1.0&new_path=/esquire/1.1

All previous tickets for this theme: http://themes.trac.wordpress.org/query?col=id&col=summary&col=keywords&col=owner&col=status&col=resolution&keywords=~theme-esquire&order=id

http://themes.svn.wordpress.org/esquire/1.1/screenshot.png

Change History (8)

comment:1 Changed 7 months ago by kwight

  • Owner set to kwight
  • Status changed from new to assigned

comment:2 Changed 7 months ago by kwight

Hi,

Is add_theme_support( 'print-style' ) a wpcom thing? I've never heard of it, and can't find any reference in trunk or the Codex.

comment:3 Changed 7 months ago by kwight

  • Resolution set to not-approved
  • Status changed from assigned to closed

Required

  • content.php: featured image support is broken for all but audio post format. Appropriate template tags can be added to content.php, or the "featured-images" tag can be removed from style.css
  • functions.php: Remove reference to add_theme_support( 'print-styles' ), doesn't exist
  • functions.php: Attach functions to appropriate hooks, eg. add_theme_support, load_textdomain etc to after_theme_setup
  • style.css: remove doubled "post-formats" tag

Not approved, due to above requirement.

This is a partial review; there may be other issues that would prevent approval. Please be sure to check all requirements as outlined in the Theme Review Guidelines, found at http://codex.wordpress.org/Theme_Review, before submitting a newer version.

You can also keep up on the latest developments in theme requirements by subscribing to the Theme Review mailing list (http://lists.wordpress.org/mailman/listinfo/theme-reviewers) and the Theme Review Team's website (http://make.wordpress.org/themes/).

Thank you!

comment:4 Changed 6 months ago by lancewillett

Thanks for the notes, just got the email notification *today* — 8 days after. :( November 16, 2012

We'll fix things up and submit a new version soon.

comment:5 Changed 6 months ago by kobenland

Hi Kirk,

Thank you for taking the time to review the Theme!

Frankly I am a bit surprised that the revision was not approved though, given that this is a previously approved Theme with obviously very few issues. Usually we point these things out to be fixed with the next revision, rather than not-approving the Theme.

Additionally, I was not aware that we had requirements concerning where Post Thumbnails should to be displayed. It only says that "if [the feature is] incorporated, it must support the core WordPress implementation" - which it does (it adds support, and retrieves the information via get_post_thumbnail_id()).

Would you mind sharing your thoughts?

Thanks again for all your efforts,
Konstantin

comment:6 follow-up: Changed 6 months ago by kwight

Of course.

I think users downloading a theme saying it supports featured images will expect featured images to work (at minimum) on a regular post, rather than just on one rarely-used post format. I felt this was enough to ask it be addressed now rather than waiting for another version (as an older theme, we never know how long it will be until that next version rolls around).

Of course, I missed proposing the simplest fix for all involved: just add a note to readme.txt stating the limitation. Attach the new ticket here, and I'll be happy to approve it right away.

Usually I just do quick diff reviews for themes that are already approved, but sometimes I'll pull it down and have a closer look for one reason or another (in this case it was the "print-style" thing). Guidelines and requirements change, and things fall through the cracks; I think it's good for reviewers to take a closer look at approved themes now and again.

And I'm still really curious: what is add_theme_support( 'print-style' )?

Thanks!

comment:7 in reply to: ↑ 6 Changed 6 months ago by kobenland

Replying to kwight:

Of course, I missed proposing the simplest fix for all involved: just add a note to readme.txt stating the limitation.

Duh, me too! :) Done!


Guidelines and requirements change, and things fall through the cracks; I think it's good for reviewers to take a closer look at approved themes now and again.

I couldn't agree more. Maybe something for the "How can we improve the Theme Review process"-post?


And I'm still really curious: what is add_theme_support( 'print-style' )?

It selectively loads a global print stylesheet across all WP.com themes. Since this is not part of Jetpack, I removed it in #10118.

Speaking of which, #10118 is the new revision :)

Thanks,
Konstantin

Last edited 6 months ago by kobenland (previous) (diff)
Note: See TracTickets for help on using tickets.