Post Comments Form: Add "proper" visual representation in the editor #40368
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
|
Size Change: +162 B (0%) Total Size: 1.23 MB
ℹ️ View Unchanged
|
|
Is it okay if I write 2-3 automated tests for this in a separate PR and we possibly merge it into this PR? |
@ribaricplusplus Sure, go for it! |
13 tasks
DAreRodz
approved these changes
Apr 21, 2022
There was a problem hiding this comment.
Looks great, @michalczaplinski! 🙌
Just a minor note. I'm wondering if it would make sense to add a warning message (like in #40484), as the styles/structure in the editor could not match those in the frontend.
priethor
reviewed
Apr 21, 2022
There was a problem hiding this comment.
Thanks for tackling this, @michalczaplinski !
I'm experiencing a strange behavior when selecting the block, as I can click on the button and set the focus on it. The bounding box changes although the block selection doesn't as the submit button is not an inner block.
Grabacion.de.pantalla.2022-04-21.a.las.13.38.06.mov
I wonder if we should make it a more obvious placeholder instead of showing a form similar to the frontend (which, as @DAreRodz mentioned, might not be the same and can create style issues between editor and frontend). @jasmussen do you have any recommendations based on your experience with other block placeholders?
|
Nice one, thank you for the ping. Yes, I see the same: the button is selectable but not editable, and I can even set focus there: In general, I think that it's good that the editor can match the frontend in this case, visually. But I would agree it can be a bit misleading that the button appears interactive, even though it shouldn't be. In this case, it might give me the impression that I could edit the text of the button, just like I can edt the text of a Button block (which would be cool, by the way!) — but I can't. Would it be possible to make the entire block disabled, but still look the same? That way you could still select the form itself, but not the button inside. There are a few ways to do this though I'd appreciate dev input. I know we have a Disabled component that works as a wrapper, but just recently I saw a conversation around |
|
Yes, we could disable the form with diff --git a/packages/block-library/src/post-comments-form/edit.js b/packages/block-library/src/post-comments-form/edit.js
index c419a592e1..67717b220b 100644
--- a/packages/block-library/src/post-comments-form/edit.js
+++ b/packages/block-library/src/post-comments-form/edit.js
@@ -14,6 +14,7 @@ import {
} from '@wordpress/block-editor';
import { useEntityProp } from '@wordpress/core-data';
import { __, sprintf } from '@wordpress/i18n';
+import { __experimentalUseDisabled as useDisabled } from '@wordpress/compose';
export default function PostCommentsFormEdit( {
attributes,
@@ -36,6 +37,8 @@ export default function PostCommentsFormEdit( {
const isInSiteEditor = postType === undefined || postId === undefined;
+ const disabledFormRef = useDisabled();
+
return (
<>
<BlockControls group="block">
@@ -70,7 +73,11 @@ export default function PostCommentsFormEdit( {
{ ( 'open' === commentStatus || isInSiteEditor ) && (
<div className="wp-block-post-comments-form">
<h3>{ __( 'Leave a Reply' ) }</h3>
- <form noValidate className="comment-form">
+ <form
+ noValidate
+ className="comment-form"
+ ref={ disabledFormRef }
+ >
<p>
<label htmlFor="comment">
{ __( 'Comment' ) } |
|
☝️ With the change above, the "Post Comment" button stops being selectable. Would that be enough? It's the first time I use |
|
I just applied the previous diff in 5785b73. I also removed the |
|
I've also added @SantosGuillamot's suggestion #40484 (review) to the #40484 PR, which makes it even more image-like. |
ockham
reviewed
Apr 21, 2022
* Tests * Change tests directory * Remove second test * Fix test
Co-authored-by: Michael Burridge <mburridge@users.noreply.github.com>
5785b73 to
63cf602
Compare
|
Rebased 🙂 |
ockham
reviewed
Apr 21, 2022
|
Thanks for helping out everyone 🙂 I think all the issues here have been addressed but I ll wait till tomorrow with merging to see if there's anything else. The main thing still left to do (in a follow-up PR) is to apply the appropriate logic for the warnings as mentioned by Luis in #40484 (comment) |
Yeah. And we can also reuse the form template, as mentioned by @ockham in #40484 (review). |
ockham
reviewed
Apr 22, 2022
gziolo
pushed a commit
that referenced
this pull request
Apr 25, 2022
…40368) Co-authored-by: Bruno Ribarić <43731400+ribaricplusplus@users.noreply.github.com> Co-authored-by: Michael Burridge <mburridge@users.noreply.github.com> Co-authored-by: David Arenas <david.arenas@automattic.com> Co-authored-by: Bernie Reiter <ockham@raz.or.at>
|
Cherry-picked to the |
14 tasks
talldan
reviewed
Jan 27, 2023
|
|
||
| describe( 'placeholder', () => { | ||
| it( 'displays in site editor even when comments are closed by default', async () => { | ||
| await setOption( 'default_comment_status', 'closed' ); |
There was a problem hiding this comment.
Should this be reset to the previous value again after the test run has complete?
There was a problem hiding this comment.
I think it should, so I've updated the test in this PR.
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?
Adding UI elements to represent the Post Comments Form in the editor. Fixing #26999
Why?
Currently, in the editor, the Post Comments form just renders a placeholder saying "Post Comments Form". This is bad UX and we're planning to add the Post Comments Form to the Comment Template in #40256 so would need it to have a "proper" visual representation.
How?
Adding elements that are close to how the Post Comments Form looks in the frontend.
Testing Instructions
none yet.
Screenshots or screencast
This is a screenshot from the editor (still work in progress):
