Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Block Library: Add new Edit Comment Link block #35965

Merged
merged 20 commits into from
Oct 27, 2021

Conversation

cbravobernal
Copy link
Contributor

@cbravobernal cbravobernal commented Oct 26, 2021

Description

Fixes #30577
Replaces PR #30671. Props to @diedexx for the initial work.

Adds a Edit Comment Link block which can be used inside a Post Comment block which renders the edit link if the logged in user has the right capabilities to edit the comment.

How has this been tested?
I've placed a Post Comment block on a post and selected an id of an existing comment. In that block, I've placed the new Edit Comment block and modified all available properties in the sidebar and checked if these changes were reflected on the published post.->

Screenshots

Screenshot 2021-10-26 at 18 26 31

Video summary:
https://www.loom.com/share/30e0fa4f01fb4f418ee16f1dcfca5aaa

Types of changes

New feature - New block type: Edit Comment.

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.

@gziolo gziolo added [Block] Comment Template Affects the Comment Template Block New Block Suggestion for a new block labels Oct 27, 2021
@gziolo gziolo mentioned this pull request Oct 27, 2021
7 tasks
@gziolo
Copy link
Member

gziolo commented Oct 27, 2021

There are some conflicts to resolve in lib/blocks.php after I merged the changes for the Comments Author Avatar block.

$link_atts = '';

if ( ! empty( $attributes['linkTarget'] ) ) {
$link_atts .= sprintf( 'target="%s"', $attributes['linkTarget'] );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also escape the link target in case it gets updated manually in the post content.

@gziolo gziolo changed the title Add/edit comment block Block Library: Add new Edit Comment Link block Oct 27, 2021
@gziolo gziolo added the [Package] Block library /packages/block-library label Oct 27, 2021
@gziolo
Copy link
Member

gziolo commented Oct 27, 2021

This is what I see when testing.

Frontend (logged in):
Screen Shot 2021-10-27 at 13 59 24

Frontend (logged out):
Screen Shot 2021-10-27 at 14 05 01

Editor:
Screen Shot 2021-10-27 at 13 59 04

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent work. Everything tests well.

There are two known bugs that I could observe here as well. They are related to how Global Styles work. A detailed explanation is included in the PR with the Comment Reply Link block: #35774 (comment).

As a follow-up, we can also add a text align control to mirror how the Comment Reply Link works.

@gziolo gziolo merged commit 70ee670 into WordPress:trunk Oct 27, 2021
@github-actions github-actions bot added this to the Gutenberg 11.9 milestone Oct 27, 2021
@cbravobernal cbravobernal deleted the add/edit-comment-block branch October 27, 2021 13:31
import { __ } from '@wordpress/i18n';

export default function Edit( {
attributes: { className, linkTarget },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the className attribute passed always passed to the blocks's edit method in a way that I'm not aware of? Otherwise, I think you won't need it here because it's not defined in the block.json

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two features enabled by default for every block:
https://github.com/WordPress/gutenberg/blob/trunk/docs/reference-guides/block-api/block-supports.md#classname
https://github.com/WordPress/gutenberg/blob/trunk/docs/reference-guides/block-api/block-supports.md#customclassname

The second one injects a custom class name using a className attribute. I think that the handling is now automated with useBlockProps so you no longer need to explicitly pass it to useBlockProps, but it was a requirement for blocks in the past. I think we should remove className handling here to avoid confusion.

Copy link
Member

@gziolo gziolo Oct 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to test it because I see the same usage as here also in other blocks. It's very inconsistent across blocks.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we can safely remove the className handling from here. It is redundant. Well, it doesn't break anything. I'm opening a PR soon with more changes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, I see! Thanks, Greg!

@andrewserong andrewserong added the [Type] Feature New feature to highlight in changelogs. label Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Comment Template Affects the Comment Template Block New Block Suggestion for a new block [Package] Block library /packages/block-library [Type] Feature New feature to highlight in changelogs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Edit Comment Link Block
5 participants