Skip to content
This repository has been archived by the owner on Feb 23, 2024. It is now read-only.

Product Collection - Add Inherit query from template control #9485

Merged
merged 74 commits into from Jun 5, 2023

Conversation

kmanijak
Copy link
Contributor

@kmanijak kmanijak commented May 16, 2023

⚠️ This PR will be merged after PR #9634, as It also contains changes made in #9634

This PR adds a setting to Product Collection to inherit the global query.
image

When enabled:

  • all the filters and ordering options are hidden
  • the actual global query should be in use
  • make sure Product Collection works fine with the Catalog Sorting and Product Results Count blocks

Fixes #9358

Testing

Testing with Product Archive templates

  1. Open the Editor and navigate to the "Product Search Results"
  2. Replace the existing 'Classic Template' block with the 'Product Collection' block.
  3. After the 'Product Collection' block is successfully inserted, ensure that the 'Inherit query from template' option is enabled by default.
  4. Save the changes.
  5. Navigate to the frontend of your website. You can emulate a search for "shirt" by visiting the URL "/shop?s=shirt". Confirm that the search query shirt effectively displays related products.
  6. Head back to the Editor.
  7. In the 'Product Collection' block, deactivate the 'Inherit query from template' option.
  8. Subsequently, you should observe various settings and filters available in the inspector controls. Modify these filters and settings as required and validate immediately that the changes are visible within the Editor.
  9. Save your modifications and return to the frontend of your website.
  10. Ensure that the adjustments made to the 'Product Collection' block within the Editor are accurately displayed on the frontend of your website.

Here is a quick demo:

Screen.Recording.2023-05-30.at.5.47.05.PM.mov

Testing in Post

  1. Create a new post
  2. Add Product Collection block to the post
  3. With the 'Product Collection' block selected, open the Inspector Controls (located in the right sidebar). Confirm that the option inherit query from template does not appear in the list of options. This is because it's relevant only for Product Archive pages and should not be visible in this context.
Screen.Recording.2023-05-30.at.5.48.55.PM.mov
  • Do not include in the Testing Notes

WooCommerce Visibility

  • WooCommerce Core
  • Feature plugin
  • Experimental

Changelog

Product Collection - Add Inherit query from template control

imanish003 and others added 7 commits May 15, 2023 16:57
- `InspectorControls` from './inspector-controls' is now imported in `edit.tsx` and used in the returned JSX of `Edit` function.
- A new file `columns-control.tsx` is added under 'product-collection' block's 'inspector-controls' directory which exports a `ColumnsControl` component. This component uses `RangeControl` from '@wordpress/components' to control the number of columns in the product collection display layout when the layout type is 'flex'.
- The types file (`types.ts`) for 'product-collection' block is updated. The `Attributes` interface is renamed to `ProductCollectionAttributes` and the `ProductCollectionContext` interface is removed. The `ProductCollectionAttributes` now includes 'queryContext', 'templateSlug', and 'displayLayout' properties.
This commit simplifies the fallback return value of the ColumnsControl component. Instead of returning an empty fragment (<> </>), it now returns null when the condition isn't met. This change improves readability and aligns with best practices for conditional rendering in React.
This commit adds a new 'Order By' control to the product collection inspector. The control allows users to specify the order of products in a collection by various attributes such as title and date. To support this, a new component 'OrderByControl' has been created and included in the product collection inspector. Additionally, the types for 'order' and 'orderBy' attributes have been updated and exported for reuse.
…ocks into 9359-product-collection-editor-settings-order-by
@github-actions
Copy link
Contributor

github-actions bot commented May 16, 2023

The release ZIP for this PR is accessible via:

https://wcblocks.wpcomstaging.com/wp-content/uploads/woocommerce-gutenberg-products-block-9485.zip

Script Dependencies Report

There is no changed script dependency between this branch and trunk.

This comment was automatically generated by the ./github/compare-assets action.

TypeScript Errors Report

  • Files with errors: 469
  • Total errors: 2256

🎉 🎉 This PR does not introduce new TS errors.

comments-aggregator

@github-actions
Copy link
Contributor

github-actions bot commented May 16, 2023

Size Change: +534 B (0%)

Total Size: 1.1 MB

Filename Size Change
build/active-filters.js 7.47 kB +2 B (0%)
build/all-products.js 40 kB -4 B (0%)
build/all-reviews.js 7.85 kB +2 B (0%)
build/attribute-filter.js 13.2 kB +3 B (0%)
build/cart.js 45.1 kB +1 B (0%)
build/checkout.js 46.4 kB -2 B (0%)
build/featured-category.js 15.1 kB +1 B (0%)
build/featured-product.js 15.3 kB +3 B (0%)
build/filter-wrapper-frontend.js 14.3 kB -2 B (0%)
build/handpicked-products.js 8.04 kB +3 B (0%)
build/legacy-template.js 6.45 kB -1 B (0%)
build/mini-cart-component-frontend.js 28.5 kB -1 B (0%)
build/mini-cart-contents.js 18 kB +17 B (0%)
build/mini-cart.js 4.43 kB -2 B (0%)
build/price-filter.js 8.55 kB +4 B (0%)
build/product-best-sellers.js 8.35 kB +3 B (0%)
build/product-categories.js 2.7 kB -2 B (0%)
build/product-category.js 9.37 kB +3 B (0%)
build/product-collection.js 11.2 kB +513 B (+5%) 🔍
build/product-new.js 8.65 kB +3 B (0%)
build/product-on-sale.js 8.69 kB +3 B (0%)
build/product-query.js 11.8 kB -1 B (0%)
build/product-top-rated.js 8.91 kB +2 B (0%)
build/products-by-attribute.js 9.75 kB +1 B (0%)
build/rating-filter.js 6.9 kB +7 B (0%)
build/reviews-by-category.js 12.1 kB +1 B (0%)
build/reviews-by-product.js 13.3 kB +2 B (0%)
build/single-product.js 11.1 kB +1 B (0%)
build/stock-filter.js 7.62 kB +10 B (0%)
build/wc-blocks-vendors.js 65 kB -37 B (0%)
build/wc-blocks.js 3.69 kB +1 B (0%)
ℹ️ View Unchanged
Filename Size
build/active-filters-frontend.js 8.65 kB
build/active-filters-wrapper-frontend.js 7.61 kB
build/all-products-frontend.js 12 kB
build/attribute-filter-wrapper--stock-filter-wrapper-frontend.js 4.04 kB
build/attribute-filter-wrapper-frontend.js 4.29 kB
build/blocks-checkout.js 35.1 kB
build/breadcrumbs.js 2.13 kB
build/cart-blocks/cart-accepted-payment-methods-frontend.js 1.39 kB
build/cart-blocks/cart-cross-sells-frontend.js 254 B
build/cart-blocks/cart-cross-sells-products--product-price-frontend.js 2.92 kB
build/cart-blocks/cart-cross-sells-products-frontend.js 3.71 kB
build/cart-blocks/cart-express-payment--checkout-blocks/express-payment-frontend.js 5.17 kB
build/cart-blocks/cart-express-payment-frontend.js 720 B
build/cart-blocks/cart-items-frontend.js 301 B
build/cart-blocks/cart-line-items--mini-cart-contents-block/products-table-frontend.js 5.51 kB
build/cart-blocks/cart-line-items-frontend.js 1.06 kB
build/cart-blocks/cart-order-summary-frontend.js 1.27 kB
build/cart-blocks/cart-totals-frontend.js 308 B
build/cart-blocks/empty-cart-frontend.js 346 B
build/cart-blocks/filled-cart-frontend.js 656 B
build/cart-blocks/order-summary-coupon-form-frontend.js 1.63 kB
build/cart-blocks/order-summary-discount-frontend.js 2.13 kB
build/cart-blocks/order-summary-fee-frontend.js 273 B
build/cart-blocks/order-summary-heading-frontend.js 333 B
build/cart-blocks/order-summary-shipping-frontend.js 17.1 kB
build/cart-blocks/order-summary-subtotal-frontend.js 274 B
build/cart-blocks/order-summary-taxes-frontend.js 435 B
build/cart-blocks/proceed-to-checkout-frontend.js 1.38 kB
build/cart-frontend.js 29.9 kB
build/catalog-sorting.js 1.7 kB
build/checkout-blocks/actions-frontend.js 1.85 kB
build/checkout-blocks/billing-address--checkout-blocks/shipping-address-frontend.js 4.7 kB
build/checkout-blocks/billing-address-frontend.js 1.19 kB
build/checkout-blocks/contact-information-frontend.js 2.04 kB
build/checkout-blocks/express-payment-frontend.js 1.14 kB
build/checkout-blocks/fields-frontend.js 332 B
build/checkout-blocks/order-note-frontend.js 1.14 kB
build/checkout-blocks/order-summary-cart-items-frontend.js 3.68 kB
build/checkout-blocks/order-summary-coupon-form-frontend.js 1.79 kB
build/checkout-blocks/order-summary-discount-frontend.js 2.29 kB
build/checkout-blocks/order-summary-fee-frontend.js 277 B
build/checkout-blocks/order-summary-frontend.js 1.27 kB
build/checkout-blocks/order-summary-shipping-frontend.js 17 kB
build/checkout-blocks/order-summary-subtotal-frontend.js 274 B
build/checkout-blocks/order-summary-taxes-frontend.js 436 B
build/checkout-blocks/payment-frontend.js 8.28 kB
build/checkout-blocks/pickup-options-frontend.js 4.85 kB
build/checkout-blocks/shipping-address-frontend.js 1.17 kB
build/checkout-blocks/shipping-method-frontend.js 2.63 kB
build/checkout-blocks/shipping-methods-frontend.js 6.4 kB
build/checkout-blocks/terms-frontend.js 1.56 kB
build/checkout-blocks/totals-frontend.js 361 B
build/checkout-frontend.js 32 kB
build/customer-account.js 3.18 kB
build/filter-wrapper.js 2.39 kB
build/general-style-rtl.css 1.31 kB
build/general-style.css 1.31 kB
build/mini-cart-contents-block/cart-button-frontend.js 1.73 kB
build/mini-cart-contents-block/checkout-button-frontend.js 1.73 kB
build/mini-cart-contents-block/empty-cart-frontend.js 362 B
build/mini-cart-contents-block/filled-cart-frontend.js 267 B
build/mini-cart-contents-block/footer-frontend.js 4.09 kB
build/mini-cart-contents-block/items-frontend.js 237 B
build/mini-cart-contents-block/products-table-frontend.js 593 B
build/mini-cart-contents-block/shopping-button-frontend.js 526 B
build/mini-cart-contents-block/title-frontend.js 1.9 kB
build/mini-cart-contents-block/title-items-counter-frontend.js 1.59 kB
build/mini-cart-contents-block/title-label-frontend.js 1.53 kB
build/mini-cart-frontend.js 2.72 kB
build/price-filter-wrapper-frontend.js 6.75 kB
build/price-format.js 1.19 kB
build/product-add-to-cart--product-button--product-image--product-price--product-rating--product-sale-bad--49d3ecb2.js 250 B
build/product-add-to-cart--product-button--product-image--product-rating--product-title.js 151 B
build/product-add-to-cart-frontend.js 6.52 kB
build/product-add-to-cart.js 8.84 kB
build/product-button--product-image--product-price--product-rating--product-sale-badge--product-sku--prod--5bce0384.js 954 B
build/product-button-frontend.js 2.65 kB
build/product-button.js 3.97 kB
build/product-image-frontend.js 2.62 kB
build/product-image.js 4.14 kB
build/product-price-frontend.js 203 B
build/product-price.js 1.68 kB
build/product-rating-frontend.js 2.27 kB
build/product-rating.js 971 B
build/product-results-count.js 1.66 kB
build/product-sale-badge-frontend.js 1.79 kB
build/product-sale-badge.js 665 B
build/product-search.js 2.63 kB
build/product-sku-frontend.js 1.84 kB
build/product-sku.js 535 B
build/product-stock-indicator-frontend.js 2.03 kB
build/product-stock-indicator.js 730 B
build/product-summary-frontend.js 2.19 kB
build/product-summary.js 905 B
build/product-tag.js 9.01 kB
build/product-template.js 3.34 kB
build/product-title-frontend.js 2.21 kB
build/product-title.js 3.66 kB
build/rating-filter-frontend.js 21.4 kB
build/rating-filter-wrapper-frontend.js 6.2 kB
build/reviews-frontend.js 7.18 kB
build/stock-filter-wrapper-frontend.js 2.98 kB
build/store-notices.js 1.69 kB
build/vendors--attribute-filter-wrapper--cart-blocks/order-summary-coupon-form--cart-blocks/order-summary--48e1e4bb-frontend.js 6.82 kB
build/vendors--attribute-filter-wrapper--cart-blocks/order-summary-shipping--checkout-blocks/billing-addr--d9f38f9d-frontend.js 4.21 kB
build/vendors--attribute-filter-wrapper--stock-filter-wrapper-frontend.js 5.11 kB
build/vendors--cart-blocks/cart-cross-sells-products--cart-blocks/cart-line-items--cart-blocks/cart-order--3c5fe802-frontend.js 5.26 kB
build/vendors--cart-blocks/cart-line-items--checkout-blocks/order-summary-cart-items--mini-cart-contents---233ab542-frontend.js 3.57 kB
build/vendors--cart-blocks/order-summary-shipping--checkout-blocks/billing-address--checkout-blocks/order--decc3dc6-frontend.js 19.4 kB
build/vendors--checkout-blocks/pickup-options--checkout-blocks/shipping-methods-frontend.js 8.25 kB
build/vendors--checkout-blocks/shipping-method-frontend.js 12.5 kB
build/vendors--price-filter-wrapper-frontend.js 2.2 kB
build/vendors--product-add-to-cart-frontend.js 7.26 kB
build/vendors--rating-filter-wrapper-frontend.js 5.11 kB
build/wc-blocks-data.js 22.4 kB
build/wc-blocks-editor-style-rtl.css 6.01 kB
build/wc-blocks-editor-style.css 6 kB
build/wc-blocks-google-analytics.js 1.56 kB
build/wc-blocks-middleware.js 933 B
build/wc-blocks-registry.js 3.15 kB
build/wc-blocks-shared-context.js 1.52 kB
build/wc-blocks-shared-hocs.js 1.75 kB
build/wc-blocks-style-rtl.css 27.9 kB
build/wc-blocks-style.css 27.8 kB
build/wc-blocks-vendors-style-rtl.css 1.96 kB
build/wc-blocks-vendors-style.css 1.96 kB
build/wc-payment-method-bacs.js 816 B
build/wc-payment-method-cheque.js 811 B
build/wc-payment-method-cod.js 909 B
build/wc-payment-method-paypal.js 837 B
build/wc-settings.js 2.6 kB
build/wc-shipping-method-pickup-location.js 30.4 kB
build/woo-directives-runtime.js 2.73 kB
build/woo-directives-vendors.js 7.91 kB

compressed-size-action

The main changes include:
1. Added a new property 'isProductCollectionBlock' in the block.json to denote if a block is a product collection block.
2. In the ProductCollection PHP class, a new initialization function has been defined to hook into the WordPress lifecycle, register the block, and update the query based on this block.
3. Added methods to manage query parameters for both frontend rendering and the Editor.
4. Expanded allowed 'collection_params' for the REST API to include custom 'orderby' values.
5. Defined a function to build the query based on block attributes, filters, and global WP_Query.
6. Created utility functions to handle complex query operations such as merging queries, handling custom sort values, and merging arrays recursively.

These improvements allow for more flexible and robust handling of product collections in both the front-end display and the WordPress editor. It also extends support for custom 'orderby' values in the REST API, which allows for more advanced sorting options in product collections.
@kmanijak kmanijak added skip-changelog PRs that you don't want to appear in the changelog. block: product collection Issues related to the Product Collection block labels May 18, 2023
@kmanijak kmanijak marked this pull request as ready for review May 18, 2023 07:59
@woocommercebot woocommercebot requested review from a team and albarin and removed request for a team May 18, 2023 07:59
@kmanijak kmanijak requested a review from imanish003 May 18, 2023 08:00
Copy link
Contributor

@imanish003 imanish003 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @kmanijak, great job as always! 🙌🏻

I have tested the pull request, and it's working as described in the testing steps. 🚀
I wanted to mention that currently, in the Products beta block, we hide the "Inherit query from template" setting when it doesn't make sense, such as when adding the block to a post. Maybe we should do the same for the Product Collection block as well. What do you think? 🙂

At the moment initial query settings are hardcoded in the Product Collection. Change the default settings, for example by changing the order to desc in here

I merged the trunk with this branch, and now we have an option in the sidebar settings to change the order. Therefore, we won't need to perform this step 🙂

@kmanijak kmanijak marked this pull request as draft May 18, 2023 17:20
@kmanijak
Copy link
Contributor Author

I wanted to mention that currently, in the Products beta block, we hide the "Inherit query from template" setting when it doesn't make sense, such as when adding the block to a post. Maybe we should do the same for the Product Collection block as well. What do you think? 🙂

@imanish003, I agree. I added this logic in this commit, but there's a problem that I won't have time to fix.

When adding the Product Collection, attributes are correctly set in the Editor (client-side rendered), but when you save with no changes and go to frontend then query is not consumed. This is due to fact that until any change is made in the inspector control and until setAttributes function is called the attributes are not saved to the block and hence not read by server-side.

Before changing any setting After changing setting
image image

I tried extending this initial setAttributes with the query here, but even though everything looked fine I couldn't find why the query attribute is not set. I'm pretty sure it's something dumb and simple, but under time pressure I couldn't find it.

  1. I changed the testing steps to include "Order by" setting and the logic that "Inherit..." option is enabled in templates and not disabled in post/page editor
  2. I converted this PR to a draft until the initial attributes problem is solved.
  3. Like with the other PR, feel free to pick it up or wait until I'm back - depends on the priorities 🙏

