Ensure filter blocks are not reloaded every time they are selected in the editor #8002
Conversation
nefeline
added
block: filter by price
woocommercebot
requested review from
a team and
danielwrobert
and removed request for
a team
|
The release ZIP for this PR is accessible via: |
TypeScript Errors ReportFiles with errors: 436
assets/js/base/context/hooks/collections/use-collection.ts
assets/js/blocks/rating-filter/test/block.tsx |
|
Size Change: -543 B (0%) Total Size: 1.01 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Thank you so much for the PR @nefeline! Great work, I can confirm that the filter block is no longer flashing when clicking on them.
On the
useCollectionDatahook, we are debouncingshouldSelectto delay data collection requests on the initial mount for 200ms. By reading the original PR where it was introduced, #1233, this was added to prevent multiple collection data requests, which makes total sense.
In testing this, I realize that there are still multiple collections data requests made on the page that contain multiple PR blocks. Should we revisit 1233?
Screenshot
We're passing isEditor and isSelected to our useCollection* hooks, but as we're making the debouncedShouldSelect not 'debounced' in the editor, I think only passing isEditor is enough, as shouldSelect is now always true in the editor, so the collection hook won't return null anymore. What do you think?
Thanks 🙌 !
Yes, I agree there's room for improvements regarding 1233; from my understanding, the debounce was added as a workaround to reduce the number of API requests, due to the absence of a shared context, which makes sense: if we can structure the codebase in a way we know for a fact that x filter components exist on a given post/page and make only one request based on this shared information, that would be the ideal. If we stick with the
|
|
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
assets/js/blocks/rating-filter/test/block.tsx
|
There was a problem hiding this comment.
Thanks so much for the update @nefeline! This is testing great on my end. I left an optional comment but I'm pre-approving this. 🚀
github-actions
bot
added
status: ready to merge
and removed
status: needs review
labels
Fixes #7676
This update ensures the filter components are not reloaded every time they are selected in the editor.
Why was this problem happening in the first place?
On the
useCollectionDatahook, we are debouncingshouldSelectto delay data collection requests on the initial mount for 200ms. By reading the original PR where it was introduced, #1233, this was added to prevent multiple collection data requests, which makes total sense.The problem here is when
shouldSelectis false, theuseCollectionhook returns null. In the editor, whenever we click on a component, it is re-rendered. Since the initial request is debounced (withshouldSelectbeingfalseby default), we always end up fetching again from the store.To circumvent this, the
isEditorandisSelectedprops are now passed to theuseCollectionDataanduseCollectionhooks as params. If we are in the editor,shouldSelectis set totrueby default. This way, we will always fetch the latest data when the editor page is initially loaded.When a given block is selected in the editor, we now return the same data from the initial request (from
currentResults) instead of fetching from the store again on re-render. This change only affects the editor, where the components are disabled by default.Screenshots
User Facing Testing
WooCommerce Visibility
Performance Impact
We have some improvements in performance here, since, now, in the editor, we are only fetching data from the store on the initial render, relying on the
currentResultswhenever the component is selected.Changelog