[theme-reviewers] A few issues to look out for
Chip Bennett
chip at chipbennett.net
Wed Apr 27 03:39:38 UTC 2011
I'm totally not following.
'type=comment' will exclude *anything* else. So, no matter what I put in the
callback, no other comment type, core or custom, will be included in the
output.
And, why would I add in another call to wp_list_comments() - which I assume
your #5 would require, in order to account for the "default" type - if I've
already accounted for output of all core comment types?
Note that this is a deliberate design decision (and one that is currently
*recommended* in the Guidelines) to *completely separate* comments and pings
into separate lists. Under such a design, a switch doesn't help at all.
Chip
On Tue, Apr 26, 2011 at 10:31 PM, Sayontan Sinha <sayontan at gmail.com> wrote:
> 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.
>
>
> _______________________________________________
> 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/20110426/ab42ee6f/attachment-0001.htm>
More information about the theme-reviewers
mailing list