Product Search Results template: fix the preview when the Inherit Query option is enabled #7965
Conversation
woocommercebot
requested review from
a team and
nefeline
and removed request for
a team
|
The release ZIP for this PR is accessible via: |
TypeScript Errors ReportFiles with errors: 432 🎉 🎉 This PR does not introduce new TS errors. |
|
Size Change: +109 B (0%) Total Size: 1.12 MB
ℹ️ View Unchanged
|
|
This PR has been marked as If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label. |
github-actions
bot
added
the
status: stale
gigitux
removed
the
status: stale
gigitux
changed the title
|
This PR has been marked as If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label. |
github-actions
bot
added
the
status: stale
gigitux
removed
the
status: stale
|
The release ZIP for this PR is accessible via: Script Dependencies ReportThere is no changed script dependency between this branch and trunk. This comment was automatically generated by the TypeScript Errors Report
🎉 🎉 This PR does not introduce new TS errors. |
gigitux
force-pushed
the
fix/7921-product-query-product-search-result-preview
branch
from
f89afa8
to
654b9bc
Compare
gigitux
added
block-type: product-query
gigitux
force-pushed
the
fix/7921-product-query-product-search-result-preview
branch
3 times, most recently
from
6b7f546
to
da8089b
Compare
gigitux
changed the title
|
This PR has been marked as If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label. |
github-actions
bot
added
the
status: stale
gigitux
removed
the
status: stale
You can already review it. This PR includes changes from #7956. I suggest reviewing #7956 and, after that, reviewing this PR. |
|
Great work! 👏
I see @danieldudzic is already assigned to review that one, so I'll focus here, but heads up, some tests here are failing (both on the CI and locally). User facing test results:
✅ Confirmed that when the sale status filter is enabled, only products on sale are displayed
✅ Confirmed all products are displayed when the Inherit query from template option is enabled.
✅ Confirmed only on-sale products are visible. e2e test results:The tests on this branch are failing. When running the product-query test suite locally, it also failed, so probably worth investigating the causes. |
|
Thanks for the review!
The E2E tests fail in trunk too. I will take a look at them next week! |
gigitux
force-pushed
the
fix/7921-product-query-product-search-result-preview
branch
from
07da73a
to
a8e49aa
Compare
gigitux
force-pushed
the
fix/7921-product-query-product-search-result-preview
branch
from
a8e49aa
to
fc5ccce
Compare
| <Control | ||
| { ...props } | ||
| queryObjectBeforeInheritEnabled={ | ||
| queryObjectBeforeInheritEnabled | ||
| } | ||
| defaultWooQueryParams={ | ||
| defaultWooQueryParams | ||
| } | ||
| key={ key } | ||
| /> |
There was a problem hiding this comment.
AFAIU queryObjectBeforeInheritEnabled & defaultWooQueryParams will be used in only wooInherit. If that's the case then I think instead of passing these as props, we should calculate it inside wooInherit function.
As we are using hooks (ex. usePrevious), I believe we will need to extract the wooInherit function as a component, something like this.
wooInherit: WooInheritToggleControl
IMO this would make the code easier to understand, but please feel free to ignore my suggestion if you disagree.
There was a problem hiding this comment.
Nice suggestion! I addressed it via 33fbac9.
I tested it, and it works. Please, could you recheck it?
There was a problem hiding this comment.
Hi @gigitux,
There is one issue. If I set keyword in filters as shown in screenshot below:
And then enable Inherit query from template then, keyword effect is still there, i.e., I still see products that are filtered by the Keyword.
Is it expected? AFAIK filters shouldn't have any effect when Inherit query from template is enabled. I might be wrong 🤷🏻♂️
That is a good point, but I can replicate this issue on the trunk. I think that it is a bug in the Query Loop block. Should we open another issue or write the issue on WordPress/gutenberg#29438? |
There was a problem hiding this comment.
@gigitux You are right. We can open a new issue on Gutenberg repo. Approving the PR 🚀
Although I think the issue you mentioned is a different one & not related to Inherit query from template toggle. You can also see my comment on the issue.
|
Thanks for the review!
Can you take care of this? |
gigitux
deleted the
fix/7921-product-query-product-search-result-preview
branch
Sure, I will take care of this 🤝 |
With this PR, when the user disabled the "Inherit Query option", the previous options are restored.
Fixes #7921
Testing
Automated Tests
User Facing Testing
WooCommerce Visibility