Add modal editor for use in product editor #37937
Conversation
|
Hi @joelclimbsthings, @woocommerce/mothra 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: |
Test Results SummaryCommit SHA: 8f3a5b5
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
requested review from
joelclimbsthings
and removed request for
a team
|
Hey @joshuatf , I attempted to review this but the pnpm upgrades were causing issues. I attempted to downgrade again, but still no 🎲 . This will need a rebase as is, so feel free to ping me for that review when that's done! |
|
Hey @joelclimbsthings I just rebased this and should be good for review. You may still have to upgrade pnpm or clean cache as I know these have been updated in |
joelclimbsthings
left a comment
joelclimbsthings
left a comment
There was a problem hiding this comment.
Nice work @joshuatf ! I had a couple minor questions regarding the styles. I'm also assuming that the appearance isn't intended to be part of this PR, but just in case I'll mention that the paragraph blocks still appear gray on my end.
|
|
||
| @import 'components/radio-field/style.scss'; | ||
| @import 'components/iframe-editor/style.scss'; | ||
| @import 'components/details-categories-field/style.scss'; |
There was a problem hiding this comment.
Did you intend to remove these two files for the categories field?
There was a problem hiding this comment.
Nope. Error on my part during wonky rebase. Added back in.
| @@ -0,0 +1 @@ | |||
| @import './modal-editor.scss'; | |||
There was a problem hiding this comment.
I'm curious why we have multiple levels of scss imports here? Do we need both this file and modal-editor-scss files to exist?
There was a problem hiding this comment.
I was following our pattern of having an index.ts file with a separate component-name.tsx file that is imported there. It's easier to find and identify stylesheets this way that are named component-name.scss. It also creates an entry file when multiple imports do exist.
That said, I'm not overly opinionated on this and have removed it for the time being.
joelclimbsthings
left a comment
joelclimbsthings
left a comment
There was a problem hiding this comment.
Thanks @joshuatf , all looks well on my end 🚢
* Add modal editor for use in product editor * Allow title of modal to be modified by consumer * Add changelog entry * Remove errant style imports after rebase * Handle PR feedback
Submission Review Guidelines:
Changes proposed in this Pull Request:
Adds a modal editor that can be used in the product editor for descriptions and other fields that require separate editors.
This PR only addresses fitting the iframe editor into the editor and does not include the toolbar, sidebars, or other functionality which will be addressed in follow-ups.
Closes #37898 .
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions: