That's probably something worth a bit more explanation, either as a post on the Make site, or in the Guidelines, for what to look for.<div><br></div><div>Chip<br><br><div class="gmail_quote">On Wed, Apr 27, 2011 at 10:33 PM, Justin Tadlock <span dir="ltr"><<a href="mailto:justin@justintadlock.com">justin@justintadlock.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div text="#000000" bgcolor="#ffffff">
I also check that themes handle public posts types and taxonomies
too, at least to the degree that a theme can handle those things. I
assumed this was something everyone else was doing as well. I do
this with every theme I review.<br>
<br>
For example, if a theme had a filter on 'single_template' that
didn't take into account custom post types and failed to display a
post of a custom post type at all, I'd point that out in the review
and suggest a fix.<div><div></div><div class="h5"><br>
<br>
On 4/26/2011 10:33 PM, Chip Bennett wrote:
<blockquote type="cite">This:
<div><br>
</div>
<blockquote style="margin:0pt 0pt 0pt 40px;border:medium none;padding:0px">
<div>
<i>as a matter of official Theme review, I care a great deal
about non-core comment types, including "tweetbacks" (even
if they are evil).</i> :)</div>
</blockquote>
<div>
<div><br>
</div>
<div>But, why? Why should Theme Review be concerned with
arbitrary, non-core content? And, why only comment types? Why
not other types of content that could have arbitrary types
added (taxonomies, posts, etc.)?</div>
<div><br>
</div>
<div>IMHO, that's really the key question to answer.</div>
<div><br>
</div>
<div>Chip<br>
<br>
<div class="gmail_quote">On Wed, Apr 27, 2011 at 10:19 PM,
Justin Tadlock <span dir="ltr"><<a href="mailto:justin@justintadlock.com" target="_blank">justin@justintadlock.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204, 204, 204);padding-left:1ex">
<div text="#000000" bgcolor="#ffffff"> Your use case is
perfectly fine by me. That scenario is not what I've
been talking about. However, it too can handle custom
comment types with a little tweaking.<br>
<br>
Put bluntly: <i>as a matter of official Theme review, I
care a great deal about non-core comment types,
including "tweetbacks" (even if they are evil).</i> :)
<div>
<div><br>
<br>
On 4/26/2011 10:09 PM, Chip Bennett wrote:
<blockquote type="cite">Here's an example of my use
case:
<div> <a href="https://github.com/chipbennett/oenology/blob/master/comments.php" target="_blank">https://github.com/chipbennett/oenology/blob/master/comments.php</a></div>
<div><br>
</div>
<div>(And consider that the Guidelines currently
*suggest* separating pings from comments.)</div>
<div><br>
</div>
<div>My primary issue is with this assertion:</div>
<div><br>
</div>
<blockquote style="margin:0pt 0pt 0pt 40px;border:medium none;padding:0px">
<div> how will this be displayed if a theme is
deliberately overwriting core functionality
and not showing the output of alternate
comment types?</div>
</blockquote>
<div><br>
</div>
<div>Passing a valid argument to a core function
is not "overwriting core functionality". Those
arbitrary, "alternate" comment types *aren't
part of core*. Put bluntly: <i>as a matter of
official Theme review, I don't care about any
non-core comment types, including "tweetback"</i>.</div>
<div><br>
</div>
<div>Again: if a Plugin adds a custom comment
type, then the *Plugin* is responsible for
either hooking that custom content into the
Theme, or else for providing instructions to the
end user for how to incorporate that custom
content. (Yes, it might mean instructing the
user to add a call to wp_list_comments(
'type=tweetback' ). I see no problem with that.)</div>
<div><br>
</div>
<div>I don't agree that WordPress "handles it
beautifully", because, aesthetically speaking, I
think that pings mixed in with comments looks
utterly horrid. Seeing "tweetbacks" mixed in
with comments AND pings would look even worse.</div>
<div><br>
</div>
<div>On the other hand: I do agree with you that
all code should be added deliberately.
Copy/pasting TwentyTen's comments callback
should be done deliberately. And we should
absolutely be checking such a Theme's
comment-list output, to ensure that it is
appropriate - and wherever possible, helping to
educate Theme developers on the proper usage and
powerful potential of implementing such custom
callbacks. Such effort will only be to the
benefit of end users.</div>
<div><br>
</div>
<div>Chip<br>
<br>
<div class="gmail_quote">On Wed, Apr 27, 2011 at
9:50 PM, Justin Tadlock <span dir="ltr"><<a href="mailto:justin@justintadlock.com" target="_blank">justin@justintadlock.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204, 204, 204);padding-left:1ex">
<div text="#000000" bgcolor="#ffffff"> My
question is: If a plugin adds a custom
comment type (for example, Facebook
comments, tweetbacks, or something of the
sort), how will this be displayed if a
theme is deliberately overwriting core
functionality and not showing the output
of alternate comment types?<br>
<br>
By default, WordPress handles this
beautifully. It's only when a theme
overwrites this functionality that it
breaks.<br>
<br>
The fix is really quite simple for most
themes. Just create a default case in
that copy-pasted switch statement used in
about 90% of the themes based off
TwentyTen's comment system.<br>
<br>
I'm not suggesting we make a new guideline
here. I'm just suggesting we be on the
lookout for this in themes where devs just
copy/paste comment callback functions
without giving it much thought. I could
certainly understand an intentional design
choice to exclude custom comment types.
Whatever we decide, I'll be sure to
continue educating theme authors on this
because it is a legitimate problem that
themes create for plugin authors.
<div>
<div><br>
<br>
On 4/26/2011 9:36 PM, Chip Bennett
wrote:
<blockquote type="cite">If a Theme is
providing callback output for
'comment', 'pingback', and
'trackback', then it IS handling
every core comment type; thus, I
disagree that a Theme is not
"handling every scenario that core
handles by default". A Theme cannot
know what a Plugin might possibly
hook into, or what content it might
provide.
<div> <br>
</div>
<div>Now, if a Theme provided
callback functions for only
'comment' comment types, but not
for 'pings' (or if it accounted
for 'trackback' but not
'pingback', or something similar),
then I would agree. </div>
<div><br>
</div>
<div>One of the most common features
is for a Theme to separate
comments form pings. That very act
of separation - however
accomplished - would require
explicitly declaring 'comment' and
'pings' comment types. Thus, it
would not be using the 'all'
comment type. And thus, such
Themes would no longer be flexible
enough to handle some non-core
comment type added by a Plugin.</div>
<div><br>
</div>
<div>Or am I missing something?</div>
<div><br>
</div>
<div>Chip<br>
<br>
<div class="gmail_quote">On Wed,
Apr 27, 2011 at 6:02 PM, Justin
Tadlock <span dir="ltr"><<a href="mailto:justin@justintadlock.com" target="_blank">justin@justintadlock.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204, 204, 204);padding-left:1ex">
<div text="#000000" bgcolor="#ffffff"> I
probably didn't explain
myself well enough in the
first email.<br>
<br>
We're not looking at the
"type" parameter of
wp_list_comments(). We're
looking at the "callback"
parameter here. This is
where a theme is overriding
core functionality. If the
theme didn't override this
functionality with a custom
function, WordPress would
display other comment types
by default. <br>
<br>
A plugin cannot be
responsible for
incorporating custom comment
types if a theme is
purposely not allowing
comments of a custom type to
show. There's no hook to
allow a plugin to override
what a theme is doing
there. Even if there was a
hook there, this would be a
major problem if a plugin
was changing how a theme
handled the display of
comments.<br>
<br>
What's happening here is
themes are overriding core
functionality without
handling every scenario that
core handles by default.
<div>
<div><br>
<br>
On 4/26/2011 3:18 PM,
Chip Bennett wrote:
<blockquote type="cite">Quite
possibly. But it is
not the responsibility
of Themes to account
for content added by
Plugins.
<div><br>
</div>
<div>I see no reason
to require Themes to
support a non-core
'tweetback'
comment-type. If a
Plugin adds this
comment-type, then
the Plugin should be
responsible for
incorporating it.</div>
<div><br>
</div>
<div>Chip<br>
<br>
<div class="gmail_quote">On
Tue, Apr 26, 2011
at 3:14 PM,
Sayontan Sinha <span dir="ltr"><<a href="mailto:sayontan@gmail.com" target="_blank">sayontan@gmail.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204, 204, 204);padding-left:1ex"> Chip,<br>
I believe Justin
is referring to
the fact that
plugins can add
the type
"tweetback". If
that is the
case, then a
theme that is
explicitly
checking only
for "comment",
"pingback" and
"trackback" is
missing out on
the ones that
don't fall into
these buckets,
i.e. it is
missing a
catch-all for
types introduced
by plugins.<br>
<br>
Sayontan.
<div>
<div><br>
<br>
<div class="gmail_quote">On
Tue, Apr 26,
2011 at 12:49
PM, Chip
Bennett <span dir="ltr"><<a href="mailto:chip@chipbennett.net" target="_blank">chip@chipbennett.net</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204, 204, 204);padding-left:1ex"> I can't
find that
'tweetback' is
a core comment
type.
<div><br>
</div>
<div><a href="http://codex.wordpress.org/Function_Reference/wp_list_comments" target="_blank">According
to the Codex</a>,
the valid
types are: <span style="font-family:'Lucida Grande',Verdana,'Bitstream Vera Sans',Arial,sans-serif;font-size:12px;line-height:22px">'all',
'comment',
'trackback',
'pingback', or
'pings'</span></div>
<div><br>
</div>
<div>So, if a
Theme accounts
for these
types, that
should be
sufficient.
For instance,
if a Theme
accounts for
'comments' and
'pings', all
bases are
covered.</div>
<div><br>
</div>
<div><font color="#888888">Chip</font>
<div>
<div><br>
<br>
<div class="gmail_quote">On
Tue, Apr 26,
2011 at 2:39
PM, Chip
Bennett <span dir="ltr"><<a href="mailto:chip@chipbennett.net" target="_blank">chip@chipbennett.net</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204, 204, 204);padding-left:1ex"> Hmm...
I don't think
I've yet seen
a Theme that
explicitly
handles
tweetbacks.
(Honestly, I
didn't even
realize such a
comment type
existed.)
<div><br>
</div>
<div><font color="#888888">Chip</font>
<div>
<div><br>
<br>
<div class="gmail_quote">On
Wed, Apr 27,
2011 at 2:16
PM, Justin
Tadlock <span dir="ltr"><<a href="mailto:justin@justintadlock.com" target="_blank">justin@justintadlock.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204, 204, 204);padding-left:1ex">Here's a
few things we
should be on
the lookout
for when
reviewing
themes that I
thought I'd
bring up.<br>
<br>
The use of
the_post_thumbnail()
with
the_content()
can sometimes
be a problem.
If a user
places the
image within
the post
content (at
the beginning
of the post)
and sets the
same image as
the "feature
image," it
creates a
duplicate
image issue.
Some themes'
designs are
meant to
handle this
while others
aren't.<br>
<br>
Some themes
have a
comments
callback
function where
they don't
recognize
comment types
other than
'comment',
'pingback',
and
'trackback'.
This is also
the case in
the TwentyTen
theme. If you
look at its
switch
statement,
you'll notice
it doesn't
give a
'default'
case. It
should be
corrected to
handle all
comment types
(e.g.,
tweetbacks).<br>
<br>
Loading JS and
CSS on all
pages of the
admin.
Sometimes,
themes hook
their theme
settings page
JavaScript and
Stylesheet to
the
'admin_init'
hook or
something
similar. This
should only be
loaded on the
the theme
settings page.
If using the
add_theme_page()
function, a
hook is
created just
for that page.
A better hook
would probably
be
'load-appearance_page_$pagename'.<br>
_______________________________________________<br>
theme-reviewers
mailing list<br>
<a href="mailto:theme-reviewers@lists.wordpress.org" target="_blank">theme-reviewers@lists.wordpress.org</a><br>
<a href="http://lists.wordpress.org/mailman/listinfo/theme-reviewers" target="_blank">http://lists.wordpress.org/mailman/listinfo/theme-reviewers</a><br>
</blockquote>
</div>
<br>
</div>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</div>
</div>
<br>
_______________________________________________<br>
theme-reviewers
mailing list<br>
<a href="mailto:theme-reviewers@lists.wordpress.org" target="_blank">theme-reviewers@lists.wordpress.org</a><br>
<a href="http://lists.wordpress.org/mailman/listinfo/theme-reviewers" target="_blank">http://lists.wordpress.org/mailman/listinfo/theme-reviewers</a><br>
<br>
</blockquote>
</div>
<br>
<br clear="all">
<br>
</div>
</div>
-- <br>
Sayontan Sinha<br>
<a href="http://mynethome.net" target="_blank">http://mynethome.net</a> | <a href="http://mynethome.net/blog" target="_blank">http://mynethome.net/blog</a><br>
<font color="#888888">
--<br>
Beating
Australia in
Cricket is
like killing a
celebrity. The
death gets
more coverage
than the
crime.<br>
<br>
</font><br>
_______________________________________________<br>
theme-reviewers
mailing list<br>
<a href="mailto:theme-reviewers@lists.wordpress.org" target="_blank">theme-reviewers@lists.wordpress.org</a><br>
<a href="http://lists.wordpress.org/mailman/listinfo/theme-reviewers" target="_blank">http://lists.wordpress.org/mailman/listinfo/theme-reviewers</a><br>
<br>
</blockquote>
</div>
<br>
</div>
<pre><fieldset></fieldset>
_______________________________________________
theme-reviewers mailing list
<a href="mailto:theme-reviewers@lists.wordpress.org" target="_blank">theme-reviewers@lists.wordpress.org</a>
<a href="http://lists.wordpress.org/mailman/listinfo/theme-reviewers" target="_blank">http://lists.wordpress.org/mailman/listinfo/theme-reviewers</a>
</pre>
</blockquote>
</div>
</div>
</div>
<br>
_______________________________________________<br>
theme-reviewers mailing list<br>
<a href="mailto:theme-reviewers@lists.wordpress.org" target="_blank">theme-reviewers@lists.wordpress.org</a><br>
<a href="http://lists.wordpress.org/mailman/listinfo/theme-reviewers" target="_blank">http://lists.wordpress.org/mailman/listinfo/theme-reviewers</a><br>
<br>
</blockquote>
</div>
<br>
</div>
<pre><fieldset></fieldset>
_______________________________________________
theme-reviewers mailing list
<a href="mailto:theme-reviewers@lists.wordpress.org" target="_blank">theme-reviewers@lists.wordpress.org</a>
<a href="http://lists.wordpress.org/mailman/listinfo/theme-reviewers" target="_blank">http://lists.wordpress.org/mailman/listinfo/theme-reviewers</a>
</pre>
</blockquote>
</div>
</div>
</div>
<br>
_______________________________________________<br>
theme-reviewers mailing list<br>
<a href="mailto:theme-reviewers@lists.wordpress.org" target="_blank">theme-reviewers@lists.wordpress.org</a><br>
<a href="http://lists.wordpress.org/mailman/listinfo/theme-reviewers" target="_blank">http://lists.wordpress.org/mailman/listinfo/theme-reviewers</a><br>
<br>
</blockquote>
</div>
<br>
</div>
<pre><fieldset></fieldset>
_______________________________________________
theme-reviewers mailing list
<a href="mailto:theme-reviewers@lists.wordpress.org" target="_blank">theme-reviewers@lists.wordpress.org</a>
<a href="http://lists.wordpress.org/mailman/listinfo/theme-reviewers" target="_blank">http://lists.wordpress.org/mailman/listinfo/theme-reviewers</a>
</pre>
</blockquote>
</div>
</div>
</div>
<br>
_______________________________________________<br>
theme-reviewers mailing list<br>
<a href="mailto:theme-reviewers@lists.wordpress.org" target="_blank">theme-reviewers@lists.wordpress.org</a><br>
<a href="http://lists.wordpress.org/mailman/listinfo/theme-reviewers" target="_blank">http://lists.wordpress.org/mailman/listinfo/theme-reviewers</a><br>
<br>
</blockquote>
</div>
<br>
</div>
</div>
<pre><fieldset></fieldset>
_______________________________________________
theme-reviewers mailing list
<a href="mailto:theme-reviewers@lists.wordpress.org" target="_blank">theme-reviewers@lists.wordpress.org</a>
<a href="http://lists.wordpress.org/mailman/listinfo/theme-reviewers" target="_blank">http://lists.wordpress.org/mailman/listinfo/theme-reviewers</a>
</pre>
</blockquote>
</div></div></div>
<br>_______________________________________________<br>
theme-reviewers mailing list<br>
<a href="mailto:theme-reviewers@lists.wordpress.org">theme-reviewers@lists.wordpress.org</a><br>
<a href="http://lists.wordpress.org/mailman/listinfo/theme-reviewers" target="_blank">http://lists.wordpress.org/mailman/listinfo/theme-reviewers</a><br>
<br></blockquote></div><br></div>