Comments blocks backports and fixes: Tracking issue

[SantosGuillamot]

Supersedes #34994

The goal of this issue is to list and keep track of some issues related to the Comments blocks that are planned to be included in 6.0.1. For full context about the Comments blocks, please take a look at the original tracking issue.

There is one PR that has already been merged but wasn't included in 6.0 so it needs a backport:

• Backport Refactor: Reuse code of the editor placeholder across Post Comments and Post Comments Form.

And some other fixes that makes sense to work on:

• Comments translation issue.
• Warning i18n issue.
• Post Comments Form: Add button that enables commenting to warning
• Comment Author and Date blocks: aligned editor markup with the frontend
• Comments Block: fixed issue with custom font sizes and links color

Update: The following issues have been postponed to WP 6.1 (see #41451 (comment)):

• Merge Comments Query Loop block into Post Comments block (Merge Comments and Post Comments blocks #41807).
• Change the name and ID to Comments and core/comments.
  • Name (Comments Query Loop block: Rename to Comments (but keep ID) #40763)
  • ID (Comments block: Change ID to core/comments #40506)

[ockham]

We might wanna add this issue which already has an in-progress PR.

[SantosGuillamot]

Thanks for sharing 🙂 Although it is related to the comments, the Avatar is a global block. Should we include it here anyway or it is better to handle it independently? Any thoughts? I don't have a strong opinion here.

[ockham]

Thanks for sharing 🙂 Although it is related to the comments, the Avatar is a global block. Should we include it here anyway or it is better to handle it independently? Any thoughts? I don't have a strong opinion here.

Since it's kind of prominently used in the Comments blocks (and impacted by that bug), I think it makes sense if we make sure a fix finds its way into 6.0.1 🙂

[ockham]

Here's another PR we should include: #41631

It's been added to the 6.0.1 project board, and I've added the "Backport to WP Minor Release" label. We might not need to edit our list every time to include issues/PRs like that (since they're covered by that project board and label), and instead use the list as our team's TODO for 6.0.1 🙂

[ockham]

As mentioned at #41807 (comment), it's probably a good idea to tackle the ID change (core/comments-query-loop) separately from the Comments/Post Comments block merge.

I've thus added #40506 to the WP 6.0.1 board.

[ockham]

Update: A few PRs that we had originally planned for inclusion in WP 6.0.1 aren’t quite ready yet and/or were otherwise deemed too risky, so we’ll be targetting WP 6.1 instead. They are:

• Merge Comments Query Loop block into Post Comments block (existing PR).
• Change the name and ID to Comments and core/comments.
  • Name (Comments Query Loop block: Rename to Comments (but keep ID) #40763)
  • ID (Comments block: Change ID to core/comments #40506)

I'll update the issue description accordingly.

Other than that, the translation issue has an in-progress PR that's slated to be included in WP 6.0.1. All other issues and/or PRs mentioned in this issue and the comments have been cherry-picked for inclusion in WP 6.0.1 🎉

[SantosGuillamot]

@c4rl0sbr4v0 I've seen that the Comments translation issue has already been merged. I assume we can consider it done right?

[cbravobernal]

Yes! Tracking issue updated 🎉

[ockham]

Another small fix here: #42131
I'll add it to the tracking issue.

[Poliuk]

@SantosGuillamot, now that 6.0.1 is out, I think it makes to close this issue and open a new one with the remaining tasks programmed for 6.1.

The following issues have been postponed to WP 6.1 (see #41451 (comment)):

• Merge Comments Query Loop block into Post Comments block (existing PR).
• Change the name and ID to Comments and core/comments.
  • Name (Comments Query Loop block: Rename to Comments (but keep ID) #40763)
  • ID (Comments block: Change ID to core/comments #40506)

Since you opened this one, I'll let you decide. What do you think?

[SantosGuillamot]

now that 6.0.1 is out, I think it makes to close this issue and open a new one with the remaining tasks programmed for 6.1.

Mmm I'm not 100% sure if opening a new issue is needed, as there are currently only a few issues left, that are kind of related. Moreover, I feel the opening post and the discussion aren't too long yet. Would it make sense to keep using this tracking issue for 6.1? Honestly, I don't have a strong opinion here and both are fine for me.

[Poliuk]

Would it make sense to keep using this tracking issue for 6.1?

Yeah, that should work as well, as long as we update this tracking issue, it should be fine 😊

[ockham]

Now that the Comments block ID change PR (#40506) is merged, the Comments/Post Comments blocks merge PR (#40521) is unblocked and has been rebased.

We now need to update a few things there (some basic stuff like s/comments-query-loop/comments/g) and give it a bit more testing.

[DAreRodz]

I've posted an update in #41807 (comment) regarding the block merge.

[DAreRodz]

Another update:

• I've added a button (#41807 comment) to switch from the legacy version to the "editable" version.
• I worked on some tweaks (#42406) after migrating the Comments block's e2e tests to Playwright.

I will work on adding the different test cases the block now has after the merge. 🙂

[ockham]

@DAreRodz and I have continued to work on the Block merge PR (#41807) for the past couple of days. We

• removed a now-obsolete block deprecation warning (on the server side)
• changed the way we remove the legacy block attribute by using setAttributes rather than replacing the monolithic legacy block with a freshly created modular one
• de-duplicated some of the warnings the user would get if comments were disabled -- now only shown in lieu of the Post Comments Form block (rather than the entire Comments block)
• made some tweaks to the e2e tests to be more semantic

Pretty much all feedback has been addressed. I've already given approval; there was one more piece of feedback that David was looking into. We might either address that as part of this PR or in a follow-up. In any case, we're planning to merge today 🤞

[DAreRodz]

I've just merged #41807 and left some notes for a follow-up PR. 😊

[annezazu]

Does this issue need to remain open? Seems the prior items listed for 6.1 have all been merged :) #41451 (comment)

[SantosGuillamot]

I was waiting until 6.1 just in case some bugs arise while testing, but maybe it makes sense to close it and open new issues if needed. I'm closing it now, thanks for the heads up! 🙂