Add taxonomy block using SelectTree #39947
Conversation
Test Results SummaryCommit SHA: 8c134f3
To view the full API test report, click here. To view the full E2E test report, click here. To view all test reports, visit the WooCommerce Test Reports Dashboard. |
|
Hi @mattsherman, Apart from reviewing the code changes, please make sure to review the testing instructions as well. You can follow this guide to find out what good testing instructions should look like: |
There was a problem hiding this comment.
Nice start on this @nathanss !
I need to take a closer look, but some initial thoughts...
- Some testing instructions that don't depend on the brands PR/plugin would be good
- Relatedly, it would be good to verify that this works with other custom taxonomies (I tried creating some with the "Custom Post Type UI" plugin, but I don't know why they don't seem to be appearing in the REST API responses (which I think might be the problem).
- It would be ideal if this block automatically handled both hierarchical and non-hierarchical taxonomies. I would think that should be possible without too much hassle.
| const [ selectedEntries, setSelectedEntries ] = useEntityProp< Taxonomy[] >( | ||
| 'postType', | ||
| 'product', | ||
| taxonomyNamePlural |
There was a problem hiding this comment.
I think this is actually not necessarily the plural version of the taxonomy name, but rather the name of the property used in the REST API.
| margin-top: 36px; | ||
| display: flex; | ||
| flex-direction: row; | ||
| gap: 8px; |
| overflow: visible; | ||
|
|
||
| &__buttons { | ||
| margin-top: 36px; |
|
@mattsherman thx for the review!
I can't think of a good way of doing this. I don't know how using
You're right, we need to have the property in the REST API. AFAIK we don't have any other property on the product to test this on for now.
I will work on that. |
Ideally, I think testing one PR should not depend on changes in another PR. I have figured out how to test this without the Brands plugin (which is needed to test the non-hierarchical case too).
I figured out how to accomplish this.
Thanks! Okay, here is an approach I have figured out to test this easily...
function YOUR_PREFIX_rest_api_add_custom1_to_product( $response, $post ) {
$post_id = $post->get_id();
if ( empty( $response->data[ 'custom1' ] ) ) {
$terms = [];
foreach ( wp_get_post_terms( $post_id, 'custom-taxonomy' ) as $term ) {
$terms[] = [
'id' => $term->term_id,
'name' => $term->name,
'slug' => $term->slug,
];
}
$response->data[ 'custom1' ] = $terms;
}
return $response;
}
add_filter( 'woocommerce_rest_prepare_product_object', 'YOUR_PREFIX_rest_api_add_custom1_to_product', 10, 2 );
$section_fields[] = [
'woocommerce/taxonomy-field',
[
'taxonomyName' => 'custom-taxonomy',
'taxonomyNameOnEntity' => 'custom1',
'label' => __( 'Custom Taxonomy', 'woocommerce' ),
'createTitle' => __( 'Create new custom taxonomy', 'woocommerce' ),
]
]; |
| "type": "string", | ||
| "__experimentalRole": "content" | ||
| }, | ||
| "taxonomyNameOnEntity": { |
There was a problem hiding this comment.
How about property? That's what other generic fields like our checkbox and radio button blocks use.
| "hierarchical": { | ||
| "type": "boolean", | ||
| "default": false, | ||
| "__experimentalRole": "content" | ||
| } |
There was a problem hiding this comment.
I don't think the consumer should set this. We can determine this by querying for details on the taxonomy.
| "keywords": [ "taxonomy"], | ||
| "textdomain": "default", | ||
| "attributes": { | ||
| "taxonomyName": { |
There was a problem hiding this comment.
This is the taxonomy slug. Maybe we should call it taxonomySlug.
|
@mattsherman Awesome review, thanks a lot. I updated the code with the suggested changes. I also updated the PR title and description, since it evolved a lot since the beginning. Can you check again? |
3df9767 to
ce9f2f3
Compare
ce9f2f3 to
d7805ce
Compare
|
Note: forgot that we have to handle setting the taxonomy in the REST request as well... updated the testing instructions to account for that. |
There was a problem hiding this comment.
Testing seems to work pretty well with the brands taxonomy, as well as two custom taxonomies I created, one hierarchical, one flat (after I added the handling to save the taxonomy in the REST handler*).
The main issue I ran into during testing was that the parent dropdown in the modal does not show parents hierarchically.
* I suppose that this generic taxonomy block only works out of the box if the custom taxonomy developer has implemented support for the REST API as is being done for brands and categories (same shape of data for the property). We'll have to document this. I think it is a reasonable limitation, and maybe one we can extend over time if there are other patterns.
| "name": "woocommerce/taxonomy-field", | ||
| "title": "Taxonomy", | ||
| "category": "widgets", | ||
| "description": "A block that displays a taxonomy field, allowing to search, select, and create new entries", |
There was a problem hiding this comment.
Suggestion...
"A block that displays a taxonomy field, allowing searching, selection, and creation of new items"
packages/js/product-editor/src/blocks/taxonomy/create-taxonomy-modal.tsx
Show resolved
Hide resolved
| const hierarchical = useSelect( ( select ) => { | ||
| const taxonomyDetails = select( 'core' ).getTaxonomy( | ||
| attributes.slug | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any |
Submission Review Guidelines:
Changes proposed in this Pull Request:
This creates a "Hierarchical Taxonomy" block, which will be firstly used for WooCommerce Brands plugin, and also replace woocommerce/product-category-field block in the future.
Part of #39206 .
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
Changelog entry
Significance
Type
Message
Comment