[theme-reviewers] A few issues to look out for
Justin Tadlock
justin at justintadlock.com
Thu Apr 28 05:01:30 UTC 2011
My patch for the comment callback function was just added to WP trunk
for the upcoming TwentyEleven theme:
http://core.trac.wordpress.org/changeset/17722
Maybe it'll be less of an issue when theme authors just copy and paste
code from it.
On 4/27/2011 10:33 PM, Justin Tadlock wrote:
> 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.
>
> 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.
>
> On 4/26/2011 10:33 PM, Chip Bennett wrote:
>> This:
>>
>> /as a matter of official Theme review, I care a great deal about
>> non-core comment types, including "tweetbacks" (even if they are
>> evil)./ :)
>>
>>
>> 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.)?
>>
>> IMHO, that's really the key question to answer.
>>
>> Chip
>>
>> On Wed, Apr 27, 2011 at 10:19 PM, Justin Tadlock
>> <justin at justintadlock.com <mailto:justin at justintadlock.com>> wrote:
>>
>> 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.
>>
>> Put bluntly: /as a matter of official Theme review, I care a
>> great deal about non-core comment types, including "tweetbacks"
>> (even if they are evil)./ :)
>>
>>
>> On 4/26/2011 10:09 PM, Chip Bennett wrote:
>>> Here's an example of my use case:
>>> https://github.com/chipbennett/oenology/blob/master/comments.php
>>>
>>> (And consider that the Guidelines currently *suggest* separating
>>> pings from comments.)
>>>
>>> My primary issue is with this assertion:
>>>
>>> how will this be displayed if a theme is deliberately
>>> overwriting core functionality and not showing the output of
>>> alternate comment types?
>>>
>>>
>>> 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: /as a matter of official
>>> Theme review, I don't care about any non-core comment types,
>>> including "tweetback"/.
>>>
>>> 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.)
>>>
>>> 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.
>>>
>>> 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.
>>>
>>> Chip
>>>
>>> On Wed, Apr 27, 2011 at 9:50 PM, Justin Tadlock
>>> <justin at justintadlock.com <mailto:justin at justintadlock.com>> wrote:
>>>
>>> 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?
>>>
>>> By default, WordPress handles this beautifully. It's only
>>> when a theme overwrites this functionality that it breaks.
>>>
>>> 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.
>>>
>>> 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.
>>>
>>>
>>> On 4/26/2011 9:36 PM, Chip Bennett wrote:
>>>> 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.
>>>>
>>>> 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.
>>>>
>>>> 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.
>>>>
>>>> Or am I missing something?
>>>>
>>>> Chip
>>>>
>>>> On Wed, Apr 27, 2011 at 6:02 PM, Justin Tadlock
>>>> <justin at justintadlock.com
>>>> <mailto:justin at justintadlock.com>> wrote:
>>>>
>>>> I probably didn't explain myself well enough in the
>>>> first email.
>>>>
>>>> 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.
>>>>
>>>> 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.
>>>>
>>>> What's happening here is themes are overriding core
>>>> functionality without handling every scenario that core
>>>> handles by default.
>>>>
>>>>
>>>> On 4/26/2011 3:18 PM, Chip Bennett wrote:
>>>>> Quite possibly. But it is not the responsibility of
>>>>> Themes to account for content added by Plugins.
>>>>>
>>>>> 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.
>>>>>
>>>>> Chip
>>>>>
>>>>> On Tue, Apr 26, 2011 at 3:14 PM, Sayontan Sinha
>>>>> <sayontan at gmail.com <mailto:sayontan at gmail.com>> wrote:
>>>>>
>>>>> Chip,
>>>>> 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.
>>>>>
>>>>> Sayontan.
>>>>>
>>>>>
>>>>> On Tue, Apr 26, 2011 at 12:49 PM, Chip Bennett
>>>>> <chip at chipbennett.net
>>>>> <mailto:chip at chipbennett.net>> wrote:
>>>>>
>>>>> I can't find that 'tweetback' is a core
>>>>> comment type.
>>>>>
>>>>> According to the Codex
>>>>> <http://codex.wordpress.org/Function_Reference/wp_list_comments>,
>>>>> the valid types are: 'all', 'comment',
>>>>> 'trackback', 'pingback', or 'pings'
>>>>>
>>>>> 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.
>>>>>
>>>>> Chip
>>>>>
>>>>>
>>>>> On Tue, Apr 26, 2011 at 2:39 PM, Chip Bennett
>>>>> <chip at chipbennett.net
>>>>> <mailto:chip at chipbennett.net>> wrote:
>>>>>
>>>>> 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.)
>>>>>
>>>>> Chip
>>>>>
>>>>>
>>>>> On Wed, Apr 27, 2011 at 2:16 PM, Justin
>>>>> Tadlock <justin at justintadlock.com
>>>>> <mailto:justin at justintadlock.com>> wrote:
>>>>>
>>>>> Here's a few things we should be on
>>>>> the lookout for when reviewing themes
>>>>> that I thought I'd bring up.
>>>>>
>>>>> 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.
>>>>>
>>>>> 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).
>>>>>
>>>>> 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'.
>>>>> _______________________________________________
>>>>> theme-reviewers mailing list
>>>>> theme-reviewers at lists.wordpress.org
>>>>> <mailto:theme-reviewers at lists.wordpress.org>
>>>>> http://lists.wordpress.org/mailman/listinfo/theme-reviewers
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> theme-reviewers mailing list
>>>>> theme-reviewers at lists.wordpress.org
>>>>> <mailto:theme-reviewers at lists.wordpress.org>
>>>>> http://lists.wordpress.org/mailman/listinfo/theme-reviewers
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Sayontan Sinha
>>>>> http://mynethome.net | http://mynethome.net/blog
>>>>> --
>>>>> Beating Australia in Cricket is like killing a
>>>>> celebrity. The death gets more coverage than the
>>>>> crime.
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> theme-reviewers mailing list
>>>>> theme-reviewers at lists.wordpress.org
>>>>> <mailto:theme-reviewers at lists.wordpress.org>
>>>>> http://lists.wordpress.org/mailman/listinfo/theme-reviewers
>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> theme-reviewers mailing list
>>>>> theme-reviewers at lists.wordpress.org <mailto:theme-reviewers at lists.wordpress.org>
>>>>> http://lists.wordpress.org/mailman/listinfo/theme-reviewers
>>>>
>>>> _______________________________________________
>>>> theme-reviewers mailing list
>>>> theme-reviewers at lists.wordpress.org
>>>> <mailto:theme-reviewers at lists.wordpress.org>
>>>> http://lists.wordpress.org/mailman/listinfo/theme-reviewers
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> theme-reviewers mailing list
>>>> theme-reviewers at lists.wordpress.org <mailto:theme-reviewers at lists.wordpress.org>
>>>> http://lists.wordpress.org/mailman/listinfo/theme-reviewers
>>>
>>> _______________________________________________
>>> theme-reviewers mailing list
>>> theme-reviewers at lists.wordpress.org
>>> <mailto:theme-reviewers at lists.wordpress.org>
>>> http://lists.wordpress.org/mailman/listinfo/theme-reviewers
>>>
>>>
>>>
>>> _______________________________________________
>>> theme-reviewers mailing list
>>> theme-reviewers at lists.wordpress.org <mailto:theme-reviewers at lists.wordpress.org>
>>> http://lists.wordpress.org/mailman/listinfo/theme-reviewers
>>
>> _______________________________________________
>> theme-reviewers mailing list
>> theme-reviewers at lists.wordpress.org
>> <mailto:theme-reviewers at lists.wordpress.org>
>> http://lists.wordpress.org/mailman/listinfo/theme-reviewers
>>
>>
>>
>> _______________________________________________
>> theme-reviewers mailing list
>> theme-reviewers at lists.wordpress.org
>> http://lists.wordpress.org/mailman/listinfo/theme-reviewers
>
>
> _______________________________________________
> theme-reviewers mailing list
> theme-reviewers at lists.wordpress.org
> http://lists.wordpress.org/mailman/listinfo/theme-reviewers
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.wordpress.org/pipermail/theme-reviewers/attachments/20110428/6c35f81a/attachment-0001.htm>
More information about the theme-reviewers
mailing list