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

Add Latest Comments block. #7369

Closed
wants to merge 20 commits into from
Closed
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions core-blocks/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import * as coverImage from './cover-image';
import * as embed from './embed';
import * as freeform from './freeform';
import * as html from './html';
import * as latestComments from './latest-comments';
import * as latestPosts from './latest-posts';
import * as list from './list';
import * as more from './more';
Expand Down Expand Up @@ -65,6 +66,7 @@ export const registerCoreBlocks = () => {
...embed.others,
freeform,
html,
latestComments,
latestPosts,
more,
nextpage,
Expand Down
100 changes: 100 additions & 0 deletions core-blocks/latest-comments/edit.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/**
* WordPress dependencies
*/
import { Component, Fragment } from '@wordpress/element';
import {
PanelBody,
RangeControl,
ToggleControl,
ServerSideRender,
} from '@wordpress/components';
import { __ } from '@wordpress/i18n';

Copy link
Member

Choose a reason for hiding this comment

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

This empty line can be removed.

import {
InspectorControls,
BlockAlignmentToolbar,
BlockControls,
} from '@wordpress/editor';

/**
* Internal dependencies.
*/
import './editor.scss';

const MIN_COMMENTS = 1;
const MAX_COMMENTS = 100;

class latestComments extends Component {
Copy link
Member

Choose a reason for hiding this comment

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

It should start with the upper-case. We are adding missing docs for this to make it more formal. See #7670.

constructor() {
super( ...arguments );
this.toggleHandler = this.toggleHandler.bind( this );
this.changeCommentsToShow = this.changeCommentsToShow.bind( this );
}

toggleHandler( propName ) {
return () => {
Copy link
Member

@gziolo gziolo Jul 11, 2018

Choose a reason for hiding this comment

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

Ideally, this arrow function should be promoted to a class method and bind to this instead of toggleHandler to avoid unnecessary rerenders.

I'd have to think how to code it differently :)

Copy link
Member

Choose a reason for hiding this comment

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

In this case the whole element re-renders from server-side data though, right? 🤷‍♂️

Copy link
Member

Choose a reason for hiding this comment

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

Wait, scratch that, I see. I think for now it's okay as we use this pattern elsewhere in the codebase, but if it's a performance issue we could see about fixing it everywhere. I'm afraid I've recommended this in the past, not realising the potential for unneeded re-renders...

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I saw it in other places, too. I don't think this is a big issue for leaf components. However, it's something we should try to avoid whenever it doesn't cause too much work.

const value = this.props.attributes[ propName ];
const { setAttributes } = this.props;

setAttributes( { [ propName ]: ! value } );
};
}

changeCommentsToShow( commentsToShow ) {
const { setAttributes } = this.props;

setAttributes( { commentsToShow: parseInt( commentsToShow, 10 ) || 0 } );
}

render() {
const { setAttributes } = this.props;
const { align, displayAvatar, displayTimestamp } = this.props.attributes;

return (
<Fragment>
<BlockControls key="controls">
Copy link
Member

Choose a reason for hiding this comment

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

There is no need to add key prop when you use Fragment component wrapper.

<BlockAlignmentToolbar
value={ align }
onChange={ ( nextAlign ) => {
setAttributes( { align: nextAlign } );
} }
controls={ [ 'left', 'center', 'right', 'wide', 'full' ] }
Copy link
Member

Choose a reason for hiding this comment

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

I think those are default values and can be omitted in here.

Copy link
Member

Choose a reason for hiding this comment

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

yes, confirmed:

const DEFAULT_CONTROLS = [ 'left', 'center', 'right', 'wide', 'full' ];

/>
</BlockControls>
<InspectorControls key="inspector">
Copy link
Member

Choose a reason for hiding this comment

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

key should be removed. See above.

<PanelBody title={ __( 'Latest Comments Settings' ) } />
Copy link
Member

Choose a reason for hiding this comment

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

I guess that in this case PanelBody should wrap all controls like it happens in other blocks.


<ToggleControl
label={ __( 'Display avatar' ) }
checked={ displayAvatar }
onChange={ this.toggleHandler( 'displayAvatar' ) }
/>

<ToggleControl
label={ __( 'Display timestamp' ) }
checked={ displayTimestamp }
onChange={ this.toggleHandler( 'displayTimestamp' ) }
/>

<ToggleControl
label={ __( 'Display excerpt' ) }
checked={ this.props.attributes.displayExcerpt }
onChange={ this.toggleHandler( 'displayExcerpt' ) }
/>

<RangeControl
label={ __( 'Number of comments to show' ) }
value={ this.props.attributes.commentsToShow }
onChange={ ( value ) => this.changeCommentsToShow( value ) }
min={ MIN_COMMENTS }
max={ MAX_COMMENTS }
/>

</InspectorControls>
<ServerSideRender key="latest-comments" block="core/latest-comments" attributes={ this.props.attributes } />
Copy link
Member

Choose a reason for hiding this comment

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

key should be removed. See above.

</Fragment>
);
}
}

export default latestComments;
33 changes: 33 additions & 0 deletions core-blocks/latest-comments/editor.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
.wp-block-latest-comments.alignright {
margin: 0 7em 2em;
}
.wp-block-latest-comments.alignleft {
margin-right: 2em;
}

.wp-block-latest-comments {
a {
pointer-events: none;
cursor: default;
}

.has-avatars img {
margin-right: 10px;
}
}

.edit-post-visual-editor {
.wp-block-latest-comments__comment-excerpt p {
font-size: 14px;
line-height: 1.8;
padding-top: 0;
margin: 5px 0 20px;
}

.wp-block-latest-comments {

Copy link
Member

Choose a reason for hiding this comment

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

This empty line can be removed.

.has-avatars > li {
min-height: 36px;
}
}
}
48 changes: 48 additions & 0 deletions core-blocks/latest-comments/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/**
* WordPress dependencies.
*/
import { __ } from '@wordpress/i18n';

/**
* Internal dependencies.
*/
import './style.scss';
import edit from './edit';

export const name = 'core/latest-comments';

export const settings = {
title: __( 'Latest Comments' ),

description: __( 'Shows a list of your site\'s most recent comments.' ),

icon: 'list-view',

category: 'widgets',

keywords: [ __( 'recent comments' ) ],

supports: {
html: false,
},

defaultAttributes: {
Copy link
Member

Choose a reason for hiding this comment

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

defaultAttributes is not supported. It should be removed as it takes no effect.

commentsToShow: 5,
displayAvatar: true,
displayExcerpt: true,
displayTimestamp: true,
},

getEditWrapperProps( attributes ) {
const { align } = attributes;
if ( 'left' === align || 'right' === align || 'wide' === align || 'full' === align ) {
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't actually include "center", which caused a bug:

screenshot 2018-07-12 12 26 51

I have a fix incoming which actually DRYs this up a bit too:

screenshot 2018-07-12 12 27 25

return { 'data-align': align };
}
},

edit: edit,

save() {
return null;
},
};
131 changes: 131 additions & 0 deletions core-blocks/latest-comments/index.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
<?php
/**
* Server-side rendering of the `core/latest-comments` block.
*
* @package gutenberg
*/

/**
* Renders the `core/latest-comments` block on server.
*
* @param array $attributes The block attributes.
*
* @return string Returns the post content with latest comments added.
*/
function gutenberg_render_block_core_latest_comments( $attributes = array() ) {

// Basic attribute validation.
if (
! is_numeric( $attributes['commentsToShow'] ) ||
$attributes['commentsToShow'] < 0 ||
$attributes['commentsToShow'] > 100
) {
$attributes['commentsToShow'] = 5;
}

$align = 'center';
if ( isset( $attributes['align'] ) && in_array( $attributes['align'], array( 'left', 'right', 'wide', 'full' ), true ) ) {
$align = $attributes['align'];
}

/** This filter is documented in wp-includes/widgets/class-wp-widget-recent-comments.php */
$comments = get_comments( apply_filters( 'widget_comments_args', array(
'number' => $attributes['commentsToShow'],
'status' => 'approve',
'post_status' => 'publish',
) ) );

$list_items_markup = '';
if ( ! empty( $comments ) ) {

// Prime cache for associated posts. This is copied from \WP_Widget_Recent_Comments::widget().
$post_ids = array_unique( wp_list_pluck( $comments, 'comment_post_ID' ) );
_prime_post_caches( $post_ids, strpos( get_option( 'permalink_structure' ), '%category%' ), false );

foreach ( $comments as $comment ) {
$list_items_markup .= '<li class="recentcomments">';
if ( $attributes['displayAvatar'] ) {
$avatar = get_avatar( $comment, 48, '', '', array(
'class' => 'wp-block-latest-comments__comment-avatar',
) );
if ( $avatar ) {
$list_items_markup .= $avatar;
}
}

$list_items_markup .= '<div class="comment-data"';
$author_url = get_comment_author_url( $comment );
if ( empty( $author_url ) && ! empty( $comment->user_id ) ) {
$author_url = get_author_posts_url( $comment->user_id );
}
if ( $author_url ) {
$list_items_markup .= '<a class="wp-block-latest-comments__comment-author" href="' . esc_url( $author_url ) . '">' . get_comment_author( $comment ) . '</a>';
} else {
$list_items_markup .= '<a class="wp-block-latest-comments__comment-author">' . get_comment_author( $comment ) . '</a>';
}

$list_items_markup .= __( ' on ', 'gutenberg' );
$list_items_markup .= '<a class="wp-block-latest-comments__comment-link" href="' . esc_url( get_comment_link( $comment ) ) . '">' . get_the_title( $comment->comment_post_ID ) . '</a>';

if ( $attributes['displayTimestamp'] ) {
$list_items_markup .= sprintf(
'<time datetime="%1$s" class="wp-block-latest-comments__comment-timestamp">%2$s</time>',
esc_attr( get_comment_date( 'c', $comment ) ),
esc_html( get_comment_date( '', $comment ) )
);
}
if ( $attributes['displayExcerpt'] ) {
$list_items_markup .= '<div class="wp-block-latest-comments__comment-excerpt">' . wpautop( get_comment_excerpt( $comment ) ) . '</div>';
}
$list_items_markup .= '</div></li>';
}
}

$class = "wp-block-latest-comments align{$align}";
if ( $attributes['displayAvatar'] ) {
$class .= ' has-avatars';
}
if ( $attributes['displayTimestamp'] ) {
$class .= ' has-timestamps';
}
if ( $attributes['displayExcerpt'] ) {
$class .= ' has-excerpts';
}

$block_content = sprintf(
'<ul class="%1$s">%2$s</ul>',
esc_attr( $class ),
$list_items_markup
);

return $block_content;
}

register_block_type( 'core/latest-comments', array(
'attributes' => array(
'className' => array(
Copy link
Member

Choose a reason for hiding this comment

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

I see some differences between default values here and those specified in defaultAttributes which I commented about before. Those values get propagated from the server to the client, so should be treated as a source of truth.

'type' => 'string',
),
'commentsToShow' => array(
'type' => 'number',
'default' => 5,
),
'displayAvatar' => array(
'type' => 'boolean',
'default' => false,
),
'displayExcerpt' => array(
'type' => 'boolean',
'default' => false,
),
'displayTimestamp' => array(
'type' => 'boolean',
'default' => false,
),
'align' => array(
'type' => 'string',
'default' => 'center',
Copy link
Member

Choose a reason for hiding this comment

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

This was causing blocks with unset alignment to look centre aligned on a new page load. I'll remove it and the default assignment to centre-alignment above.

),
),
'render_callback' => 'gutenberg_render_block_core_latest_comments',
) );
59 changes: 59 additions & 0 deletions core-blocks/latest-comments/style.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
.wp-block-latest-comments {
padding-left: 2.5em;

&__comment-timestamp {
Copy link
Member

Choose a reason for hiding this comment

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

wp-block-latest-comments __comment-timestamp should be moved to top level as its own definition. It should be also consolidated with rules from lines 23-26.

color: $dark-gray-100;
font-size: 12px;
padding: 0 2px;
}

li {
line-height: 1.1;
}

&.has-avatars > li {
min-height: 36px;
list-style: none;

.comment-data {
margin-left: 52px;
}
}

&__comment-timestamp {
display: block;
padding-top: 8px;
}

.wp-block-latest-comments__comment-excerpt p {
Copy link
Member

Choose a reason for hiding this comment

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

Those styles should be also moved to top level. In general, there is no need to nest classes which use the same pattern.

font-size: 14px;
line-height: 1.8;
margin: 5px 0 20px;
}

.recentcomments {
Copy link
Member

Choose a reason for hiding this comment

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

Is recentcomments something that is named for backward compatibility?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changing to latestcomments, was not intentional.

line-height: 2.2;
}

&.has-timestamps,
&.has-excerpts {
.recentcomments {
line-height: 1.5;
}
}
}

.wp-block-latest-comments > li {
Copy link
Member

Choose a reason for hiding this comment

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

We have styles for the same element in line 10, they should be consolidated there.

margin-bottom: 1em;
font-size: 15px;
}

.wp-block-latest-comments .avatar,
.wp-block-latest-comments__comment-avatar {
display: block;
float: left;
height: 40px;
width: 40px;
border-radius: 24px;
margin-right: 12px;
}
1 change: 1 addition & 0 deletions core-blocks/test/fixtures/core__latest-comments.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<!-- wp:latest-comments {"displayAvatar":true,"displayExcerpt":true,"displayTimestamp":true} /-->
Loading