Post Comments block - add style supports for text and background settings. #24080
Conversation
|
Size Change: +108 B (0%) Total Size: 1.15 MB
ℹ️ View Unchanged
|
Addison-Stavlo
changed the title
Addison-Stavlo
requested review from
ajitbohra,
Soean and
talldan
as code owners
| <p key={ comment.id }>{ comment.content.raw }</p> | ||
| <p key={ comment.id }> | ||
| <RawHTML>{ comment.content.raw }</RawHTML> | ||
| </p> |
There was a problem hiding this comment.
Wrapping RawHTML inside a p is semantically incorrect because RawHTML renders as a div.
I think this could be improved in several ways:
-
Use the comment content's
renderedproperty instead ofraw.
comment.content.rawis what the user submitted (or, more precisely, what has been saved in the DB after sanitization), and it's supposed to be used when editing the comment.
comment.content.renderedis what WP uses when displaying the comment.
In this case, we are not editing but displaying, sorenderedis more appropriate.
It also automatically wraps paragraphs (as in: blocks of text separated by 2 newlines) inpelements! ✨ -
Pass a CSS class to the mapped comment (something like
.wp-block-post-comments__comment), which should make it easier to style it.
Not strictly needed in this PR, but eventually we will need to visually separate comments between each other! -
Drop the
pand just return a map ofRawHTML.
Eventually it would look somewhat like this:
return comments.map( comment =>
<RawHTML className="wp-block-post-comments__comment" key={ comment.id }>
{ comment.content.rendered }
</RawHTML>
);There was a problem hiding this comment.
Wrapping RawHTML inside a p is semantically incorrect because RawHTML renders as a div.
I was under the impression this was not the case:
gutenberg/packages/element/src/raw-html.js
Lines 23 to 24 in 24c6114
There was a problem hiding this comment.
Use the comment content's rendered property instead of raw.
I do think this makes sense. I will have to test it out. I wonder why it was using raw in the first place.
There was a problem hiding this comment.
I was under the impression this was not the case:
Yeah, I kinda think that comment is not exactly correct. 😄
Either way, we would be passing more props (the key, possibly a className), so the div should better show up!
(it does in my tests)
There was a problem hiding this comment.
Awesome, these suggestions make things work much better. Thanks!
The rendered content also has <br/>s in it as well which makes it look much better and more in-line with what is on the front end.
| return ( | ||
| <Warning> | ||
| { __( | ||
| 'Post comments block: Comments are only available in posts. Please add this block to a post instead.' |
There was a problem hiding this comment.
I don't really agree with this, as it's not strictly true.
I'd rather we checked if the post type (or even the post!) supports comments, than just relying on this check.
There was a problem hiding this comment.
I removed this extra check for now, so currently it will just fall back to 'No comments.' in that odd case.
I'd rather we checked if the post type (or even the post!) supports comments
That sounds like a good way to do this! Although I'm not sure how to go about checking that currently and it may be best to wait as it seems like this block will require some significant restructuring/design in the future (#24101).
There was a problem hiding this comment.
Oh yes definitely!
Right now I see this block as almost like a placeholder for the comments loop, so I think it's reasonable to keep chiseling it into shape.
The feature support check should be a major concern for when we move to the multipage editing, especially considering how extensible everything in WP is.
Anything could be added or removed from any post types, or even posts (e.g. comments can be disabled on specific posts with the Allow Comments toggle in the document sidebar).
But, this affects almost all FSE blocks, so we'll need to figure out a proper solution that fits most/all use cases.
tl;dr this PR is not the most appropriate place to figure it out. 🙂
Description
Adds some style support for the Post Comments block:
As well as fixes an issue where links in the post comments were not appearing as links in the editor:
Note - there may be a handful of issues between editor and front-end parity for this block and some future work to be done ahead as noted in #24101
How has this been tested?
Verify these settings persist across the editor and front-end.
Screenshots
Types of changes
Checklist: