[wp-trac] [WordPress Trac] #24288: Timestamps in chat formats should have IDs

[WordPress Trac]

#24288: Timestamps in chat formats should have IDs
---------------------------+--------------------
 Reporter:  aaroncampbell  |       Owner:
     Type:  defect (bug)   |      Status:  new
 Priority:  normal         |   Milestone:  3.6
Component:  Post Formats   |     Version:  trunk
 Severity:  normal         |  Resolution:
 Keywords:  has-patch      |
---------------------------+--------------------

Comment (by toscho):

 Looks good.

 `if ( $timeslug == $prev_timeslug )` should use strict comparison: `if (
 $timeslug === $prev_timeslug )`. Slightly faster.

 I am not so sure about the deep nesting level. Nested `foreach` constructs
 are usually a cry for separate functions. :)

 The very long line with `sprintf()` would be more readable broken down
 into separate lines:

 {{{
 $time = sprintf(
         '<time class="chat-timestamp" id="%1$s"><a href="#%1$s"
 tabindex="-1">%2$s</a></time>',
         esc_attr( 'time_' . $timeslug ),
         esc_html( $row['time'] )
 );
 }}}

 Not part of your patch, but could be fixed here too: the white space in
 `</cite>: </dt>` should be removed.
 By default `dt` is a block element, and the white space will be removed by
 the parser. When `dt` and `dd` are rendered as inline elements, the space
 should be controlled by CSS; extra white space is rather difficult to
 handle then. We had this problem with the comment form, if I remember that
 correctly.

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/24288#comment:9>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software