[wp-trac] [WordPress Trac] #17748: Twenty Eleven code review & consistancy

[WordPress Trac]

#17748: Twenty Eleven code review & consistancy
---------------------------+-----------------------
 Reporter:  dd32           |       Owner:  azaozz
     Type:  defect (bug)   |      Status:  reopened
 Priority:  normal         |   Milestone:  3.2
Component:  Bundled Theme  |     Version:  3.2
 Severity:  normal         |  Resolution:
 Keywords:                 |
---------------------------+-----------------------

Old description:

> I've just done a review of the Twenty Eleven code looking for anything
> out of place or inconsistent with other pages.
>
> attached is a diff of problems which I'm going to walk through:
>  * ~~ archive.php - doesnt need to call the_post(); only uses global
> query conditionals.~~
>  * ~~ author.php - comment typo~~
>  * comments.php - has to check get_option( 'page_comments' ) ? Shouldn't
> that be included within get_comment_pages_count() instead to short-
> circuit it?
>  * ~~ content-aside.php, content-link.php, content-image.php, content-
> quote.php,  - use get_post_type() instead of $post->post_type~~
>  * ~~ content-gallery.php - indentation (trivial I know)~~
>  * content-image.php & a few others - Doesnt use {{{<span class="sep"> |
> </span>}}} between utility entries?
>  * content-single.php doesnt have a post = post_type conditional on date
> meta like most of the rest of the content items.
>  * ~~ content-status.php - Dead code, and avoid using the post type
> itself~~
>  * content.php - ~~Applies to most files, using 'echo=0' on a few
> items,~~ $show_sep code is rather convuluted as well, perhaps appending
> to an array and imploding it would be cleaner, This style of code is used
> in a few places, but not all.
>  * ~~image.php - Has verbose Comments vs. Trackbacks being closed text,
> This text isnt used on other pages, only mention of Trackbacks are on
> this template too~~
>  * ~~inc/theme-options.php - Use $hook_suffix within an hook name instead
> of attaching to the generic action~~
>  * ~~single.php() - Dead code~~
>
> Most of this really just needs a sanity check on what is supposed to
> apply, and apply it to the rest of the template files.

New description:

 I've just done a review of the Twenty Eleven code looking for anything out
 of place or inconsistent with other pages.

 attached is a diff of problems which I'm going to walk through:
  * ~~ archive.php - doesnt need to call the_post(); only uses global query
 conditionals.~~
  * ~~ author.php - comment typo~~
  * ~~ comments.php - has to check get_option( 'page_comments' ) ?
 Shouldn't that be included within get_comment_pages_count() instead to
 short-circuit it? ~~ See #17778
  * ~~ content-aside.php, content-link.php, content-image.php, content-
 quote.php,  - use get_post_type() instead of $post->post_type~~
  * ~~ content-gallery.php - indentation (trivial I know)~~
  * content-image.php & a few others - Doesnt use {{{<span class="sep"> |
 </span>}}} between utility entries?
  * content-single.php doesnt have a post = post_type conditional on date
 meta like most of the rest of the content items.
  * ~~ content-status.php - Dead code, and avoid using the post type
 itself~~
  * content.php - ~~Applies to most files, using 'echo=0' on a few items,
 $show_sep code is rather convuluted as well, perhaps appending to an array
 and imploding it would be cleaner, This style of code is used in a few
 places, but not all.~~ $show_sep is stuck there for the time being,
 array+imploding can't be done with all of the meta's due to echo-only
 items. Strcuture/indentation could clean it up
  * ~~image.php - Has verbose Comments vs. Trackbacks being closed text,
 This text isnt used on other pages, only mention of Trackbacks are on this
 template too~~
  * ~~inc/theme-options.php - Use $hook_suffix within an hook name instead
 of attaching to the generic action~~
  * ~~single.php() - Dead code~~

 Most of this really just needs a sanity check on what is supposed to
 apply, and apply it to the rest of the template files.

--

Comment (by dd32):

 Another one:
  7. UI / String - Edit link, (Edit) for pingbacks, [Edit] for Comments,
 and Edit (with a grey rounded rectangle background) for
 Posts/Pages/Attachments etc.
   * Merge strings? Go for all Edit, or just merge the Comment/Pingback
 ones to [Edit]
   * If merge all, leaves us with a styling issue, I vote to style them the
 same as Post Edit links and float to the upper right of the comment. See
 [http://dd32.id.au/files/wordpress/temp/2011-comments.jpg
 2011-comments.jpg]
   * The edit link in archive views is in the Lower left, In singular views
 it's in the Upper Right.
  8. Child comments have the Author on it's own line, See
 [http://dd32.id.au/files/wordpress/temp/2011-comments.jpg
 2011-comments.jpg] again for that, Single line leaves whitespace however.

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