Update/34885 category field in product editor

[louwie17]

All Submissions:

• Have you followed the WooCommerce Contributing guideline?
• Does your code follow the WordPress' coding standards?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

This updates the product category metabox with the current product editor with a react version of it that replaced the list of categories with a async search dropdown.
The async search dropdown makes use of the Tree Select Control component.

Closes #34885 .

Kapture.2023-02-21.at.16.51.21.mp4

• This PR is a very minor change/addition and does not require testing instructions (if checked you can ignore/remove the next section).

How to test the changes in this Pull Request:

• Have you followed the Writing high-quality testing instructions guide?

1. Enable the async-product-editor-category-field feature flag using the WooCommerce Beta Tester.
2. Load this branch and create a new store and be sure to create some new Categories.
3. Go to Products > Add New
4. The usual category metabox should be present if you have less then a 100 categories.
5. Create either more then a 100 categories or add this filter to your mu-plugin -> add_filter( 'woocommerce_product_category_metabox_search_threshold', function() { return 5; } );
6. Add a name and scroll down to the Product Categories metabox on the left hand side
7. Try search for some of the categories you previously added and select these.
8. Save the product, the categories should persist.
9. Other items to check:
   • Selecting a popular category should automatically select the category within the popular tab and vice versa
   • Creating a new category should automatically add the new item to the selected list.
   • Creating a new category and selecting a parent should work correctly.

Other information:

• [] Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes, as applicable?
• Have you created a changelog file for each project being changed, ie pnpm --filter=<project> changelog add?

FOR PR REVIEWER ONLY:

• I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

[codecov]

Codecov Report

Merging #36869 (dbcdfc5) into trunk (c226fa1) will decrease coverage by 0.0%.
The diff coverage is 0.0%.

❗ Current head dbcdfc5 differs from pull request most recent head 28c8964. Consider uploading reports for the commit 28c8964 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             trunk   #36869     +/-   ##
==========================================
- Coverage     51.6%    51.5%   -0.0%     
- Complexity   17260    17279     +19     
==========================================
  Files          429      430      +1     
  Lines        79928    79950     +22     
==========================================
- Hits         41213    41211      -2     
- Misses       38715    38739     +24     

Impacted Files	Coverage Δ
...ocommerce/includes/admin/class-wc-admin-assets.php	0.0% <0.0%> (ø)
...eta-boxes/class-wc-meta-box-product-categories.php	0.0% <0.0%> (ø)
plugins/woocommerce/includes/class-wc-ajax.php	4.3% <0.0%> (-0.1%)	⬇️
...ugins/woocommerce/includes/class-wc-post-types.php	1.2% <0.0%> (+0.1%)	⬆️

... and 2 files with indirect coverage changes

[github-actions]

Test Results Summary

Commit SHA: 28c8964

Test 🧪	Passed ✅	Failed 🚨	Broken 🚧	Skipped ⏭️	Unknown ❔	Total 📊	Duration ⏱️
API Tests	259	0	0	2	0	261	1m 6s
E2E Tests	187	0	0	10	0	197	14m 10s

---

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.

[nathanss]

I saw these two errors at the console at some point: (trying to figure out how to reproduce, I'll edit if/when I figure it out):

Warning: validateDOMNesting(...): <ul> cannot appear as a descendant of <p>.
Warning: validateDOMNesting(...): <div> cannot appear as a descendant of <p>.

[nathanss]

Overall feedback:

It's working well, I couldn't find any bugs while testing, besides the console error I reported in the previous comment. Congratulations, it must have been pretty hard and complex to do this!

As for the appearance, the sizing looks bigger than the rest of the elements. f.e the 30px height of the input compared to the product tags input, 20px height of the checkbox when comparing to the brand's checkbox. Some screenshots for reference:

[nathanss]

'ele' is 'him' in Portuguese... That's funny. No need to change it, though 😂

[mattsherman]

element would be more readable. Or even categoryElement.

[github-actions]

Hi @mattsherman, @nathanss,

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:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

[louwie17]

@nathanss this should be ready for a re-review, I addressed all your changes, thanks for the initial review and sorry for the delay in updating this :)

[mattsherman]

Overall, works pretty well, though I didn't test extensively, since I spent a lot of time reviewing the code (this PR would have been nice to split up... perhaps into one that just reviewed the React component and another that focused on the integration).

@louwie17 This is a pretty impressive job... it's cool to see the existing screen being evolved with a React component. 🥳

Left a bunch of comments with suggestions for readability improvements.... most of them are just suggestions.

Functionality-wise, one change that jumped out to me was that you can't select a child without selecting the parent in the new tree. You can remove the parent from the list below the "Add category" field though, so maybe that is okay?

Echoing what @nathanss already mentioned, the font size in the new component looks too big compared with the rest of the UI it is in...

Changing the font size to 12px results in a things looking better, though we'd also want to adjust the checkbox sizing since it still looks too big...

The case where a user enters in text into the "Add category" field that doesn't match anything feels off... I'd probably expect to be able to add a new category when I did that, instead of the field just sitting there with no indication that anything is wrong or what to do next.

[mattsherman]

element would be more readable. Or even categoryElement.

[louwie17]

@mattsherman thanks for the detailed review, I believe I have addressed all your feedback and fixed the font sizing issues.

This PR should be ready for a re-review.

[mattsherman]

@mattsherman thanks for the detailed review, I believe I have addressed all your feedback and fixed the font sizing issues.

This PR should be ready for a re-review.

There are a number of unresolved comments from the last time I reviewed this PR. I have went through and marked the ones that were actually resolved as resolved. It would be great to resolve those remaining unresolved comments, ideally with a short comment, so that I know you looked at the comment and considered it. Many are suggestions, so it's okay if you resolve them by saying "thanks but no thanks" 😄. I just don't want them to go unnoticed!

More importantly though, there are a number of bugs I found in my latest round of testing...

• Add category field
  • If the dropdown is dismissed via ESC key, it doesn't come back unless you un-focus and then re-focus the field
  • When I start typing "ho", "Clothing", the parent for "Hoodies" shows in the dropdown list. But, "Hoodies" itself doesn't show until I type "hoo"
• Parent category search
  • When typing something in, immediately erases it
  • Once a parent is selected from dropdown list, there is no way to remove the parent category
  • No way to select text in the parent category field

Here is a video that demos the bugs... complete with a non-polished voice-over!

Product.categories.bugs.mov

[mattsherman]

Found bugs that need to be addressed (see other comment).

[louwie17]

Thanks for taking another detailed look @mattsherman 💯

There are a number of unresolved comments from the last time I reviewed this PR. I have went through and marked the ones that were actually resolved as resolved. It would be great to resolve those remaining unresolved comments, ideally with a short comment, so that I know you looked at the comment and considered it. Many are suggestions, so it's okay if you resolve them by saying "thanks but no thanks" 😄. I just don't want them to go unnoticed!

Sorry, and thanks for calling this out. I felled in a bit of a hurry (for some reason, I think before my AFK at that time), so end up only replying to a few. I believe I have addressed all your items now, and left a couple comments or responded with an Emoji :)

• Add category field

  • If the dropdown is dismissed via ESC key, it doesn't come back unless you un-focus and then re-focus the field

This was a bug with the Tree select control, I have fixed it here: 6729f4d ( I fixed the item below in this commit as well)

• When I start typing "ho", "Clothing", the parent for "Hoodies" shows in the dropdown list. But, "Hoodies" itself doesn't show until I type "hoo"

Yeah they had a minimum filter limit set, which was set to 3, I added a prop to manually set it and changed it to 2 (1 seemed a bit unnecessary).

• Parent category search

  • When typing something in, immediately erases it

This was an annoying one to figure out, but this was because of a useEffect logic that had the getItemLabel as a dependency, moving this function outside of the component fixed this: b0d4568

• Once a parent is selected from dropdown list, there is no way to remove the parent category

I added support for this in the tree select control by exposing a individuallySelectParent prop: d207573

• No way to select text in the parent category field

This should be fixed now, this was a bug in the select control where preventDefault was being called even when the input field was being clicked, fixed here: 4251ebe

@mattsherman this should be ready for a re-view assuming all the actions will pass at the time of writing this :)

[louwie17]

@mattsherman I wrapped the logic in a async-product-editor-category-field feature flag, so it will only modify the category taxonomy config type if the feature is enabled.
This will allow us to merge this first part that has been tested, and to work on the follow ups.

[mattsherman]

@louwie17 Thanks for putting this behind a feature flag.

I verified that with the feature flag disabled, the old category selector is used. With the feature flag enabled, the new category selector is used if the number of categories exceeds the threshold.