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: Post comment author]: Add link settings and block supports #35595

Merged
merged 8 commits into from Oct 19, 2021
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 24 additions & 1 deletion packages/block-library/src/post-comment-author/block.json
Expand Up @@ -6,8 +6,31 @@
"parent": [ "core/post-comment" ],
"description": "Post comment author.",
"textdomain": "default",
"attributes": {
"isLink": {
"type": "boolean",
"default": false
},
"linkTarget": {
"type": "string",
"default": "_self"
}
},
"usesContext": [ "commentId" ],
"supports": {
"html": false
"html": false,
"color": {
"gradients": true,
"link": true
},
"typography": {
"fontSize": true,
"lineHeight": true,
"__experimentalFontFamily": true,
"__experimentalFontWeight": true,
"__experimentalFontStyle": true,
"__experimentalTextTransform": true,
"__experimentalLetterSpacing": true
}
}
}
56 changes: 49 additions & 7 deletions packages/block-library/src/post-comment-author/edit.js
Expand Up @@ -3,12 +3,24 @@
*/
import { __ } from '@wordpress/i18n';
import { useSelect } from '@wordpress/data';
import { useBlockProps } from '@wordpress/block-editor';
import { InspectorControls, useBlockProps } from '@wordpress/block-editor';
import { store as coreStore } from '@wordpress/core-data';
import { PanelBody, ToggleControl } from '@wordpress/components';

export default function Edit( { attributes, context } ) {
const { className } = attributes;
/**
* Renders the `core/post-comment-author` block on the editor.
*
* @param {Object} props React props.
* @param {Object} props.setAttributes Callback for updating block attributes.
* @param {Object} props.attributes Block attributes.
* @param {Object} props.context Inherited context.
*
* @return {JSX.Element} React element.
*/
export default function Edit( { attributes, context, setAttributes } ) {
const { className, isLink, linkTarget } = attributes;
const { commentId } = context;
const blockProps = useBlockProps( { className } );

const displayName = useSelect(
( select ) => {
Expand All @@ -21,15 +33,45 @@ export default function Edit( { attributes, context } ) {
const user = getEntityRecord( 'root', 'user', comment.author );
return user?.name ?? __( 'Anonymous' );
}

return authorName ?? '';
},
[ commentId ]
);

const displayAuthor = isLink ? (
<a
href="#comment-author-pseudo-link"
onClick={ ( event ) => event.preventDefault() }
>
{ displayName }
</a>
) : (
<p>{ displayName }</p>
);

return (
<div { ...useBlockProps() }>
<p className={ className }>{ displayName }</p>
</div>
<>
<InspectorControls>
<PanelBody title={ __( 'Link settings' ) }>
<ToggleControl
label={ __( 'Link to authors URL' ) }
onChange={ () => setAttributes( { isLink: ! isLink } ) }
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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?

Copy link
Member

Choose a reason for hiding this comment

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

New issue for unification sounds like a good plan 👍

checked={ isLink }
/>
{ isLink && (
<ToggleControl
label={ __( 'Open in new tab' ) }
onChange={ ( value ) =>
setAttributes( {
linkTarget: value ? '_blank' : '_self',
} )
}
checked={ linkTarget === '_blank' }
/>
) }
</PanelBody>
</InspectorControls>
<div { ...blockProps }>{ displayAuthor }</div>
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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?

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I put the p tag anyway 🙂

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we remove the p on the editor the comments are all misaligned due to padding issues.
Screenshot 2021-10-19 at 16 24 57

What would be a better approach?

  • Leave as it was before this PR. This means no <p> on frontend, but <p> on editor.
  • Add <p> on frontend, to have it both ways the same.
  • Remove <p> 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 <p>, <span>, nothing, <div>

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

Copy link
Member

Choose a reason for hiding this comment

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

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

</>
);
}
2 changes: 1 addition & 1 deletion packages/block-library/src/post-comment-author/index.js
Expand Up @@ -7,7 +7,7 @@ import edit from './edit';
/**
* WordPress dependencies
*/
import { postAuthor as icon } from '@wordpress/icons';
import { commentAuthor as icon } from '@wordpress/icons';

const { name } = metadata;
export { metadata, name };
Expand Down
10 changes: 8 additions & 2 deletions packages/block-library/src/post-comment-author/index.php
Expand Up @@ -11,19 +11,25 @@
* @param array $attributes Block attributes.
* @param string $content Block default content.
* @param WP_Block $block Block instance.
* @return string Return the post comment's content.
* @return string Return the post comment's author.
*/
function render_block_core_post_comment_author( $attributes, $content, $block ) {
if ( ! isset( $block->context['commentId'] ) ) {
return '';
}

$wrapper_attributes = get_block_wrapper_attributes();
$comment_author = get_comment_author( $block->context['commentId'] );
$link = get_comment_author_url( $block->context['commentId'] );

if ( ! empty( $attributes['isLink'] ) && ! empty( $attributes['linkTarget'] ) ) {
$comment_author = sprintf( '<a rel="external nofollow ugc" href="%1s" target="%2s" >%3s</a>', $link, $attributes['linkTarget'], $comment_author );
}

return sprintf(
'<div %1$s>%2$s</div>',
$wrapper_attributes,
get_comment_author( $block->context['commentId'] )
$comment_author
);
}

Expand Down
1 change: 1 addition & 0 deletions packages/icons/src/index.js
Expand Up @@ -49,6 +49,7 @@ export { default as color } from './library/color';
export { default as column } from './library/column';
export { default as columns } from './library/columns';
export { default as comment } from './library/comment';
export { default as commentAuthor } from './library/comment-author';
export { default as cover } from './library/cover';
export { default as create } from './library/create';
export { default as crop } from './library/crop';
Expand Down
22 changes: 22 additions & 0 deletions packages/icons/src/library/comment-author.js
@@ -0,0 +1,22 @@
/**
* WordPress dependencies
*/
import { SVG, Path, Circle } from '@wordpress/primitives';

const commentAuthor = (
<SVG viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg" fill="#FFFFFF">
<Path
d="M18 4H6c-1.1 0-2 .9-2 2v12.9c0 .6.5 1.1 1.1 1.1.3 0 .5-.1.8-.3L8.5 17H18c1.1 0 2-.9 2-2V6c0-1.1-.9-2-2-2zm.5 11c0 .3-.2.5-.5.5H7.9l-2.4 2.4V6c0-.3.2-.5.5-.5h12c.3 0 .5.2.5.5v9z"
fillRule="evenodd"
clipRule="evenodd"
/>
<Path
d="M15 15V15C15 13.8954 14.1046 13 13 13L11 13C9.89543 13 9 13.8954 9 15V15"
fillRule="evenodd"
clipRule="evenodd"
/>
<Circle cx="12" cy="9" r="2" fillRule="evenodd" clipRule="evenodd" />
</SVG>
);

export default commentAuthor;
Expand Up @@ -3,7 +3,10 @@
"clientId": "_clientId_0",
"name": "core/post-comment-author",
"isValid": true,
"attributes": {},
"attributes": {
"isLink": false,
"linkTarget": "_self"
},
"innerBlocks": [],
"originalContent": ""
}
Expand Down