-
Notifications
You must be signed in to change notification settings - Fork 10.7k
Update empty state for product attributes tab #38126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Test Results SummaryCommit SHA: 190820c
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. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## trunk #38126 +/- ##
==========================================
- Coverage 51.3% 51.2% -0.0%
- Complexity 17409 17417 +8
==========================================
Files 440 440
Lines 80584 80634 +50
==========================================
- Hits 41311 41310 -1
- Misses 39273 39324 +51
|
|
Hi @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: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
@nathanss Your screen recording shows the |
|
@mattsherman I'll double check. But I was checked out at the branch and ran build. |
|
@mattsherman It is happening in this branch. Here's a print from the minified code:
|
|
@nathanss That is not the minified code from the source in this PR. That is from older source. The minified code from this branch is... var e, o, r;
null === (o = document.querySelector("#product_attributes .add_custom_attribute")) || void 0 === o || o.addEventListener("click", (()=>{
(0,
t.recordEvent)("product_attributes_buttons", {
action: "add_new"
})
}
)),
null === (r = window) || void 0 === r || r.jQuery("select.wc-attribute-search").on("select2:select", (function() {
(0,
t.recordEvent)("product_attributes_buttons", {
action: "add_existing"
})
}
)),What build command are you running? The |
|
@mattsherman Sorry. It was a problem at my end. All manual tests are passing! |
plugins/woocommerce/client/legacy/js/admin/meta-boxes-product.js
Outdated
Show resolved
Hide resolved
plugins/woocommerce/client/legacy/js/admin/meta-boxes-product.js
Outdated
Show resolved
Hide resolved
…t.preventDefault is used.
| .get(); | ||
|
|
||
| // If the product has no attributes, add an empty attribute to be filled out by the user. | ||
| $( function add_blank_custom_attribute_if_no_attributes() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function doesn't necessarily need to be named, especially if it's only being used in this one place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naming functions is very helpful when you are looking at an outline of the code in an editor, debugging a stack trace, and to help document what the code is doing.



Submission Review Guidelines:
Changes proposed in this Pull Request:
This PR updates the empty state for the product
Attributestab so that it always shows an empty local attribute form, which makes it quicker for users to add attributes to a product. This empty local attribute form is shown regardless of whether any global attributes are in the system.Also, it is mentioned in the issue acceptance criteria, but bears calling out here as well.. the header buttons are now always the
Add newandAdd existingones, replacing the olderCustom product attributedropdown andAddbutton.The implementation of this change required some updates to the logging of the following Tracks events:
wcadmin_product_attributes_buttonswithaction: 'add_new'Add newbutton is clickedwcadmin_product_attributes_buttonswithaction: 'add_existing'Add existingdropdownwcadmin_product_attributes_add_termCreate valueis clicked to create a new value for a global attribute (this is logged regardless of whether a new value is actually created, to keep behavior consistent with how it was prior to this PR)The e2e tests were updated to reflect the change in CSS classes used in the implementation.
Before
Empty state when no global attributes exist:
Empty state when global attributes exist:
After
Closes #38104.
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
Common instructions
Products>Add NewProducts>All ProductsProducts>AttributesSave attributesbutton on theAttributestabPublish(orUpdate, if an existing product)Tools>WCA Test Helper>Tools>Enable wc-admin Trackingrecordeventin the browser dev consoleNo global attributes in system
AttributestabGlobal attributes in system, adding a new local attribute
AttributestabGlobal attributes in system, adding a global attribute
AttributestabAdd existingdropdownAdd existingdropdownEditing an existing product with no attributes
Attributestab is shownEditing an existing product with local attributes
Attributestab is shownEditing an existing product with global attributes
Attributestab is shownTracks events
Add newshould logwcadmin_product_attributes_buttonswithaction: 'add_new'Add existingshould logwcadmin_product_attributes_buttonswithaction: 'add_existing'Create valuefor a global attribute should logwcadmin_product_attributes_add_term