Make WordPress Core

Opened 16 years ago

Closed 16 years ago

#8091 closed enhancement (fixed)

"Parent" Comment Class

Reported by: themeshaper's profile ThemeShaper Owned by:
Milestone: 2.7 Priority: normal
Severity: normal Version: 2.7
Component: Comments Keywords: has-patch Walker_Comment
Focuses: Cc:

Description

A comment <li> with "children" should get a class of "parent".

Attachments (2)

parent_class.diff (1.1 KB) - added by filosofo 16 years ago.
consistent_start_el_args.diff (1.4 KB) - added by filosofo 16 years ago.

Download all attachments as: .zip

Change History (15)

#1 @Viper007Bond
16 years ago

+1

Extra CSS classes can never hurt and it makes it easier to select parents for styling purposes.

#2 @Viper007Bond
16 years ago

Hmm.

+1 to idea

-1 after looking at the code. It doesn't seem easily doable unless I'm missing something.

#3 @Viper007Bond
16 years ago

  • Keywords needs-patch added
  • Milestone changed from 2.7 to 2.8
  • Version set to 2.7

Pushing back to 2.8.

#4 @filosofo
16 years ago

  • Keywords has-patch Walker_Comment added; needs-patch removed
  • Milestone changed from 2.8 to 2.7

A two-line patch isn't too bad for 2.7, right?

#5 @Viper007Bond
16 years ago

Wow, I feel stupid. I totally forgot comment_class() accepted parameters. I was looking at comment_class() itself trying to figure out how it could know if there were children or not (ugly globals, SQL queries, etc.).

#6 follow-up: @DD32
16 years ago

$args[0]['has_children'] = ( empty( $children_elements[$element->$id_field] ) ) ? false : true; 

Eugh.. Can i just say i'm sick of if statements(let alone condensed if statements) with boolean return values.. I can understand for something complex, But seriously:

$args[0]['has_children'] = ! empty( $children_elements[$element->$id_field] );

reads much cleaner to me.

Sorry for the mini rant.. Its just a small anoyance of mine :) (aside from extra brackets around boolean questions)

#7 in reply to: ↑ 6 @filosofo
16 years ago

Replying to DD32:

reads much cleaner to me.

You're absolutely right. I refreshed the patch.

#8 @markjaquith
16 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [9812]) Add a "parent" class to comment LIs with children. props filosofo. fixes #8091

#9 @westi
16 years ago

  • Keywords needss-patch added; has-patch removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

I'm reverting this for now as it introduces errors - #8303

#10 @westi
16 years ago

That is reverting the change to classes.php

#11 @westi
16 years ago

Make that re-opening.... (need coffee)

#12 @filosofo
16 years ago

  • Keywords has-patch added; needss-patch removed

Patch consistent_start_el_args.diff fixes this on two levels:

  • It checks to make sure that $args[0] is actually an array. The empty check was pointless, since it should never be empty.
  • If fixes a more fundamental problem: walk_page_tree() calls the "walk" method with arguments in an inconsistent order. One of these is not like the others in argument order, and it's the "start_el" method for Walker_Page (defined on line 1155 of wp-includes/classes.php):

wp-admin/includes/template.php:412: function start_el(&$output, $category, $depth, $args) {
wp-includes/comment-template.php:1151: function start_el(&$output, $comment, $depth, $args) {
wp-includes/classes.php:782: function start_el(&$output) {}
wp-includes/classes.php:1155: function start_el(&$output, $page, $depth, $current_page, $args) {
wp-includes/classes.php:1233: function start_el(&$output, $page, $depth, $args) {
wp-includes/classes.php:1310: function start_el(&$output, $category, $depth, $args) {
wp-includes/classes.php:1425: function start_el(&$output, $category, $depth, $args) {

#13 @westi
16 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [9830]) Make arguments to start_el consistent and ensure that page walking is consistent. Fixes #8303 and #8091 props filosofo.

Note: See TracTickets for help on using tickets.