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

Post Comments block: Merge Comments Query Loop block #40521

Closed
ockham opened this issue Apr 21, 2022 · 11 comments
Closed

Post Comments block: Merge Comments Query Loop block #40521

ockham opened this issue Apr 21, 2022 · 11 comments
Labels
[Block] Comments (legacy) The legacy mode of the Comments block (formerly known as Post Comments) [Block] Comments Affects the Comments Block - formerly known as Comments Query Loop

Comments

@ockham
Copy link
Contributor

ockham commented Apr 21, 2022

As per discussion with @mtias here and via DM, we should merge the Comments Query Loop block into the existing Post Comments block, with the following behavior:

  • If it doesn’t have any child blocks: Use the current Post Comments behavior, i.e. render the “monolithic” WP comment_form(). In the editor, we’ll display the placeholder.
  • If it has children: Use the Comments Query Loop behavior (i.e. render those child blocks).

If it’s newly inserted: insert child blocks as specified in its default template (as Comments Query Loop does already).

This means that the block will provide backward-compat for Post Comments (where it’s been used for existing themes); OTOH, it will provide the new functionality by default when newly inserted.

Quoting Matías:

The important things:

  • Replacing the block not creating a new one so we keep the namespace.
  • That existing uses of the PHP comments don’t break.
  • That inserting the block for the first time in a template uses the new set of blocks.

Note that there will be some edge cases that can lead to slightly awkward user experience, e.g. trying to edit a previously inserted empty Post Comments block (will show a placeholder in the editor). Matías said not to worry about these for 6.0.

The rest can be improved through iterations [after WP 6.0]
Including a “convert to blocks” and the addition of some nicely designed patterns

cc/ @WordPress/frontend-dx @priethor @gziolo

@ockham ockham added [Block] Comments Affects the Comments Block - formerly known as Comments Query Loop [Block] Comments (legacy) The legacy mode of the Comments block (formerly known as Post Comments) labels Apr 21, 2022
@ockham
Copy link
Contributor Author

ockham commented Apr 21, 2022

Living TODO list:

  • Un-deprecate Post Comments block
  • Merge Comments Query Loop code into Post Comments block
    • Solve the static-dynamic rendering of Post Comments block
    • Move the edit.js code from Comments Query Loop block to Post Comments blocks
    • Change parent attribute of comment blocks from Comments Query Loop to Post Comments
    • Update unit and e2e tests
  • Update the warning that was introduced by Add placeholder to Post Comments block #40484. It should explain that this is the empty block fallback, and that it's possible to add children to the block in order to build a customized Comments block.
  • Add a legacy block update to convert Comments Query Loop into Post Comments (see).

@peterwilsoncc
Copy link
Contributor

@mtias @ockham @gziolo, I presume this is to be targeted for 6.1 rather than 6.0, is that correct?

@ockham
Copy link
Contributor Author

ockham commented Apr 25, 2022

@mtias @ockham @gziolo, I presume this is to be targeted for 6.1 rather than 6.0, is that correct?

@peterwilsoncc We'd actually like to include it in 6.0 😊 We're hoping to get the PR ready for inclusion in Beta 3 (or RC1, at latest) so there'll be still a couple of weeks to test and polish them.

The major driver here is avoiding polluting the block namespace, as per Matías' list that I quoted in the issue desc, which will in turn help avoiding confusion to users about too many competing Comments related blocks (plus needing to deprecate and migrate them later).

The change itself should be fairly self-contained: We're basically adding logic to the Post Comments' index.php that will render its child blocks if there are any, and otherwise fall back to its current behavior. That, plus a couple of editor-side changes to reconcile the UIs of the Comments Query Loop with the Post Comments block.

@peterwilsoncc
Copy link
Contributor

WordPress has been in feature freeze since Beta 1 and I think this is a major enhancement that should not be put in. I can't see it been close to ready in time for the final beta on Tuesday.

I noticed on Friday the new comments block is missing a few features that would prevent 1:1 transformation, for example the pre-moderation preview is not shown to new commenters.

See #40601

@ockham
Copy link
Contributor Author

ockham commented Apr 26, 2022

WordPress has been in feature freeze since Beta 1 and I think this is a major enhancement that should not be put in. I can't see it been close to ready in time for the final beta on Tuesday.

Fair enough 😅 I was probably a bit over-optimistic, but since #40522 has hit a few bumps, we'll punt that and won't try to get it into 6.0.

I noticed on Friday the new comments block is missing a few features that would prevent 1:1 transformation, for example the pre-moderation preview is not shown to new commenters.

See #40601

Thanks for flagging, we'll look into this!

@mtias
Copy link
Member

mtias commented Apr 26, 2022

I'm fine with punting since this is running into some issues, but I'm still fairly concerned about the namespace issue — offering multiple blocks that do the same with no strategy for consolidating is not very reasonable. What are the options to minimize the impact there?

@peterwilsoncc
Copy link
Contributor

I think hiding one or the other from the inserter makes sense, I'll leave it to y'all to decide which.

I suggest:

  • provide a filter to reverse the hiding for developers wishing to use the legacy block (I guess this exists already)
  • document how to reverse the hiding in a dev-note

This avoids migrating too early and alienating some of the biggest external advocates for block themes.

Once there is feature parity the migration can occur. I think the approach Dave detailed in #33440 would need to be used.

Neither approach it optimal but I think this is the least sub-optimal as it avoids rushing a migration after feature freeze and removing features in the process.

@ockham
Copy link
Contributor Author

ockham commented Apr 27, 2022

Thanks @peterwilsoncc, that sounds like a good strategy!

provide a filter to reverse the hiding for developers wishing to use the legacy block (I guess this exists already)

Yeah, one of the registration-level block filters should do the trick. We'll write up a note!

@SantosGuillamot
Copy link
Contributor

Yeah, one of the registration-level block filters should do the trick. We'll write up a note!

I assume the block_type_metadata filter should do it right? I've quickly test this code and it seems to work:

function enable_post_comments_legacy_block( $metadata ) {
    if ( 'core/post-comments' === $metadata['name'] ) {
        $metadata['title'] = 'Post Comments';
	$metadata['supports']['inserter'] = true;
    }
    return $metadata;
}
add_filter( 'block_type_metadata', 'enable_post_comments_legacy_block' );

What do you think? Does it make sense?

@ockham
Copy link
Contributor Author

ockham commented Apr 28, 2022

@SantosGuillamot kindly posted the dev note here: #39654 (comment)

@ockham
Copy link
Contributor Author

ockham commented Jul 26, 2022

Fixed by #41807 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Comments (legacy) The legacy mode of the Comments block (formerly known as Post Comments) [Block] Comments Affects the Comments Block - formerly known as Comments Query Loop
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants