[Block: Post comment author]: Add link settings and block supports #35595

cbravobernal · 2021-10-13T14:45:01Z

Description

Closes #30572.

Used #30573 as a reference.

How has this been tested?

Add a post comment block (using an existing comment ID)
Add an inner post comment author block
Confirm that the color and typography options work
Confirm that the link option works.
Confirm that the target blank/self works.

Screenshots

Loom with a summary:
https://www.loom.com/share/924ffd9e1d9d469ea4bbba6fe6529219

Source code generated on the frontend:

Types of changes

Enhancement

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
I've tested my changes with keyboard and screen readers.
My code has proper inline documentation.
I've included developer documentation if appropriate.
I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

github-actions · 2021-10-13T14:45:28Z

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @c4rl0sbr4v0! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

cbravobernal · 2021-10-14T10:58:17Z

As the task is quite similar to #30573, and @carolinan and @gziolo both worked on that.
Would you mind taking it a look?
Thanks!

gziolo

I had some questions but overall this looks great. It’s very similar to the PR you linked 👍

gziolo · 2021-10-14T11:09:24Z

packages/block-library/src/post-comment-author/edit.js

+				<PanelBody title={ __( 'Link settings' ) }>
+					<ToggleControl
+						label={ __( 'Link to authors URL' ) }
+						onChange={ () => setAttributes( { isLink: ! isLink } ) }


Should it also reset the link target when link gets disabled?

I copied that attribute from Post Title block. There is no reset when you enable/disable the link.

Of course I can add a reset, but maybe we could create an issue to unify on all blocks with the same attribute to have the same behaviour?

New issue for unification sounds like a good plan 👍

gziolo · 2021-10-14T11:19:54Z

packages/block-library/src/post-comment-author/edit.js

+					) }
+				</PanelBody>
+			</InspectorControls>
+			<div { ...blockProps }>{ displayAuthor }</div>


The HTML generated doesn’t use  tag anymore. Is that intentional?

It was, but now that you mention, I will edit in order to have it again.

Is there any guide about how a block html structure should be?

My best answer would be to ask theme experts @carolinan and @aristath for advice 😃

I put the p tag anyway 🙂

Sure, we can iterate on it since we don't save the HTML output in the post/theme content, but use render_callback on the server.

It looks like there is no  on the frontend when there is no link, so let's remove it here, too.

If we remove the p on the editor the comments are all misaligned due to padding issues.

What would be a better approach?

Leave as it was before this PR. This means no  on frontend, but  on editor.

Add  on frontend, to have it both ways the same.

Remove  in both, use CSS to add a padding (personally I don't like this one really much).

Add an option so users can choose what they want to show on frontend , , nothing, <div>

All feedback will be appreciated! I will leave by the moment as first option, as it was previously.

Let's leave the old behavior and address it seperately.

packages/block-library/src/post-comment-author/index.js

gziolo · 2021-10-14T11:25:34Z

Loom with a summary:
https://www.loom.com/share/924ffd9e1d9d469ea4bbba6fe6529219

This is very useful. Thank you for including. In this type of PRs it would be helpful to show also source code generated on the frontend.

gziolo · 2021-10-19T12:42:53Z

~~The link on the frontend references the homepage. I see it also in the screenshot from the description. Is that the expected URL for the admin user?~~

Update: everything is correct. It read the URL from the user's profile. When I added another user and a comment it links to the URL is set in the profile 👍🏻

gziolo · 2021-10-19T12:55:00Z

With link

Editor:

Frontend:

No link

Editor:

Frontend:

gziolo

Everything looks good in my testing. The only thing that would be nice to unify between the frontend and the editor is the  tag used when there is no link as noted in https://github.com/WordPress/gutenberg/pull/35595/files#r731835989.

cbravobernal requested a review from ajitbohra as a code owner October 13, 2021 14:45

github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Oct 13, 2021

gziolo reviewed Oct 14, 2021

View reviewed changes

gziolo requested a review from carolinan October 14, 2021 11:25

cbravobernal force-pushed the update/post-comment-author branch from 1be5200 to 4527237 Compare October 14, 2021 13:37

This was referenced Oct 15, 2021

Comment Content Block: Add typography, color and padding support #35183

Merged

Comments Loop Block: Tracking issue #34994

Closed

Carlos Bravo added 6 commits October 18, 2021 17:08

Update post comment author icon

848bb3a

Update post comment author with new options

3c0e791

Refactor to move the link wrapper outside the select function

95a0735

Fix linter comma trailing on php

e1824c7

Fixed some comments

4d53bf0

Add p tag inside, renamed icon

f9c26e4

cbravobernal force-pushed the update/post-comment-author branch from 4527237 to f9c26e4 Compare October 18, 2021 15:09

gziolo approved these changes Oct 19, 2021

View reviewed changes

Carlos Bravo added 2 commits October 19, 2021 16:08

Unify removing p tag on both editor and frontend

db11cc3

Reset p to previous status

bd3ff13

gziolo merged commit fac0447 into WordPress:trunk Oct 19, 2021

github-actions bot added this to the Gutenberg 11.8 milestone Oct 19, 2021

cbravobernal deleted the update/post-comment-author branch October 19, 2021 20:31

michalczaplinski mentioned this pull request Oct 22, 2021

Site Editor: Add new Comments Query Loop block #35231

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Block: Post comment author]: Add link settings and block supports #35595

[Block: Post comment author]: Add link settings and block supports #35595

cbravobernal commented Oct 13, 2021 •

edited

github-actions bot commented Oct 13, 2021

cbravobernal commented Oct 14, 2021

gziolo left a comment

gziolo Oct 14, 2021

cbravobernal Oct 14, 2021

gziolo Oct 14, 2021

gziolo Oct 14, 2021

cbravobernal Oct 14, 2021

gziolo Oct 14, 2021

cbravobernal Oct 15, 2021

gziolo Oct 15, 2021

gziolo Oct 19, 2021

cbravobernal Oct 19, 2021 •

edited

gziolo Oct 19, 2021

gziolo commented Oct 14, 2021

gziolo commented Oct 19, 2021 •

edited

gziolo commented Oct 19, 2021

gziolo left a comment

[Block: Post comment author]: Add link settings and block supports #35595

[Block: Post comment author]: Add link settings and block supports #35595

Conversation

cbravobernal commented Oct 13, 2021 • edited

Description

How has this been tested?

Screenshots

Types of changes

Checklist:

github-actions bot commented Oct 13, 2021

cbravobernal commented Oct 14, 2021

gziolo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cbravobernal Oct 19, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gziolo commented Oct 14, 2021

gziolo commented Oct 19, 2021 • edited

gziolo commented Oct 19, 2021

With link

No link

gziolo left a comment

Choose a reason for hiding this comment

cbravobernal commented Oct 13, 2021 •

edited

cbravobernal Oct 19, 2021 •

edited

gziolo commented Oct 19, 2021 •

edited