[theme-reviewers] A few issues to look out for

Wed Apr 27 13:13:33 UTC 2011

I do like that addition to the switch!

Chip

On Thu, Apr 28, 2011 at 12:01 AM, Justin Tadlock
<justin at justintadlock.com>wrote:

>  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
> > 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
>> > 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> 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>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>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>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> 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
>>>>>>>> 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
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>  --
>>>>> 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
>>>>> http://lists.wordpress.org/mailman/listinfo/theme-reviewers
>>>>>
>>>>>
>>>>
>>>> _______________________________________________
>>>> theme-reviewers mailing listtheme-reviewers at lists.wordpress.orghttp://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 listtheme-reviewers at lists.wordpress.orghttp://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 listtheme-reviewers at lists.wordpress.orghttp://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 listtheme-reviewers at lists.wordpress.orghttp://lists.wordpress.org/mailman/listinfo/theme-reviewers
>
>
> _______________________________________________
> theme-reviewers mailing listtheme-reviewers at lists.wordpress.orghttp://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/20110427/18a1f324/attachment-0001.htm>