…ction block

This commit introduces several changes to the product collection block.
- First, it adds a new 'on sale' filter that can be used to display only the products that are currently on sale.
- It also refactors the settings management in the product collection block to use the experimental ToolsPanel component from WordPress, which provides a more flexible and intuitive way to manage block settings.
- It moves the 'Columns' control into the ToolsPanel, along with the 'Order by' control.
- A new utility function `setQueryAttribute` is introduced to simplify setting nested query parameters.
- The structure of the `ProductCollectionAttributes` and `ProductCollectionQuery` types have been adjusted to accommodate the changes.
- Finally, it makes corresponding changes in the PHP part to handle the new 'on sale' query parameter.

This should enhance the flexibility and user-friendliness of the product collection block.
the taxonomy controls have been enhanced in the following ways:
1. Modified the BASE_QUERY object to include 'slug' in the '_fields' property. This will ensure that the 'slug' of the taxonomy term is fetched along with its 'id' and 'name'.

2. Added a 'slug' property to the Term type to store the 'slug' of each term.

3. Updated the useEffect hook inside the TaxonomyItem function to generate suggestions based on search results. The suggestions now include the 'slug' of a term if the term's name is not unique. This change will help users distinguish between terms with the same name but different slugs.
Following changes were made:

1. The useSelect hooks which were being used to fetch existing terms and search results have been moved into their own custom hooks named 'useExistingTerms' and 'useSearchResults' respectively. This simplifies the TaxonomyItem function's body and makes the hooks' purposes clearer.

2. The comments and props destructuring for the TaxonomyItem function have been moved up to make it easier to understand the function's purpose and the props it receives.

3. This refactor does not introduce any changes in functionality. It only changes how the code is organized and presented, which will make future development easier.
This commit enhances the `TaxonomyControls` component within `product-collection` block by adding memoization and improving term uniqueness handling.

Changes:

1. Imported `useMemo` from `@wordpress/element` for memoizing certain results.

2. `getTermIdByTermValue` function has been modified to use a `termIdToNameMap` (term ids as keys and term names as values). This provides a more efficient and direct mapping for term search.

3. Introduced `useTermIdToNameMap` function, which returns a `Map` where term ids are keys and term names are values. It handles duplicate term names by appending the term slug to the name, ensuring unique term names.

4. Updated the `useExistingTerms` and `useSearchResults` to include `taxonomy` in their dependency arrays for `useSelect` hook. This will force re-computation when `taxonomy` changes.

5. Changed `TaxonomyItem` from a function declaration to a const arrow function, consistent with the rest of the codebase.

6. Updated `onTermsChange` function in `TaxonomyItem` to accommodate the changes in `getTermIdByTermValue` and the introduction of `termIdToNameMap`.

7. Replaced `Set` with a standard array for storing new term IDs in `onTermsChange`. The `Set` was unnecessary as term IDs are unique by default.

8. Updated `TaxonomyItem`'s effects and rendering to work with `termIdToNameMap`, ensuring the displayed term names are unique.

This update will result in more efficient term search and handling, and it will solve issues related to duplicate term names.
This commit restructures the taxonomy controls in the product collection block for improved clarity and maintainability.
- The file `taxonomy-controls.tsx` has been deleted, and its functionality has been divided into two new files: `index.tsx` and `taxonomy-item.tsx`.

- The `index.tsx` file contains the main TaxonomyControls component, which is responsible for displaying taxonomy-related options in the block's inspector controls. It includes a custom hook `useTaxonomies` that fetches and returns taxonomies associated with product post type.

- The `taxonomy-item.tsx` file, on the other hand, contains a TaxonomyItem component that handles the rendering of individual taxonomy items. It also contains some utility functions for mapping term names and ids and fetching terms based on the search query.

This refactor aims to improve code readability and separation of concerns, thus making future changes and maintenance easier.
@imanish003 imanish003 changed the title Add Inherit global query control to the Product Collection Product Collection - Add Inherit query from template control Jun 1, 2023
@imanish003 imanish003 changed the base branch from trunk to 9364-product-collection-filters-taxonomies June 1, 2023 11:57
This change enhances the search functionality of the FormTokenField by introducing support for case insensitive search. This has been achieved by adding a lower-case version of the term name to the 'termNameToIdMap'.

This is an important enhancement as it will make the search process more user-friendly and resilient to different casing inputs. Users will now be able to find the desired taxonomy term regardless of their input's case.
This commit does a couple of important things:
1. Reorders the definition of constants in `TaxonomyItemProps` for clarity.
2. Refactors the `getTermIdByTermValue` function. Instead of checking for an exact term name match in a convoluted manner, it now directly tries to fetch the `id` from the `searchTerm` if it is an object. If the `searchTerm` is not an object, the function tries to match it against the `termNameToIdMap` in both normal and lowercase forms. This simplification makes the function more readable and concise.
3. Updates the `newSuggestions` mapping in the `TaxonomyItem` component. It now has a fallback to `searchResult.name` if a term's name is not found in `termIdToNameMap`. This change ensures that even if the term's name is not in the map for some reason, we can still display a suggestion using the original name of the term.
This commit introduces a couple of improvements to the TaxonomyItem component.

1. The initial state of the 'search' state variable has been updated to 'undefined'. This change helps prevent unnecessary initial fetching of terms when the search input is empty.

2. Term fetching logic has been optimized to only enable term fetching when necessary:
   a) Fetching based on the search query is only enabled when 'search' is not 'undefined'.
   b) Fetching existing terms is only enabled when there are term IDs.

3. The block of code responsible for fetching existing terms and setting the current value has been moved upwards. This reordering of code does not change the functionality, but it groups together similar pieces of code, enhancing readability and maintainability.

These optimizations make the component more efficient by reducing unnecessary requests and computations, and they improve the code organization.
Base automatically changed from 9364-product-collection-filters-taxonomies to trunk June 2, 2023 10:01
Copy link
Contributor

@Aljullu Aljullu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is testing as expected and the code looks overall good, but I left some questions/comments below: 👇

