Add create attribute term modal #35131

louwie17 · 2022-10-17T16:29:53Z

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 allows users to create a new attribute term if the term they are searching for doesn't exist within the attribute term list.
This is currently dependent on #34999 to be merged, which is why it's pointing at that branch.

Kapture.2022-10-17.at.13.25.48.mp4

Address the term creation part in #34331 .

How to test the changes in this Pull Request:

Load this branch, build it, and make sure you have the new-product-management-experience feature flag enabled (you can do so using the latest version of the Beta tester within the mono repo).
Go to Products > Add new (MVP) and fill out some of the initial details like name and price
Scroll down and click Add first attribute in the attributes section
Select one of the attributes
Search for a attribute term that doesn't exist yet
The 'Create "your new attribute term name"' should show as an option
Click that and another modal should show with the create fields.
Fill out all the fields and click Add this should create the attribute term and add it as a selected item within the terms field
With localStorage.setItem( 'debug', 'wc-admin:*' ); run prior, it should show that a product_attribute_term_add and product_attribute_term_add_success event has been fired.
Open the modal again and select cancel this shouldn't do anything.
A user should not be able to click add when the name is removed.

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 successfully run tests with your changes locally?
Have you created a changelog file for each project being changed, ie pnpm --filter=<project> run 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.

github-actions · 2022-10-17T16:52:53Z

Test Results Summary

Commit SHA: 38dcbb3

Test 🧪	Passed ✅	Failed 🚨	Broken 🚧	Skipped ⏭️	Unknown ❔	Total 📊	Duration ⏱️
API Tests	232	0	0	3	0	235	0m 57s
E2E Tests	186	0	0	6	0	192	14m 59s

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.

joelclimbsthings · 2022-10-27T19:36:10Z

...merce-admin/client/products/fields/attribute-term-input-field/attribute-term-input-field.tsx

+				getFilteredItems={ ( allItems, inputValue ) => {
+					if (
+						inputValue.length > 0 &&
+						! allItems.find(


Just a note that this will fail if it returns an item that is falsy in any way, but it looks unlikely to cause any issues here.

Yeah that should be fine, plus the find method already assume the each item contains a name attribute, so, so the item being return will have to contain a name attribute none the less.

...merce-admin/client/products/fields/attribute-term-input-field/attribute-term-input-field.tsx

joelclimbsthings · 2022-10-27T19:51:20Z

...erce-admin/client/products/fields/attribute-term-input-field/create-attribute-term-modal.tsx

+type CreateAttributeTermModalProps = {
+	initialAttributeTermName: string;
+	attributeId: number;
+	onCancel: () => void;


I'm wondering if there's a possible use case in which these handlers should be optional?

I don't think so, but I made them optional just incase, I defaulted them to empty functions in this commit: 2f39f27

joelclimbsthings · 2022-10-27T20:00:45Z

...erce-admin/client/products/fields/attribute-term-input-field/create-attribute-term-modal.tsx

+							<TextControl
+								label={ __( 'Name', 'woocommerce' ) }
+								{ ...nameInputProps }
+								onBlur={ () => {


If the onBlur here fires, it actually closes both the terms modal and the add attribute modal, which I'd guess wouldn't be expected. Should we stop it from bubbling down to the add modal as well?

Hmm could you maybe share a video of this? Things work as expected for me and the Modal doesn't auto close for me when the onBlur is being fired.

joelclimbsthings

Great job @louwie17 !
Added a few very minor comments, and I also noticed that the fields are not responsive on smaller viewports, which might be worth addressing here.

It's also worth noting that we're definitely going to have some conflicts with the edit attributes PR which includes some semi-foundational changes, so we might want to get that merged before too long.

louwie17 · 2022-10-28T18:48:21Z

Great job @louwie17 ! Added a few very minor comments, and I also noticed that the fields are not responsive on smaller viewports, which might be worth addressing here.

Thanks for the feedback @joelclimbsthings I addressed most of your feedback, the only item I couldn't reproduce was the onBlur issue you are encountering, I was curious if you could share a video or something, it seems a bit odd to me that this onBlur would trigger that.
I did also address the smaller viewports issue, nice catch!

It's also worth noting that we're definitely going to have some conflicts with the edit attributes PR which includes some semi-foundational changes, so we might want to get that merged before too long.

Get which one merged first?

joelclimbsthings

Right on, thanks @louwie17 ! I'm not sure if this behavior is actually much of a problem, but I can see it being annoying to users:

At the end of the animation I'm clicking at the spot I'm circling, and you can see that it's closing both modals, not just the terms modal.

Edit: Either way on merging the PRs. I'm good to resolve the conflicts for this one, but don't want the edit modal PR to get too bogged down with other changes coming down.

louwie17 · 2022-10-28T20:39:08Z

Right on, thanks @louwie17 ! I'm not sure if this behavior is actually much of a problem, but I can see it being annoying to users:

At the end of the animation I'm clicking at the spot I'm circling, and you can see that it's closing both modals, not just the terms modal.

Aaaha thanks that video helps a lot, I should have a fix Monday morning sometime, it should be relatively simple to handle.

…cking outside

louwie17 · 2022-10-31T13:09:24Z

@joelclimbsthings I fixed the issue in this commit: 38dcbb3
Only the second modal should close now, and afterwards the first modal should still close fine.

Hope you have some time to look at it again.

joelclimbsthings · 2022-10-31T21:02:52Z

Awesome @louwie17 , works beautifully. 🚢

github-actions · 2022-11-01T12:06:03Z

Hi @louwie17, thanks for merging this pull request. Please take a look at these follow-up tasks you may need to perform:

Add the release: add testing instructions label

louwie17 requested a review from a team October 17, 2022 16:29

github-actions bot added focus: react admin release: highlight Issues that have a high user impact and need to be discussed/paid attention to. labels Oct 17, 2022

louwie17 force-pushed the add/34331_add_attributes_modal branch from 8d5ce35 to 7420a10 Compare October 18, 2022 20:22

Base automatically changed from add/34331_add_attributes_modal to trunk October 19, 2022 19:28

louwie17 force-pushed the add/34331_create_attribute_term_modal branch from 80e7590 to 32001b9 Compare October 20, 2022 12:15

github-actions bot added plugin: woocommerce Issues related to the WooCommerce Core plugin. and removed release: highlight Issues that have a high user impact and need to be discussed/paid attention to. labels Oct 20, 2022

joelclimbsthings requested review from joelclimbsthings and removed request for a team October 27, 2022 18:59

joelclimbsthings reviewed Oct 27, 2022

View reviewed changes

...merce-admin/client/products/fields/attribute-term-input-field/attribute-term-input-field.tsx Outdated Show resolved Hide resolved

joelclimbsthings reviewed Oct 27, 2022

View reviewed changes

joelclimbsthings requested changes Oct 27, 2022

View reviewed changes

louwie17 force-pushed the add/34331_create_attribute_term_modal branch from 111c87c to 2f39f27 Compare October 28, 2022 18:44

louwie17 requested a review from joelclimbsthings October 28, 2022 18:48

joelclimbsthings requested changes Oct 28, 2022

View reviewed changes

louwie17 added 6 commits October 31, 2022 10:07

Add create attribute term modal

e2a0179

Add back filter missed during rebase

496ee77

Add changelog

c1b8c1d

Fix lint error

74b2335

Address some feedback from PR review

8e89947

Prevent first modal from closing if closing the second modal when cli…

38dcbb3

…cking outside

louwie17 force-pushed the add/34331_create_attribute_term_modal branch from 2f39f27 to 38dcbb3 Compare October 31, 2022 13:07

louwie17 requested a review from joelclimbsthings October 31, 2022 13:09

louwie17 requested a review from a team October 31, 2022 13:09

joelclimbsthings approved these changes Oct 31, 2022

View reviewed changes

louwie17 merged commit ff2b06c into trunk Nov 1, 2022

louwie17 deleted the add/34331_create_attribute_term_modal branch November 1, 2022 12:05

github-actions bot added this to the 7.2.0 milestone Nov 1, 2022

github-actions bot mentioned this pull request Dec 15, 2022

Test WooCommerce 7.2 UVLabs/location-picker-at-checkout-for-woocommerce#57

Closed

github-actions bot mentioned this pull request Jan 25, 2023

Test WooCommerce 7.3 UVLabs/location-picker-at-checkout-for-woocommerce#59

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add create attribute term modal #35131

Add create attribute term modal #35131

louwie17 commented Oct 17, 2022

github-actions bot commented Oct 17, 2022 •

edited

Loading

joelclimbsthings Oct 27, 2022

louwie17 Oct 28, 2022

joelclimbsthings Oct 27, 2022

louwie17 Oct 28, 2022

joelclimbsthings Oct 27, 2022

louwie17 Oct 28, 2022

joelclimbsthings left a comment •

edited

Loading

louwie17 commented Oct 28, 2022

joelclimbsthings left a comment •

edited

Loading

louwie17 commented Oct 28, 2022

louwie17 commented Oct 31, 2022

joelclimbsthings commented Oct 31, 2022

github-actions bot commented Nov 1, 2022

Add create attribute term modal #35131

Add create attribute term modal #35131

Conversation

louwie17 commented Oct 17, 2022

All Submissions:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Other information:

FOR PR REVIEWER ONLY:

github-actions bot commented Oct 17, 2022 • edited Loading

Test Results Summary

joelclimbsthings Oct 27, 2022

Choose a reason for hiding this comment

louwie17 Oct 28, 2022

Choose a reason for hiding this comment

joelclimbsthings Oct 27, 2022

Choose a reason for hiding this comment

louwie17 Oct 28, 2022

Choose a reason for hiding this comment

joelclimbsthings Oct 27, 2022

Choose a reason for hiding this comment

louwie17 Oct 28, 2022

Choose a reason for hiding this comment

joelclimbsthings left a comment • edited Loading

Choose a reason for hiding this comment

louwie17 commented Oct 28, 2022

joelclimbsthings left a comment • edited Loading

Choose a reason for hiding this comment

louwie17 commented Oct 28, 2022

louwie17 commented Oct 31, 2022

joelclimbsthings commented Oct 31, 2022

github-actions bot commented Nov 1, 2022

github-actions bot commented Oct 17, 2022 •

edited

Loading

joelclimbsthings left a comment •

edited

Loading

joelclimbsthings left a comment •

edited

Loading