-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Mark FSE blocks as non experimental #35979
Conversation
packages/block-library/src/index.js
Outdated
spacer, | ||
table, | ||
// tableOfContents, | ||
tagCloud, | ||
templatePart, // TODO: Should this be site editor only? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about this one. We could potentially move it to packages/edit-site
if we feel that it's not useful outside of the site editor.
Size Change: +1.2 kB (0%) Total Size: 1.08 MB
ℹ️ View Unchanged
|
packages/block-library/src/index.js
Outdated
postComments, | ||
postCommentsCount, | ||
postCommentsForm, | ||
postCommentsLink, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not certain but I think some of these comments block (not all of them) were supposed to be replaced by a "Query" block like version. So not all of them might need to move to Core as stable. cc @c4rl0sbr4v0 @ntsekouras
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we lean to remove the PostComment
in favor of the Comments Loop
, but in general there's a lot of activity in changing these blocks and I'm not 100% sure if all of them are ready to be included in core.
Maybe @gziolo or @michalczaplinski could share their thoughts (or ping the right people) about their status.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We plan to completely deprecate the Post Comment (postComment
) block that is going to be replaced with the Comment Template block (WIP #35231) that is going to be composed of inner blocks like Comment Author (postcommentAuthor
), Comment Content (postCommentContent
) or Comment Date (postCommentDate
). There are other similar blocks that have just landed or will land this week. All those blocks aren't very useful without the Comment Query Loop block that might land this week with #35231 as well, but it was never tested in the plugin and it's far from the feature parity that the Query Loop block has.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that @kjellr shared a list of blocks used in the Twenty Twenty-Two theme in #35979 (comment). The only block listed here is the Post Comments block (postComments
). I think it would be reasonable to move only this single block out of the experimental phase. It's a simpler wrapper for comments_template()
PHP function call.
I don't know about Post Comments Count or Post Comments Link blocks, but they might be useful to use with Query Loop and Post Template blocks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should make stable postComments
as it relies on php template. In my mind this block will be removed when the Comments Loop
will mature more.
Also should we consider making the renames of the comment blocks (loose the post
prefix) before landing in core?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should make stable postComments as it relies on php template. In my mind this block will be removed when the Comments Loop will mature more.
How will we support Twenty Twenty-two then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the idea is that 2022 uses Comments Loop
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right! Twenty Twenty-two will be in core as well 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comments Loop block is still in progress: #35231. It's very basic and it doesn't support yet nested comments and pagination. It's hard to tell how much work is still necessary to bring feature parity with what comments_template()
does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...( enableFSEBlocks | ||
? [ | ||
templatePart, | ||
postAuthor, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know that the post author has been removed from the 5.8 release because it was deemed not ready (as not flexible enough for all use cases and user-* blocks were being considered), that said, it seems too late to replace at the moment for 5.9, so maybe it's fine to land this one as (as a simple opinionated one) and add the more flexible ones later. cc @mtias
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also hesitant that we need more granular ones. As long as you can show / hide avatar and show / hide name it seems you can do a lot with it. If you want to show avatar and name in different layouts you can add two author blocks with different settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting thoughts @mtias. We have more granular blocks for the Comment Template block:
- Comment Author
- Comment Author Avatar
Should we consolidate them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure :) There are some tradeoffs to both approaches.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, either way we need a way to display the post author in 5.9. 😄 I think the current one is workable if we want to just continue iterating on it.
Pinging @kjellr here to check what blocks are mandatory for 2022 and what's the story for comments and post author blocks. |
Here are all the blocks we're currently using for Twenty Twenty-Two. I don't expect that we'll be adding any more of the experimental blocks:
We are using the post author block, but all we actually display is the author name: For comments, we use only the Post Comments block. It's working well for us after this recent change: #35743 |
1b6b655
to
f9b13a2
Compare
Thanks everyone! I updated it so that we're only marking the blocks that are used by Twenty Twenty-two as stable:
|
f9b13a2
to
4a0fae2
Compare
I've updated this so that Post Comments remains experimental. Instead, we have Comments Query Loop as of #35231. |
4a0fae2
to
a964f1f
Compare
@noisysocks I have some questions about the plan for comments related blocks in core 5.9:
|
As noted by Greg above the Comments Query Loop block is still very basic and IMHO is not ready to be included in 5.9. It doesn't support nested comments, pagination and is not very well tested. I am not very familiar with how the "old" Post Comments block was working in the twenty-twentytwo theme but I think that we should stick with it and not move the Comments Query Loop out of experimental status yet. |
Hi @michalczaplinski, thanks for letting me know. Looks like I made the wrong call here. Happy to defer to you on the readiness of Comments Query Loop. I'll open a PR which marks Post Comments as stable (included in WP 5.9) and Comments Query Loop as experimental (plugin only). We can deprecate Post Comments in a future release (e.g. WP 6.0). |
Answering @jffng's questions for clarity:
The plan is now to include Post Comments in WP 5.9. Comments Query Loop will only be available in the plugin.
Twenty Twenty-two can continue to use Post Comments to achieve this. Post Comments Form will not be available in WP 5.9.
None of these blocks will now be included in WP 5.9. |
Thank you for clarifying @noisysocks ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a subtle change that we missed in this PR:
The blocks that were moved were not made available in "widgets", "navigation" screens, they were also not available in the "post editor" if the theme didn't have "block templates" support. (enableFSEBlocks
flag)
But my moving them without the check, we made these blocks available in all of these situations, maybe it's fine but I wanted to confirm that it's what we want. and if it's the case we should probably remove the enableFSEBlocks
flag entirely.
(To clarify we probably did the same mistake on WP 5.8 when we moved some of the similar blocks)
Description
Grab bag of plugin changes needed ahead of WP 5.9.
__experimentalRegisterExperimentalCoreBlocks
toregisterCoreBlocks
.Removes__experimentalRegisterExperimentalCoreBlocks
as there are no more experimental blocks.RemovesGUTENBERG_PHASE
checks no longer needed for WP 5.9.gutenberg_
. It did not work correctly when the function declaration is indented, causing duplicate function errors if you run the plugin against WP 5.9.How has this been tested?
Checklist:
*.native.js
files for terms that need renaming or removal).