@@ -22,6 +22,8 @@ import { getDefaultSettings } from '../constants';
const ColumnsControl = (
props: BlockEditProps< ProductCollectionAttributes >
) => {
if ( ! props.attributes.displayLayout ) return null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused by this change, does it have any relation to the rest of the PR or is it a generic code cleanup? (Sorry if it might seem obvious, but wanted to make sure I'm not missing anything)

As a side note, I noticed the ColumnsControl doesn't have any title or label. Is that on purpose? I feel it's a bit confusing to have a range input field without any info about what it does:

imatge

(No need to fix it in this PR, but wanted to mention it anyway 🙂 )

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused by this change, does it have any relation to the rest of the PR or is it a generic code cleanup? (Sorry if it might seem obvious, but wanted to make sure I'm not missing anything)

Actually, there was an issue where the displayLayout sometimes became null. Therefore, I added an early return statement. However, I have now removed it in 34762c5 as it's no longer necessary due to other changes made in the commit.

I truly appreciate your thorough PR reviews. 🙇‍♂️

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a side note, I noticed the ColumnsControl doesn't have any title or label. Is that on purpose? I feel it's a bit confusing to have a range input field without any info about what it does:

I have also added a missing label in 34762c5. Thanks for pointing this out 🙌🏻

Comment on lines 85 to 87
query={
props.attributes.query as ProductCollectionQuery
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this could be changed to query, now that we define it in line 33, no? Also, on the first render after inserting the block, query might be undefined, so I would remove or update the type (as ProductCollectionQuery). What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated, Thanks for pointing this out 🙌🏻

Comment on lines 63 to 65
setQueryAttribute( {
inherit: initialValue,
} );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes, when adding the Product Collection block into the Site Editor, I get this JS warning in the console:

Warning: Cannot update a component (DocumentActions) while rendering a different component (InheritQueryControl). To locate the bad setState() call inside InheritQueryControl, follow the stack trace as described in...

Can you reproduce as well?

I think the error is caused by this setQueryAttribute(). If I'm understanding the code correctly, it's trying to update the state during the render function, which causes an unnecessary re-render and might cause other side effects. I think we should set the value of inherit to initialValue right when we read it, so in assets/js/blocks/product-collection/inspector-controls/index.tsx. Would that work? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can also see this warning. I have relocated the code responsible for calculating the inherit value to:
edit.tsx.

Now, I am setting the inherit value while defining the default attributes. Additionally, I have made some other minor improvements in 34762c5.

What are your thoughts on the recent changes? Do they look good to you?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, looks good. I initially thought the useEffect() would not be necessary, but I guess because of WordPress/gutenberg#7342, there is no way we can avoid it. 😞

This commit makes several changes:
1. The useEffect that sets the default attributes was moved and modified. This now includes a `query` attribute that utilizes the imported function `getDefaultValueOfInheritQueryFromTemplate`.
2. An early return was added in `edit.tsx` to prevent rendering until default attributes are set.
3. In `columns-control.tsx`, the early return was removed and a label was added to the `RangeControl` component.
4. In `inherit-query-control.tsx`, logic related to `inherit` value initial setting was refactored using a `useMemo` hook with `getDefaultValueOfInheritQueryFromTemplate` function. This logic was moved to a separate utility function in `utils`.
5. The `query` attribute is no longer optional in `types.ts`.
6. A new utility function `getDefaultValueOfInheritQueryFromTemplate` was created in `utils.tsx` to encapsulate the logic of deciding the default value of `inherit` query attribute based on the current template.

These changes aim to improve code clarity and maintainability.
@imanish003
Copy link
Contributor

@Aljullu I have made the changes. Can you please take a look? 🙂

Also, I am immensely grateful for yet another remarkable PR review you have provided. Your consistent delivery of exceptional feedback is truly appreciated. Thank you for consistently going above and beyond in providing such outstanding feedback. 🙌🏻 🙇‍♂️

@imanish003 imanish003 requested a review from Aljullu June 2, 2023 12:42
Copy link
Contributor

@Aljullu Aljullu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for all the improvements, @imanish003. It looks good to me! 🚢

@@ -10,11 +10,12 @@ import { useEffect } from '@wordpress/element';
* Internal dependencies
*/
import { ImageSizing } from '../../atomic/blocks/product-elements/image/types';
import { ProductCollectionAttributes } from './types';
import { ProductCollectionAttributes, ProductCollectionQuery } from './types';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: should we intentionally use import type here?

Suggested change
import { ProductCollectionAttributes, ProductCollectionQuery } from './types';
import type { ProductCollectionAttributes, ProductCollectionQuery } from './types';

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although I don't have a strong opinion about this so I will go ahead and add the type with import. It might help in clarifying the intent. 🤝

Comment on lines 63 to 65
setQueryAttribute( {
inherit: initialValue,
} );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, looks good. I initially thought the useEffect() would not be necessary, but I guess because of WordPress/gutenberg#7342, there is no way we can avoid it. 😞

@github-actions github-actions bot added this to the 10.4.0 milestone Jun 2, 2023
@imanish003 imanish003 enabled auto-merge (squash) June 5, 2023 05:52
@imanish003 imanish003 merged commit dc59d5f into trunk Jun 5, 2023
29 checks passed
@imanish003 imanish003 deleted the add/product-collection-inherit branch June 5, 2023 06:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
block: product collection Issues related to the Product Collection block skip-changelog PRs that you don't want to appear in the changelog.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Product Collection - Editor settings: Inherit query from template
4 participants