Create comments title with simple styling #40419
Conversation
|
Size Change: +935 B (0%) Total Size: 1.23 MB
ℹ️ View Unchanged
|
|
I'm wondering if we should have a "Heading Level" dropdown in the toolbar. This would be consistent with other "title" blocks like Site Title, Query Title, and Post Title. The |
There was a problem hiding this comment.
Nice work @c4rl0sbr4v0 and @justintadlock 👏
I've pushed a couple minor commits:
- Fixing some minor typos and wording
- Fixing the
phpcs-related issues (why the unit tests were failing) - Adding quotation marks around the title so that it in the editor it says
"some title"instead of justsome title.
Aside from this, a couple of other things:
-
Agreed that we could add the "Heading Level" dropdown in the toolbar
-
When the "show post title" is turned off, the title should not say "X responses to" but rather just "X responses".
Screen.Recording.2022-04-18.at.19.13.51.mov
| if ( currentPostId === postId ) { | ||
| setCommentsCount( parseInt( res.headers.get( 'X-WP-Total' ) ) ); | ||
| } | ||
| } ); |
There was a problem hiding this comment.
Shouldn't we .catch() the error here?
| <PlainText | ||
| __experimentalVersion={ 2 } | ||
| tagName="span" | ||
| aria-label={ __( ' Responses to ' ) } | ||
| placeholder={ __( ' Responses to ' ) } | ||
| value={ multipleCommentsLabel } | ||
| onChange={ ( newLabel ) => | ||
| setAttributes( { | ||
| multipleCommentsLabel: newLabel, | ||
| } ) | ||
| } | ||
| /> |
There was a problem hiding this comment.
Why are we using `<PlainText /> here instead of just rendering a plain string?
I see that it is causing weird CSS issues in the editor (the color and opacity are inherited from some unrelated classes for example):
Screen.Recording.2022-04-18.at.19.33.49.mov
There was a problem hiding this comment.
<PlainText /> is a placeholder where you can edit the content. It is used for example in pagination links, where you can edit the text. It seems that the opacity is a way to tell the user that text is editable.
There was a problem hiding this comment.
Hmm, does that mean the user should be able to edit this in the Site Editor? I have tried but can't seem to...
There was a problem hiding this comment.
Yep! That's the idea!
There was a problem hiding this comment.
Hmm, seems great. But for the life of me, I can't seem to edit the Comment Title -- neither in the Site Editor nor the Post Editor... 😕
There was a problem hiding this comment.
@DAreRodz helped me figure this out: These are actual placeholders (like the greyed-out value you see in a form input before you enter anything), so you really just click anywhere on them and start typing.
The important thing is that clicking on them won't set a cursor, nor is it possible to highlight part of it (e.g. by double-clicking) -- because they're placeholders 😅
Here's the screen recording that David shared:
Screen.Recording.2022-04-19.at.19.34.07.mov
I'm not really sure if we should update that automatically, as the text can be edited by the user. |
cbravobernal
requested review from
TimothyBJacobs,
spacedmonkey,
ajitbohra,
mkaz,
ryanwelcher and
juanmaguitar
as code owners
cbravobernal
force-pushed
the
add/comments-title-block
branch
from
a646fcb
to
e5d2a2b
Compare
|
There's one more UX aspect I'd like to change: The bottom toggle in the sidebar
The first two toggles determine if we're including the number and the post title (i.e. if we're rendering the The third toggle however doesn't change what we're displaying; instead, it switches the block in the editor between singular and plural mode, in order to allow editing those.
In order to make that a bit clearer visually, I'll try using a Note that that still won't solve a harder problem: In a number of languages, there is more than one "plural" form. E.g. Czech uses singular for 1 item, nominative plural for 2-4 items, and uses genitive for 5+ items (see). That's precisely the problem that WP's (and UX-wise, this is a rather hard problem (we will probably need to infer how many different "plural" forms exist for a given language, and make each of them editable) that we cannot possibly tackle now. |
|
Per b649cba:
Turns out we also don't need a block attribute, since per the above reasoning, this doesn't change anything on the fronted (but just sets if we're currently editing the singular or plural variant in the editor). |
|
@jasmussen Uh, I just had a realization. "Comments Title" might be a bit misleading -- it's not really a given comment's title (there's no such thing really!), but rather the header on top of a number of comments that says This might have some implications for the icon as well, no? 😅 I guess it would need to look less consistent with Post/Query/... Title icons... 🤔 |
|
Maybe we should rename the block to "Comments Heading" or something like that to differentiate it better from the concept of an enitity's title? |
|
Good question. I'd lean towards "Comments Title" simply because it is related to all the other similar "Title" blocks. But it's not a strong opionion. Alternately we could call the icon file "commentsTitle" but have the block itself be called "Comments Heading". |
|
Thanks @jasmussen! Okay, let's leave it as-is then. Maybe I was influenced too strongly by the Post Title; something like the Archive Title might be actually closer to this block -- which is also a heading for a collection of things 😄 |
|
Not that commentsTitle (plural) could still make sense over the singular commentTitle. |
|
Right, that's what we already have for the block name, but not for the icon. Might make sense to change, yeah 👍 |
|
Just noting why I called this "Comments Title" to start with: Historically, this has been referred to as "comments title" in theme code. Despite there not being a dedicated template tag, past Twenty* themes used it for the heading class and translator's context: <h2 class="comments-title">
_nx( '%s comment', '%s comments', $twenty_twenty_one_comment_count, 'Comments title', 'twentytwentyone' )Plus, this was the naming scheme used for the Archive Title block. |
|
Thanks @justintadlock, makes sense! 😄 Comments Title it is 👍 |
|
I think the line between new features, improvements, and fixes can sometimes be very blurry. In this case, the PR adds a new block, making it easy to think it's a totally new feature. However, the PR originated thanks to feedback fathered during the Beta testing, identifying a missing/unexpected functionality. While not a bugfix (it wasn't broken), I'd lean towards labeling this as a low-risk UX fix/polish and including it in Beta3, but would love to hear from @adamziel and @gziolo, too. |
ndiego
added
the
Backport to WP 6.7 Beta/RC
|
We included it for WP Beta 3. I agree with the reasoning that it's a small part of the bigger block. Still, the size of the PR definitely stands out when compared with all other PRs included in Beta 3. |
Co-authored-by: Michal Czaplinski <mmczaplinski@gmail.com> Co-authored-by: Bernie Reiter <ockham@raz.or.at>
|
Cherry-picked to the |
gziolo
removed
the
Backport to WP 6.7 Beta/RC
What?
Closes #40264
Adds a block to include "X replies to Post" title.
Why?
This way we keep consistency with the previous Comments list of previous templates. Mentioned in WP Tavern.
How?
Adding a new block. I have to include:
One Response toandX responses toTesting Instructions
1.) Add a Comments Query Loop.
2.) Add
Comments Title3.) Edit styling.
4.) Check that the frontend displays the styles chosen in the editor.
Screenshots or screencast