-
Notifications
You must be signed in to change notification settings - Fork 10.7k
Add product title to header when available #35431
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: 236c53e
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.
Nice work @joshuatf, this tested well! Also thanks for separating this into different components 🎉
I just left one comment in relation to the call to registerPlugin otherwise this looks good.
| registerPlugin( 'woocommerce-product-title', { | ||
| render: ProductTitle, | ||
| icon: 'admin-generic', | ||
| } ); |
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 registerPlugin is not necessary as we are rendering it as part of the ProductFormHeader anyway, so we could remove this.
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 catch!
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 🚀 Thanks for removing the registerPlugin logic, this looks good!
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.
Rebase looks good, LGTM 🚀
All Submissions:
Changes proposed in this Pull Request:
Adds the product title to the header when available, otherwise adds a "add new" string to the header based on product type.
Note that product types haven't been officially added yet, so a simple product type placeholder exists.
Closes #35172 .
How to test the changes in this Pull Request:
Other information:
pnpm --filter=<project> changelog add?FOR PR REVIEWER ONLY: