WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 5 years ago

Last modified 5 months ago

#20088 closed enhancement (fixed)

Improve wp_list_comments() comment markup

Reported by: lancewillett Owned by: markjaquith
Milestone: 3.6 Priority: normal
Severity: normal Version: 3.3.1
Component: Comments Keywords: has-patch twentythirteen dev-feedback
Focuses: Cc:

Description (last modified by lancewillett)

The default comment markup generated by wp_list_comments() needs a small revamp. This will potentially help theme authors avoid using their own comment callback functions by incorporating a structure popularized by the last two default themes.

Twenty Twelve, for example, could almost drop the custom comments callback in functions.php with these improvements, with the exception of not being able to use HTML5 semantic elements instead of divs.

Patch has code cleanup and improvements to the comment-template.php file, including:

start_lvl()

  • Use double quotes for HTML output

end_lvl()

  • Add in .children end element HTML comments

start_el()

  • Add switch statement for handling pingback/trackback first, to run before "other" types of comments
  • Add pingback/trackback HTML output based on Twenty Ten / Eleven: much simpler than normal comments
  • Wrap comment_text() in a div element so it can be styled separately
  • Change comment-awaiting-moderation to be wrapped in a paragraph element, removing presentational line break
  • Use updated comment-meta data based on Twenty Ten / Eleven: include an author link (if it exists), better time/date formatting, and use esc_url instead of htmlspecialchars
  • Add in several end element HTML comments
  • Fix spacing around parens
  • Better indenting for PHP and HTML code throughout

end_el()

  • Fix spacing around parens
  • Add in #comment-## end element HTML comments

Attachments (9)

20088.diff (5.3 KB) - added by lancewillett 6 years ago.
20088.2.diff (5.2 KB) - added by lancewillett 6 years ago.
Second pass, revert changes to existing class values and markup
20088.3.diff (5.2 KB) - added by lancewillett 6 years ago.
Better back compat markup for pingback/trackback case
20088.4.diff (5.2 KB) - added by lancewillett 6 years ago.
Use esc_url properly, props kobenland
20088.5.diff (6.9 KB) - added by obenland 5 years ago.
20088.6.diff (8.1 KB) - added by obenland 5 years ago.
20088.7.diff (8.2 KB) - added by obenland 5 years ago.
20088.8.diff (8.2 KB) - added by georgestephanis 5 years ago.
20088.9.diff (8.3 KB) - added by obenland 5 years ago.

Download all attachments as: .zip

Change History (35)

@lancewillett
6 years ago

#1 @nacin
6 years ago

comment-awaiting-moderation was added in #15206. I agree that the <br/> is lame.

Curious to hear about any backwards compatibility concerns and considerations.

#2 @lancewillett
6 years ago

Back compat concerns are class and ID name changes, which I considered and tried to keep as minimal as possible.

If themes are not currently using a custom callback they might need small CSS tweaks to selectors for .comment-body.

#3 @lancewillett
6 years ago

Also, if this lands in trunk soonish, I could test it out with all the WP.com themes to see if there are serious back compat issues.

#4 @lancewillett
6 years ago

  • Description modified (diff)

@lancewillett
6 years ago

Second pass, revert changes to existing class values and markup

#5 @lancewillett
6 years ago

I redid my changes after a bunch of testing on WP.com, with themes that do not have a custom comment callback.

The patch now is back to simply improving code quality, and only adding in classes and markup, not changing existing markup and styles.

@lancewillett
6 years ago

Better back compat markup for pingback/trackback case

#6 @lancewillett
6 years ago

New patch handles back compat for pingbacks/trackbacks.

Found a small issue for back compat: the pingback element should be a div with class of "comment-body" so that previous styles won't break.

Props chellycat (Michelle Langston) for help in testing and for catching the pingback class issue.

Tested with themes that don't use a custom comments callback: Brand New Day, Chunk, Clean Home, Contempt, Dusk to Dawn, Fleur, Greyzed, Grid Focus, Manifest, Oulipo, Quintus, Retro Mac OS, Selecta, Structure, and Wu Wei.

#7 follow-up: @kobenland
6 years ago

  • Cc obenland@… added

Out of curiosity (in 20088.3.diff):

  • Is there a reason you used esc_html() rather than esc_url() on line 1374?
  • Would it make sense to use wp_parse_args() on line 1388?

@lancewillett
6 years ago

Use esc_url properly, props kobenland

#8 in reply to: ↑ 7 @lancewillett
6 years ago

Replying to kobenland:

  • Is there a reason you used esc_html() rather than esc_url() on line 1374?

Good eye, fixed in new patch.

  • Would it make sense to use wp_parse_args() on line 1388?

I don't think it helps in this case. What do you think is the benefit?

#9 @kobenland
6 years ago

I thought for consistency with how merging arguments in WP is handled. But you're right, it'll use array_merge() anyway at some point.

#10 @nacin
5 years ago

  • Milestone changed from Awaiting Review to 3.5

We need this to not change any existing themes. That means new arguments for wp_list_comments().

Thoughts:

  • You can currently specify 'style' => div, ol, or ul. You should also be able to specify tags and wrappers to the point where you can re-create the markup in Twenty Twelve.
  • Repositioning the body inside a div might require a wrapper => false by default.
  • For pingbacks, you should be able to specify 'ping_style' => 'concise' or something along those lines.
  • We may be able to get away with converting <em class="comment-awaiting-moderation"></em><br /> to <p class="comment-awaiting-moderation"></p>. May.

Beyond those rough guidelines put together in about 5 minutes, I'd just see what you can come up with.

#11 @nacin
5 years ago

  • Keywords needs-patch added; has-patch dev-feedback removed

#12 @lancewillett
5 years ago

Added #22005 for Twenty Twelve compatibility. It's really close to freeze for 1.0 (today!) so we might need to punt.

@obenland
5 years ago

#13 @obenland
5 years ago

  • Keywords has-patch added; needs-patch removed

Built 20088.5.diff off of Lance's patch and I think I hit four out of four on Nacin's guidelines. :)

Two things I'm not completely happy with yet:

  • Naming of the new arguments for wp_list_comments()
  • Switch of the old and new way of displaying the comment meta part

I'd love to hear you opinions!

Last edited 5 years ago by obenland (previous) (diff)

#14 @nacin
5 years ago

  • Milestone changed from 3.5 to Future Release

#15 @nacin
5 years ago

  • Keywords 3.6-early twentythirteen added

#16 @rachelbaker
5 years ago

  • Cc rachelbaker added

#17 @bpetty
5 years ago

  • Keywords 3.6-early removed
  • Milestone changed from Future Release to 3.6

@obenland
5 years ago

#18 @obenland
5 years ago

  • Keywords dev-feedback added

New patch, introducing protected methods for better back/forward compat.

@obenland
5 years ago

#19 @lancewillett
5 years ago

See also #23128 for retrofitting Twenty Twelve a bit, for better "bypostauthor" comment markup.

#20 @georgestephanis
5 years ago

RE: #15080 and [23667] we may want to change the default $format from an empty string to 'xhtml' for clarity's sake.

#21 @georgestephanis
5 years ago

(Patch attached changed 'markup' argument default from empty string to xhtml and tweaked the comment a touch.

No functional changes.

@obenland
5 years ago

#22 @obenland
5 years ago

Latest patch switches to 'format' as designation for HTML5 support, to be in line with #15080 and #15081. Also adopts the 'div-comment-##' in HTML5 comments for the reply form to be added inside the <li>.

#23 @markjaquith
5 years ago

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

In 23694:

Improve wp_list_comments() markup.

  • Whitespace cleanup
  • Accepts format=html5 which uses some of the HTML5 elements
  • Some helpful HTML comments to help untangle the HTML
  • Other misc code cleanup

props lancewillett, obenland, georgestephanis. fixes #20088.

#24 @lancewillett
5 years ago

In 23696:

Twenty Thirteen: remove custom comment callback in favor of core comment_form() defaults. Props obenland, closes #22005. See #20088.

#25 @SergeyBiryukov
4 years ago

#14697 was marked as a duplicate.

#26 @faustjonson
5 months ago

a duplicate https://www.essayhave.com/blog/wp-content/uploads/2017/07/smile1.png

Last edited 5 months ago by faustjonson (previous) (diff)
Note: See TracTickets for help on using tickets.