[theme-reviewers] A few issues to look out for
Sayontan Sinha
sayontan at gmail.com
Wed Apr 27 03:15:32 UTC 2011
In your code you have this:
$comments_by_type = $wp_query->comments_by_type;
That is going to return an array with the comment_type as the key and the
comments as the value. This will include custom comment types.
So, if you are iterating over this:
foreach ($comments_by_type as $comment_type => $comments) {
switch ($comment_type) {
case 'comment':
// comment processing, including wp_list_comments
break;
case 'trackback':
// trackback processing, including wp_list_comments
break;
case 'pingback':
// pingback processing, including wp_list_comments
break;
default:
wp_list_comments('type' => $comment_type); // you are
simply passing the $comment_type to it.
break;
}
}
On Tue, Apr 26, 2011 at 8:09 PM, Chip Bennett <chip at chipbennett.net> 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 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.wordpress.org/pipermail/theme-reviewers/attachments/20110426/42d2ff9b/attachment-0001.htm>
More information about the theme-reviewers
mailing list