[theme-reviewers] A few issues to look out for
Sayontan Sinha
sayontan at gmail.com
Wed Apr 27 03:31:59 UTC 2011
Basically I am just suggesting some tinkering of the flow. Once you have
done your call to $wp_query->comments_by_type, you have got the array by
comment type. What I am suggesting is that you can move the two blocks of
your code from your mail into a switch block after the call to
$wp_query->comments_by_type. Because, this call is going to return to you
the standard types, 'comment', 'trackback', 'pingback', 'pings' and any
other custom comment type. Once you have the array, you can check which keys
are present, then have your code built out that way.
Think about it this way:
1. You call $wp_query->comments_by_type first. Currently you have it
invoked in line 64, just before your second block. I am suggesting moving it
much higher up in the flow, around line 34.
2. Then you start the switch statement.
3. In your case statement for 'comment' you put in the first block from
below. You can even add in conditions to display it if the array size > 0
4. In your next case statement for 'pings' you put in the second block
from below
5. You can then add the default case type.
Hope this makes sense.
On Tue, Apr 26, 2011 at 8:19 PM, Chip Bennett <chip at chipbennett.net> wrote:
> One of us is misunderstanding the other.
>
> Yes, I know that a switch can have a default case. But that doesn't help,
> if the calls to wp_list_comments() themselves *explicitly declare the
> comment type*.
>
> In my Theme:
>
> <ol class="commentlist">
> <?php wp_list_comments( 'type=comment&avatar_size=40' ); ?>
> </ol>
>
>
> <h3 class='trackbackheader'>Trackbacks</h3>
> <ol class="trackbacklist">
> <?php wp_list_comments( array( 'type' => 'pings', 'callback' => 'oenology_comment_list_pings' ) ); ?>
> </ol>
>
>
> So, where, in those two calls, will some arbitrary, non-core comment type
> be displayed?
>
> Chip
>
>
> On Tue, Apr 26, 2011 at 10:15 PM, Sayontan Sinha <sayontan at gmail.com>wrote:
>
>> 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.
>>
>>
>> _______________________________________________
>> 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/69f2f2c8/attachment-0001.htm>
More information about the theme-reviewers
mailing list