Comment Content block: Update placeholder copy #67468
Open
+6
−2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ockham
added
Needs Design Feedback
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +5 B (0%) Total Size: 1.83 MB
ℹ️ View Unchanged
|
ockham
force-pushed
the
update/comment-content-block-placeholder-copy
branch
from
ca50c52 to
0f5f258
Compare
|
Seems like a step in the right direction to me :) |
|
It's a step in the right direction, yes, however I think we can do even better. Keep in mind, this text serves not just the purpose of being a placeholder, it serves as some visual style indication: "your comment will look like this". That's one of the reasons the Post Content placeholder is a fair bit longer, to better indicate: a lot of text might go here. Could we do, instead:
A bit dry still, but captures the spirit of the Post Content block. |
|
I like it! Applied in 050e926. Thank you @jasmussen!
|
|
I guess it looks a bit repeated. I'm okay if others are okay, but just to be sure: would it be possible to show different placeholder content for the nested comments? Perhaps just in the combined block? |
Yeah, I can’t argue with that 😅 FWIW, we’re displaying three nested comments in the Site Editor to illustrate the concept of comments nesting. But we could do things differently, e.g. show only one. Do you think that's preferable?
Hmm, which one do you mean with "combined" block? The top level (i.e. first) one? I'll try and tweak it a bit, but I'm not sure we can easily change the placeholder text in one instance of the Comment Content block but not in the others (or at least not without e.g. requiring an extra block attribute just for the placeholders -- which wouldn't make any sense IMO). |
No, we definitely need to show multiple, to show that comments can nest. What I mean is this, in ASCII mockup: Essentially you'd need to contextually override the placeholder content based on the nesting level, just so this default state is visually improved. |
|
Gotcha, makes sense 👍 Unfortunately, implementing different copy for each placeholder seems prohibitively complex 😕 So I had a look at the code, and it seems like this would require a lot of changes to make it work. The problem is basically that we currently don't have any way to tell the comment content placeholders apart. The component renders a placeholder if the Internally, that block does use negative values for comment IDs to distinguish placeholders; but unfortunately, if we start passing those to its child blocks (rather than replacing them with Finally, this still doesn't really give us any way to know if a block placeholder is nested or not. Even if we assign fixed negative numbers to the placeholders'
Sooo I think it's not worth it 😬 |
|
Yes indeed. Thanks for trying these things, and exploring, it helps map out the territory. Upon reflection, because it's duplicated, we may need to go simple after all. Perhaps we go back, almost to the start, and say:
It's barely different from what's shipping in trunk, but perhaps it threads the needle? Not sure, we can also go all the way back to "This is the.." etc. Outside of the larger improvements outlined, this may be where we have to end up after all. |
|
Edit: Spoke too soon. I'm not getting the first comment in the template, but the currently selected one (which happens to default to the first one). This means that with my latest change, the copy would change depending on which comment you click on. I'll revert. |
|
Could you use React context to detect which Comment Content block is in the thread? If that doable the you could use the context provider to change the value indicating if the comment is top level or not. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Update the placeholder copy of the Comment Content block in the editor to read "This is the Comment Content block." instead of just "Comment Content".
(I'm open to other suggestion 🙂)
Why?
To make it less terse, and more consistent with the Post Content block:
gutenberg/packages/block-library/src/post-content/edit.js
Lines 138 to 152 in 22c43ff
How?
By replacing a string.
Testing Instructions
In the Site Editor, view any template that contains the Comments block (and thus, the Comment Content block). Or insert one.
Screenshots or screencast