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

Comments Loop Block: Tracking issue #34994

Closed
13 tasks done
SantosGuillamot opened this issue Sep 21, 2021 · 66 comments
Closed
13 tasks done

Comments Loop Block: Tracking issue #34994

SantosGuillamot opened this issue Sep 21, 2021 · 66 comments
Assignees
Labels
[Block] Comments Affects the Comments Block - formerly known as Comments Query Loop [Feature] Blocks Overall functionality of blocks [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. New Block Suggestion for a new block [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues.

Comments

@SantosGuillamot
Copy link
Contributor

SantosGuillamot commented Sep 21, 2021

Description

The idea of this issue is to end up with a Comments Loop Block (name may change) that loops over the comments of a given post. This would be similar to the Query Loop used for posts. Users should be able to define and change the layout of the post comments directly from the Gutenberg editor, and both the editor and the frontend should match.

This is how the different comment blocks should look like:

Comments Query Loop

https://excalidraw.com/#json=6537760064143360,EgfZIljXt2SI2DTSFebHQQ

Existing Solution / Workaround

Right now, there are two similar blocks:

  • Post Comments block: This block currently renders the comments.php file of the PHP theme in the frontend and a loop of paragraphs in the editor. This way, the editor and the frontend are different and users aren't able to change the layout unless they change the PHP code.
  • Post Comment block: This block allows users to insert a specific comment by ID in their posts/pages and edit the layout including other blocks like Post Comment Author, Post Comment Content or Post Comment Date. However, it isn't possible to loop through all the comments of a specific post.

Tasks

These are the steps we should follow to finish the tracking issue:

Please note that I'll be adding the relevant issues once they are created.

Out of scope

There are some functionalities that could be nice to work on but that won't be covered in this tracking issue, although they may be addressed later in the future:

  • Comments Form: There is an already working Post Comments Form block, that allows users to leave a reply. Just adding the Reply Link block pointing to this form should be enough for now.
  • Comments Metadata: It could be nice to create a block to show any meta data added for the comments in the future.
  • Comments Status: It could also make sense to create a block to show the status of the comments (approved, pending...), specially when they are submitted.

Questions

  • Once this is done, should we deprecate the existing Post Comments block?
  • As explained previously, there are already working blocks for the Post Comment Author, Post Comment Date, Post Comment Content that are used for the Post Comment Block. I guess we could work on the existing ones right? Or would it be better to create new ones for this purpose?
  • It seems to me that, while working on the Block Elements, we should keep in mind that they should also work fine for the Post Comment Block right?
  • Once the Comments Loop block is finished, should we even deprecate the Post Comment Block and create a setting in the loop to select just an id?
  • I feel that, as already pointed in other issues, it would be better to have two different blocks for the Comment Author and the Comment Author Avatar. However, right now in the posts, the Avatar is just a setting in the Post Author block. Is there any reason to not create two different blocks?
  • Should we create in the future different block elements for the Comments Form block as well to be able to edit its layout?
  • Anything else you think we should address and not covered here?

Of course, any feedback apart from these questions is more than welcome 🙂

Previous work

There has been already some work done related to the Comments Block that was tracked in this issue → #24101

@mcsf
Copy link
Contributor

mcsf commented Sep 21, 2021

👋 Excited to see these great issues popping up!

We can start with a soft deprecation, i.e. supporting the old block but deprioritising it (e.g. hiding it from the inserter). In other words, we don't need formal deprecation notices yet. The fact that it's a dynamic block means that, later on, we have some flexibility to determine how existing Post Comments blocks should be rendered.

If the old blocks' usesContext: [ 'commentId' ] directive is compatible with the new blocks' intended implementation, I'd say it's worth reusing.

  • It seems to me that, while working on the Block Elements, we should keep in mind that they should also work fine for the Post Comment Block right?

Yes, as I mentioned above — unless supporting both becomes an unreasonable burden!

  • Once the Comments Loop block is finished, should we even deprecate the Post Comment Block and create a setting in the loop to select just an id?

IMO, it depends on what the benefits are, or what the use cases could be for this. It feels lower priority to me.

  • I feel that, as already pointed in other issues, it would be better to have two different blocks for the Comment Author and the Comment Author Avatar. However, right now in the posts, the Avatar is just a setting in the Post Author block. Is there any reason to not create two different blocks?

It's probably the trade-off between the flexibility of more granular blocks (Author + Avatar) vs. the user-friendliness and polish of dedicated multi-functional blocks (Author with integrated Avatar). With the ongoing improvements in layouts and alignments, the balance may soon tip towards granular blocks. I suggest experimenting with both and seeing what feels right. :)

@jameskoster
Copy link
Contributor

@SantosGuillamot One design update we might consider: the new Dimensions and Layout panels will be useful on blocks like Avatar, but potentially all of them.

With the ongoing improvements in layouts and alignments, the balance may soon tip towards granular blocks.

Probably worth getting a theme dev's input here (cc @kjellr), but I suspect that the flexibility afforded by separate blocks for author name and author avatar will be more useful in general.

@kjellr
Copy link
Contributor

kjellr commented Sep 22, 2021

Probably worth getting a theme dev's input here (cc @kjellr), but I suspect that the flexibility afforded by separate blocks for author name and author avatar will be more useful in general.

Yeah, the current iteration of the Post Author block is pretty inflexible and limiting for theme authors. There was a lot of talk about its issues (and the idea of splitting it into sub-blocks) in #24952. This is one of the reasons the block was left out of 5.8.

@carolinan
Copy link
Contributor

carolinan commented Sep 22, 2021

Author with integrated Avatar -sounds like a good default block pattern or a block with inner blocks.
The post author block is really just waiting for a decision to be made, so that work can start.

@luisherranz luisherranz added the [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues. label Sep 22, 2021
@luisherranz
Copy link
Member

@SantosGuillamot these three blocks don't have design, so I'm going to label them with Needs Design:

@SantosGuillamot
Copy link
Contributor Author

@SantosGuillamot One design update we might consider: the new Dimensions and Layout panels will be useful on blocks like Avatar, but potentially all of them.

I agree it makes sense for most of these blocks 🙂 . I can see that the Dimension panel was integrated in this Pull Request and there is another Pull Request to create a unified Layout panel. Do you know which is the best place to understand both panels better and how to integrate them in the blocks?

@jameskoster
Copy link
Contributor

I believe the Group block has already implemented the Dimensions panel, so that might be a good place to look?

To clarify, it probably only makes sense for the Layout panel to be added to container blocks in this context, IE the comment query loop and the comment template. But obviously we can only do that once the feature has been added – my apologies, I thought it already had been.

@carolinan
Copy link
Contributor

The post comment block is in the theme category, but the inner blocks are in the design category, is that correct or should they all be in the theme category?

@ockham
Copy link
Contributor

ockham commented Apr 20, 2022

Adding a list of PRs that we’ll also want to include in WP 6.0 (which aren’t in my post because I missed them, or they were filed later as follow-up bugfixes):

(I’ll update this comment with more PRs if needed.)

@ockham
Copy link
Contributor

ockham commented Apr 21, 2022

For example, can we have a default template (inner blocks setup) that mimics the layout of the current post comments block? What if it just works as a fallback when no children are passed and when loaded in the editor you can pick a different pattern and that would transform into child blocks?

For reference, this is what inserting a (posts) Query Loop block looks like:

With pattern Start blank
post-query-loop post-query-loop-pattern

I imagine you're suggesting to do something like this for the Comments Query Loop block.


To me, what we have now -- which is basically

a default template (inner blocks setup) that mimics the layout of the current post comments block

seems good as a baseline for WP 6.0. (Since users can customize those inner blocks by moving them around and/or styling them.) The pattern selection mechanism seems like a good feature to add later.

@ockham
Copy link
Contributor

ockham commented Apr 22, 2022

Update: Matías clarified via DM how he wants the Comments Query Loop logic to be merged into Post Comments, and how they should behave. I've filed #40521 with the details.

@ockham
Copy link
Contributor

ockham commented Apr 26, 2022

Update: Matías clarified via DM how he wants the Comments Query Loop logic to be merged into Post Comments, and how they should behave. I've filed #40521 with the details.

Per discussion on the issue, and due to the problems we've encountered, we've decided not to attempt having the Comments Query Loop merged into the Post Comments block for WP 6.0. We'll revisit after it's released.

@ockham
Copy link
Contributor

ockham commented Apr 26, 2022

As a corollary to the above, and per Slack discussion with @priethor:

The Post Comments block has been deprecated and removed from the inserter for a while, so people won’t be able to newly insert it. UX-wise, this is not so different from what the block merge would’ve achieved (where we would’ve retained the childless/monolithic version of the block for backwards compat, but upon newly inserting it, users would’ve gotten the modular version).

We still would like to rename Comments Query Loop (to Comments) though, as per our original plan.

@gziolo
Copy link
Member

gziolo commented Apr 26, 2022

We caught two issues during backporting the latest changes to WordPress core:

  1. A simple fix to sort out whitespaces to follow WP coding standards:
    /**
    * Registers the `core/comments-title` block on the server.
    */
    function register_block_core_comments_title() {
    register_block_type_from_metadata(
    __DIR__ . '/comments-title',
    array(
    'render_callback' => 'render_block_core_comments_title',
    )
    );
    }
    add_action( 'init', 'register_block_core_comments_title' );
  2. The hook that is added globally in the Post Comments Form will also impact the Post Comments block:
    /**
    * Use the button block classes for the form-submit button.
    *
    * @param array $fields The default comment form arguments.
    *
    * @return array Returns the modified fields.
    */
    function post_comments_form_block_form_defaults( $fields ) {
    if ( wp_is_block_theme() ) {
    $fields['submit_button'] = '<input name="%1$s" type="submit" id="%2$s" class="%3$s wp-block-button__link" value="%4$s" />';
    $fields['submit_field'] = '<p class="form-submit wp-block-button">%1$s %2$s</p>';
    }
    return $fields;
    }
    add_filter( 'comment_form_defaults', 'post_comments_form_block_form_defaults' );

    It should get applied in the render callback and removed after the output content is ready.

@gziolo
Copy link
Member

gziolo commented Apr 26, 2022

Another small thing caught while testing in WordPress core.

Post Comments block on the front end:

Screenshot 2022-04-26 at 15 09 54

Comments Query Loop on the front end:

Screenshot 2022-04-26 at 15 09 48

@cbravobernal
Copy link
Contributor

cbravobernal commented Apr 26, 2022

Regarding the title size, is because we used h2 instead of h3. As @justintadlock recommended. We can go back and set the h3 as a default.
#40419 (comment)

Regarding the date, we have a selector in the block that allows you to choose the date format. We can set with hours and minutes by default. Adding the 'at' would be trickier though. Also, the format is usually defined by the theme. In our block, we get the default format from the date settings.

Regarding the Author, we can address an issue to include a plaintext like in the Comments Title, so the user can write "says:", putting it as a default.

@gziolo
Copy link
Member

gziolo commented Apr 26, 2022

Please note also "3 Responses" vs "3 responses".

@ockham
Copy link
Contributor

ockham commented Apr 26, 2022

Thanks @gziolo! Working on fixing those here: #40628.

@peterwilsoncc
Copy link
Contributor

As Beta 3 was the string freeze (no new strings), could the comment's title block use the same ones as in the template. This will avoid breaking the freeze & reduce the number of strings requiring translations.

<?php
if ( 1 == get_comments_number() ) {
	printf(
		/* translators: %s: Post title. */
		__( 'One response to %s' ),
		'&#8220;' . get_the_title() . '&#8221;'
	);
} else {
	printf(
		/* translators: 1: Number of comments, 2: Post title. */
		_n( '%1$s response to %2$s', '%1$s responses to %2$s', get_comments_number() ),
		number_format_i18n( get_comments_number() ),
		'&#8220;' . get_the_title() . '&#8221;'
	);
}
?>

-- source

I think the logic will need to be more complex to allow for users modifying the title in the block.

@cbravobernal
Copy link
Contributor

Please note also "3 Responses" vs "3 responses".

Now with the block, we are allowing some more options like:

  • Show/Hide the comments count.
  • Show/Hide the post title.
  • Choose the text wanted for the string between the comments and the title.

Covering all those different options with different strings for uppercase/lowercase would be hard to maintain. I put it as an uppercase to cover when the Comments Count is hidden, but we can do it the other way. In both cases, the user will need to update the string if they change the default layout, according to their language.

Also, as @ockham mentioned in the title PR, there is the possibility of having more than two different ways of plurals, so we should find a way to translate _n functionality into blocks.

@ockham
Copy link
Contributor

ockham commented Apr 29, 2022

Please note also "3 Responses" vs "3 responses".

This is being addressed by @c4rl0sbr4v0's #40728.

@SantosGuillamot
Copy link
Contributor Author

SantosGuillamot commented May 4, 2022

This is the progress of the latest Pull Requests. The changes made in the PRs that are backported will be included in 6.0 version. If I am not mistaken, the other ones will have to wait for future versions (6.0.1 is expected in June/July).

Feel free to correct me or add anything missing 🙂

Merged and backported in beta/RC

Merged but NOT backported

Not finished yet

@ockham
Copy link
Contributor

ockham commented May 4, 2022

@SantosGuillamot I updated your comment to add two links to issues that I created (plus one for a PR).

@annezazu
Copy link
Contributor

Anything else you think we should address and not covered here?

In case it's helpful, some feedback came in through the FSE Outreach Program for the fourteenth call for testing that explored these new blocks. Specifically, the following ideas were thrown out:

  • A block that shows when the last comment happened
  • A block that shows how many people have commented total (across the whole site rather than just the post)

Happy to open individual issues for these if it's desired but wanted to lightly put on the radar for now as possible ideas to consider.

@cbravobernal
Copy link
Contributor

Thanks for the feedback @annezazu 🎉 😄

A block that shows how many people have commented total (across the whole site rather than just the post)

There is already a Post Comments Count block, so we may just add an option to display the whole site comments count. It could be a Good First Issue if the community wants this feature. But I don't know if this would be a good Core block feature.

A block that shows when the last comment happened

Would be this one just a date? I cannot find any use case here, but, with Latest Comments block, you can achieve something similar.

Screenshot 2022-05-25 at 11 53 19

@annezazu
Copy link
Contributor

Would be this one just a date? I cannot find any use case here, but, with Latest Comments block, you can achieve something similar.

Yes! Just the date as a way to show perhaps more active commenting areas compared to others. Neat to see you can do this already with latest comments block 💥

@ockham
Copy link
Contributor

ockham commented May 27, 2022

Hey @annezazu , thanks a lot for the feedback! 😄

Specifically, the following ideas were thrown out:

  • A block that shows when the last comment happened
  • A block that shows how many people have commented total (across the whole site rather than just the post)

While those make sense, I’m afraid the team won’t prioritize these blocks in the near future. TBH, since their use cases seem a bit less common than our garden variety Comments blocks, I think it’s arguable that they could be third-party plugin material — especially the latter; and for the former, there’s the Latest Comments block that @c4rl0sbr4v0 pointed out that could serve as a substitute. If you think it would still make sense to have issues for those in the GB repo (potentially to discuss that these blocks should be in GB/Core), please go ahead and file them 😊🙏


FWIW, we’re going to close this tracking issue soon (since WP 6.0 has been released), and will file a new one with a list of some smaller follow-up fixes for WP 6.0.1 (but we won’t be introducing any new blocks there). Beyond that, the @WordPress/frontend-dx team will be focusing on Block Frontend Hydration, and on modularizing the Post Comments Form block.

@ZebulanStanphill
Copy link
Member

  • A block that shows when the last comment happened

  • A block that shows how many people have commented total (across the whole site rather than just the post)

These sound like ideas more suited for the dynamic tokens concept discussed in #39831, rather than blocks.

@SantosGuillamot
Copy link
Contributor Author

Closing this in favor of #41451

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Comments Affects the Comments Block - formerly known as Comments Query Loop [Feature] Blocks Overall functionality of blocks [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. New Block Suggestion for a new block [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues.
Projects
None yet
Development

No branches or pull requests