-
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
[Block Library - Query Loop]: Pass extra query args in REST API call for accurate preview for extenders #44093
[Block Library - Query Loop]: Pass extra query args in REST API call for accurate preview for extenders #44093
Conversation
…for accurate preview for extenders
Size Change: +8 B (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
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.
@gigitux and I have reviewed this and tested the change. They work for our use-case, as can be seen in this PoC PR: woocommerce/woocommerce-blocks#6975
However, we are a bit unsure about this approach of allowing any arguments to the query instead of scoping them inside a custom object as was proposed in this PR. The concern here are the following:
- Imagine a situation in which a plugin is adding a custom block variation that uses a custom query argument which is not well namespaced. In the future, if we add a query argument in the main query with that same name, this could lead to naming conflicts.
- Developer experience is a bit clearer in our opinion with a nested object. So it's a bit easier to document.
- This approach will make the types a little bit more vague, and possibly problematic in the future.
Neither of these are big deal blockers, just some concerns that came up to us as we were playing with the code. Any thoughts?
We will approve this approach either way because it works for us and we were being very careful in namespacing our own implementation, but if there are any thoughts regarding those points, we're happy to hear 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.
Code looks good. I'm leaning on the reviews/testing of @gigitux and @sunyatasattva. I think this is good to ship for early adopters to test with while requiring appropriate additional work extending REST API endpoints via filter to obtain the desired functionality. This allows for finding out what patterns might emerge for more ergonomic APIs in future iterations.
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.
Code looks good. I'm leaning on the reviews/testing of @gigitux and @sunyatasattva. I think this is good to ship for early adopters to test with while requiring appropriate additional work extending REST API endpoints via filter to obtain the desired functionality. This allows for finding out what patterns might emerge for more ergonomic APIs in future iterations.
Not sure what happened with the duplicate approvals from me 🤷🏻 |
Thanks for reviewing and testing all!
I think that would still be the case with a specific query argument since it will be
In general GB support for |
Detail and explain how to make best use of the changes introduced in the following PRs: WordPress#43590, WordPress#43632 and WordPress#44093
* Refactor Product Query to use the latest Gutenberg APIs As we worked with Gutenberg folks in WordPress/gutenberg#43590, WordPress/gutenberg#43632 and WordPress/gutenberg#44093 we have created a standard API that could be used for our use-case. This PR refactors our WIP experimental work to use that standardized API.
* Add docs for extending the Query Loop block via variations Detail and explain how to make best use of the changes introduced in the following PRs: #43590, #43632 and #44093 * Fix typo Change enhables to enables Co-authored-by: Sören Wrede <soerenwrede@gmail.com> * Address code review feedback Specifically: * Added the complete code at the beginning * Change `isActive` from array to function * Reworded a few things * Added information about custom logic and other hints * Update docs/how-to-guides/block-tutorial/extending-the-query-loop-block.md * Change wording of opening paragraph for the example * Add section about innerBlocks and `scope` shortcomings * rename to `allowedControls` Co-authored-by: Sören Wrede <soerenwrede@gmail.com> Co-authored-by: Ryan Welcher <me@ryanwelcher.com> Co-authored-by: Nik Tsekouras <ntsekouras@outlook.com>
* Refactor Product Query to use the latest Gutenberg APIs As we worked with Gutenberg folks in WordPress/gutenberg#43590, WordPress/gutenberg#43632 and WordPress/gutenberg#44093 we have created a standard API that could be used for our use-case. This PR refactors our WIP experimental work to use that standardized API.
…7169) * Refactor Product Query to use the latest Gutenberg APIs As we worked with Gutenberg folks in WordPress/gutenberg#43590, WordPress/gutenberg#43632 and WordPress/gutenberg#44093 we have created a standard API that could be used for our use-case. This PR refactors our WIP experimental work to use that standardized API.
What?
Related to the php query_loop_block_query_vars filter added recently. This PR allows the Query Loop to be extended by third party devs, by passing extra query args in the REST API call.
This way we can have accurate previews in the editor.
Notes
Noting though that these args should either be supported by the REST API or be handled by custom REST filters like
rest_{$this->post_type}_query
.Testing Instructions
query
attribute of a Query Loop, should be passed in the REST API call.