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
Move product editor utils to product editor package #36730
Conversation
Test Results SummaryCommit SHA: a8e801f
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. |
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.
Thanks for getting this done @joshuatf ! Gave the editor a pretty good smoke test, and all works well. I added one minor comment, but it's totally optional, so feel free to .
/** | ||
* Utils | ||
*/ | ||
export { formatCurrencyDisplayValue } from './utils/format-currency-display-value'; |
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.
Likely subjective, but an alternative here would be to add an index.ts
file within the /utils
directory to export all utilities, and then potentially just export * from './utils'
. Makes it a little smoother to import these utils within the package itself, since you can import all from ../utils
.
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.
@joshuatf I also noticed that the original utils (such as getCheckboxTracks
) haven't been removed, and they're actually still in use by various fills and such. Were you planning to divide this into a separate PR for some reason?
3ca861d
to
a8e801f
Compare
Thanks for taking a look at this, @joelclimbsthings! It's been a few weeks since I opened this PR, so I'm not sure why I left the section utils separate, but I went ahead and moved them over. Also moved the util import/exports to a separate file. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## trunk #36730 +/- ##
========================================
Coverage 46.7% 46.7%
Complexity 17178 17178
========================================
Files 429 429
Lines 64779 64779
========================================
Hits 30240 30240
Misses 34539 34539 |
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.
Thanks Josh, changes look good and still tests well. 🚢
All Submissions:
Changes proposed in this Pull Request:
Moves all utils used in the new product editing experience to the
@woocommerce/product-editor
package. Other components and hooks will come in follow-up PRs.Closes #36719 .
How to test the changes in this Pull Request:
Other information:
pnpm --filter=<project> changelog add
?FOR PR REVIEWER ONLY: