-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Create Query Block #20106
Create Query Block #20106
Conversation
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 are still outstanding issues I raised on Automattic/newspack-blocks#176 and on other conversations on Slack regarding the use of a custom store for deduplicated blocks.
Based on our conversations, here are the changes that I'm going to be working on:
Once those are ready, I'll ping you for another review |
Sounds good!
|
Couple of follow-up questions on this:
|
I think it would work better if it's the actual block you insert and the Query blocks are like Navigation Link Item blocks in the Navigation block.
Yes
Breaking the block list is not compatible with most of the editor APIs unless you completely break the editing flow like reusable blocks do. We've been using |
0b95943
to
27e5afc
Compare
I'm having some trouble implementing
I'm going to keep looking at using I also attempted to rebase this off of the branch for #19572 so I could try to use the Block Context stuff but ran into some PHP fatals. I'll keep an eye on that branch and try again later. |
I was able to get the
I don't think that Should we wait for #19572 so that we can do that, or go ahead with the current working |
c83f66e
to
2c02724
Compare
I added a static block (paragraph) between the title and the date, it showed in the editor properly but not in the frontend. |
"className": { | ||
"type": "string" | ||
}, | ||
"criteria": { |
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.
After using this block for a few minutes, I found that there's a lot of work to be done to make sure the block works properly even without criteria/filtering...
I was wondering what do you think if we split this PR into two, one block without filtering where we'd only focus on how the template gets edited/updated and rendered. and the filtering can be easily added on a follow-up PR when the interactions are sorted-out?
I noticed that it's very hard to select the "Query" block from the footer, I believe that's because the block is "unselected" when one of its inner blocks are selected? |
|
||
this.debouncedCreateBlockTree = debounce( | ||
this.createBlockTree.bind( this ), | ||
1000 |
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.
While testing I noticed that my changes take a lot of time to propagate to the other blocks, Is this one-second delay necessary?
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.
It seems to be, because changes to block content / attrs will propagate and interfere with the usability.
The easiest way to see the problem is to remove the debounce, add a paragraph block inside the Query, and start typing. When you are typing, the block tree will keep getting updated and the updates will cause your typing to lag, and in some cases not register at all.
$args = core_query_attributes_to_critera( $attributes['criteria'] ); | ||
$args['posts_per_page'] = $args['posts_per_page'] + count( Blocks_Query::$displayedPostIds ); | ||
|
||
$query = new WP_Query( $args ); |
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.
What happens if we just want to use the default query object? We should probably use the default query if no arguments are present?
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.
Are you changing the default WP_Query or just looking to make sure that this does what you'd expect?
I think that getting this to align with WP_Query is a good goal, but for now there are a lot of options that don't have a UI yet. This is something I think we'll have to iterate on.
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 just envisioning using this in a block-based theme. I'd add the block in my theme but not add any custom query args so that it will use the global WP_Query object. I don't want to trigger the server to do a second query if it isn't needed.
I could be misunderstanding, but this seems like the foundational use case for a block based theme?
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 could be misunderstanding, but this seems like the foundational use case for a block based theme?
If it isn't, I don't know what is. I just tried out the new block-based theming system and the loop appears to be the only thing missing. The template tag blocks need to be placed within some kind of loop container, they can't just be directly placed in the outer part of the template. Especially for posts page, archives and search results templates where the default query returns multiple posts, the template needs to have some kind of loop container and this appears to be it.
@jasmussen if you have some time can you try out this block and see what we can do to make it better? |
Thanks for the ping George, very excited about this block and happy to see it land soon. I think there are two aspects to it: a) we need for it to reach a baseline of features and quality so we can merge it, and b) we should polish and improve beyond that baseline set of features. A is what we need to address for this PR to be merged, and B we can ticket. It feels prudent to decide what goes in A or B, and to that end I have some questions about the block, if you'll be patient with me! To that end, this is what I see: Very first glance: this is pretty cool. Wow. The Loop in a block. There are some editable fields: What am I editing here? The last pre-inserted block looks like a "Post placeholder", but it has a LEGO block icon (undefined icon). I'm also unsure exactly what I'm editing. Should I even be editing that block? Can I customize the "Read More" text here? This block: The 2nd line feels like an "insert your own block here". Is that it? And if so, do we really need it? Could I not just insert my own block after the Query block itself? It doesn't have a block toolbar, so it feels like a caption field: Honestly the above two question are the extent of my uncertainty. The block as a whole feels cool. It's as complex as The Loop, but that's fine — it IS a complex block. Therefor the options in the sidebar are also cool, and I like how it's split by the toggle right at the top. In the category of polish and refinements, we should get a new icon for the query block itself, one that is cognizant of efforts in #18667. But I can help with that in a followup PR. The other thing is to work with some better placeholders. These are all so barebones that they aren't even representative of what the post should look like. But this is a larger effort and not the responsibility of this block, and is tracked in #19256. I hope this was helpful, and don't hesitate to ping me again if there's anything I can be helpful with — PRs, feedback, CSS, anything at all. I noticed a few PHP errors, but not sure that's my own doing or not. Seeing this:
|
?> | ||
</div> | ||
<?php endwhile; ?> | ||
<?php endif; ?> |
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.
What happens if there are no results found?
The Query Block allows users to create a search on the site and choose blocks to display content from the matching posts. This allows users to highlight specific content or areas of their sites.
- DRY block attributes and move to `block.json`
b4f4c2c
to
699885e
Compare
Just a quick note: This might be planned already but would it be possible for the output of this block use standard classnames like |
Will the pagination be handled with this block as well? Asking for a friend 😂 |
@georgeh and @epiqueras: is there anything left here after the merge of #22199, or can we close the PR? |
Nope, let's close it |
Description
The Query Block allows users to create a search on the site and choose blocks to display content from the matching posts. This allows users to highlight specific content or areas of their sites.
How has this been tested?
Tested locally, behind the
__experimentalEnableFullSiteEditing
flag since it requires FSE blocks to display queried content.Screenshots
Types of changes
Checklist: