-
Notifications
You must be signed in to change notification settings - Fork 10.7k
Add product status badge to product form header #35460
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: 7c0762e
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. |
louwie17
left a comment
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 looks good and tested well, I did leave one suggestion on using a Typescript enum instead of a JS object for the product status keys, this would help make the types a bit more robust.
But approving none the less.
| draft: 'draft', | ||
| instock: 'instock', | ||
| outofstock: 'outofstock', | ||
| }; |
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.
Instead of making this an object, should we make this a TypeScript enum?
enum ProductStatusKeys {
unsaved = 'unsaved',
...
}
That way we could pass the specific status types from getProductStatus instead of just string
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.
Good call! Updated!
068ecab to
236c53e
Compare
louwie17
left a comment
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.
Nice work on this @joshuatf this looks good, thanks for the enum update as well!
Approving this, although it does look like there might be some lint errors that need fixing.
louwie17
left a comment
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.
✅ LGTM 🚀
All Submissions:
Changes proposed in this Pull Request:
Adds the product status badge to the form header.
Closes #35169 .
How to test the changes in this Pull Request:
0and saveOther information:
pnpm --filter=<project> changelog add?FOR PR REVIEWER ONLY: