-
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: Use a WordPress loop for the query block #30405
Conversation
Size Change: +977 B (0%) Total Size: 1.42 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.
Thanks for working on this Ari! In general I feel the direction is good but I would feel a lot more confident with testing the site editor as well. Do you think we should wait for woo 5.2
as is meant to include the FSE wsod fix ?
I tested locally by reverting this commit https://github.com/woocommerce/woocommerce-gutenberg-products-block/pull/3219/files in WooCommerce (that is the one referenced in the linked comment) and didn't see anything break... Reverting the commit does fix the WSOD, and there doesn't seem to be anything breaking in the site-editor, but I can't really test previewing a single-post there. I don't think it's even possible right now, unless I'm missing an obvious method? 🤔 Regardless of that however, the fix is for more than just WooCommerce, it should also fix any plugin using a custom template for a custom post-type (like events, products, jobs listings etc). |
Co-authored-by: Nik Tsekouras <[email protected]>
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 tested locally by reverting this commit https://github.com/woocommerce/woocommerce-gutenberg-products-block/pull/3219/files in WooCommerce
I tested as well a bit there, as actually the site-editor is still there - but is hiding with a display:none
:)
In general nothing broke to my testing either and the changes are reasonable. My main concerned was this and was answered.
Regardless of that however, the fix is for more than just WooCommerce,
I know, it was just easier for me to test with :)
I think we can move on with this and keep our eyes open for any regression 😄
Thanks for your work Ari!
Thanks for the fix! Just popping in here to let you know this fixed the rendering of single product pages for Woo. |
Perfect, thanks for the update @nerrad! |
I reverted the commit locally and it's the same without it :) |
Cool! I'm misunderstanding expectations here. It's all good 😄 |
Description
This PR changes the implementation of the query-loop block a bit.
Prior to this PR, we were running
get_posts()
to get the list of posts, and then ran these through aforeach
loop.With this PR, we are running a
WP_Query
and then the results of that go through a standard Loop (see WP loop docs for details).The benefit of this tweak is that now WordPress functions and hooks work as expected, without breaking backwards-compatibility.
WooCommerce compatibility
This is not limited to WooCommerce, it applies to any and all plugins hooking in WP to change the way their templates are displayed. Until they transition to an FSE structure they need to remain functional, and with this PR they work.
How has this been tested?
Tested with an FSE theme:
In all cases the frontend was showing what was expected.
Checklist:
*.native.js
files for terms that need renaming or removal).cc @ntsekouras