This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Implement Hand-Picked Products block #7925
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Implemented as another variation of the Product Query block.
|
The release ZIP for this PR is accessible via: |
Script Dependencies ReportThe
This comment was automatically generated by the |
TypeScript Errors ReportFiles with errors: 432
assets/js/blocks/product-query/variations/handpicked-products.tsx
|
|
Size Change: +295 B (0%) Total Size: 1.11 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. |
|
The release ZIP for this PR is accessible via: Script Dependencies ReportThe
This comment was automatically generated by the TypeScript Errors Report
assets/js/blocks/product-query/inspector-controls/product-selector.tsx
|
Previously, we thought of the hand-picked products block as a variation. Now we are allowing to hand-pick products on any “Products” block.
|
PR bumped to the next milestone (10.3.0) |
imanish003
approved these changes
May 8, 2023
There was a problem hiding this comment.
Hi @sunyatasattva,
I've tested the changes, and everything is working as expected. Excellent work, as always 🙌🏻
I've left a few minor comments, but they shouldn't block the PR, so I'm pre-approving it. 🚀
| import { ProductQueryBlock } from '../types'; | ||
| import { setQueryAttribute } from '../utils'; | ||
|
|
||
| export const ProductSelector = ( props: ProductQueryBlock ) => { |
There was a problem hiding this comment.
I am nitpicking here, but how do you feel about restructuring the attributes property from the props object to simplify property access:
Suggested change
| export const ProductSelector = ( props: ProductQueryBlock ) => { | |
| export const ProductSelector = ({ attributes }: ProductQueryBlock) => { |
There was a problem hiding this comment.
Thanks for the suggestion! Normally, I'd be up for it, but in this case you can see that we use props as a whole twice on L76 and L85.
Comment on lines
+24
to
+32
| const [ productsList, setProductsList ] = useState< ProductResponseItem[] >( | ||
| [] | ||
| ); | ||
|
|
||
| useEffect( () => { | ||
| getProducts( { selected: [] } ).then( ( results ) => { | ||
| setProductsList( results as ProductResponseItem[] ); | ||
| } ); | ||
| }, [] ); |
There was a problem hiding this comment.
I don't think it's needed here but just wanted to share my thoughts. Maybe we can create a custom hook to fetch and manage the productsList state to separate concerns and make the component more modular:
const useProductsList = (): ProductResponseItem[] => {
const [productsList, setProductsList] = useState<ProductResponseItem[]>([]);
useEffect(() => {
getProducts({ selected: [] }).then((results) => {
setProductsList(results as ProductResponseItem[]);
});
}, []);
return productsList;
};And use it in the component like this:
const productsList = useProductsList();
Comment on lines
55
to
89
| onChange={ ( values ) => { | ||
| const ids = values | ||
| .map( | ||
| ( nameOrId ) => | ||
| productsList.find( | ||
| ( product ) => | ||
| product.name === nameOrId || | ||
| product.id === Number( nameOrId ) | ||
| )?.id | ||
| ) | ||
| .filter( Boolean ) | ||
| .map( String ); | ||
|
|
||
| if ( ! ids.length && props.attributes.query.include ) { | ||
| const prunedQuery = objectOmit( | ||
| props.attributes.query, | ||
| 'include' | ||
| ); | ||
|
|
||
| setQueryAttribute( | ||
| { | ||
| ...props, | ||
| attributes: { | ||
| ...props.attributes, | ||
| query: prunedQuery, | ||
| }, | ||
| }, | ||
| {} | ||
| ); | ||
| } else { | ||
| setQueryAttribute( props, { | ||
| include: ids, | ||
| } ); | ||
| } | ||
| } } |
There was a problem hiding this comment.
Maybe we can extract the logic of the onChange callback into a separate function to make the component more readable and maintainable?
const handleTokensChange = (values: string[]) => {
...
};
...
<FormTokenField
...
onChange={handleTokensChange}
...
/>There was a problem hiding this comment.
Yea good call.
Comment on lines
-15
to
-23
| /** | ||
| * Returns an object without a key. | ||
| */ | ||
| function objectOmit< T, K extends keyof T >( obj: T, key: K ) { | ||
| const { [ key ]: omit, ...rest } = obj; | ||
|
|
||
| return rest; | ||
| } | ||
|
|
There was a problem hiding this comment.
Great! Thanks for moving this to utils 🙌🏻
* Refactor `useProductsList` into own hook. * Refactor `onTokenChange` outside of the component.
kmanijak
pushed a commit
that referenced
this pull request
May 8, 2023
Implements the ProductSelector advanced filter within the “Products (Beta)” block. The filter allows the merchant to narrow down the exact products to which all subsequent filters will be applied, mirroring the functionality of the existing “Hand-picked Products” plus all the other functionalities available from the “Products (Beta)” block.
kmanijak
added a commit
that referenced
this pull request
May 8, 2023
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements the
ProductSelectoradvanced filter within the “Products (Beta)” block. The filter allows the merchant to narrow down the exact products to which all subsequent filters will be applied, mirroring the functionality of the existing “Hand-picked Products” plus all the other functionalities available from the “Products (Beta)” block.Fixes #7633
Accessibility
prefers-reduced-motionOther Checks
Screenshots
handpicked-products-screen-recording.mov
The video above shows a different block called “Hand-picked Products”. The current PR doesn't include this variation and includes the Product Selector within the normal block.
Testing
Automated Tests
User Facing Testing
Simplest path
Complex path
WooCommerce Visibility
Performance Impact
Changelog