Create ProductForm component to merge duplicated UI #35783
Conversation
github-actions
bot
added
focus: react admin
plugin: woocommerce
Test Results SummaryCommit SHA: eb0907f
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. |
nathanss
changed the title
…n add-product-page and edit-product-page
nathanss
force-pushed
the
enhancement/35766
branch
from
0636194 to
eed83b7
Compare
joshuatf
left a comment
joshuatf
left a comment
There was a problem hiding this comment.
Great work, Nathan! Left one possible addition to further cut down our duplication, but this is testing well and otherwise looks good. 😄
| return ( | ||
| <Form< Partial< Product > > | ||
| initialValues={ | ||
| product || { |
There was a problem hiding this comment.
Love this! Clean and simple ❤️
| import { ProductFormFooter } from './layout/product-form-footer'; | ||
| import { ProductForm } from './product-form'; | ||
|
|
||
| const AddProductPage: React.FC = () => { |
There was a problem hiding this comment.
Could we merge AddProductPage and EditProductPage into a single component ProductPage? I think we could use const { productId } = useParams(); to distinguish whether or not we're creating a new product or editing one.
There was a problem hiding this comment.
I think so! Let me work on this
There was a problem hiding this comment.
@nathanss attempted this but we ran into 2 issues:
getPermalinkPartsdid not seem to get the parts correctly when switching between pages without a full refresh. Possibly related to encoding of slashes in the URL but I'm not entirely sure. cc @louwie17 if you have any context around this- React router did not re-render properly when switching from a created product to a new product form. This may be due to them sharing the same component and caching happening on the routing level.
Since the remainder of the logic has been merged in this PR, I don't think we should let this block us from merging this one.
…duct-page: running into some issues probably with the controller
…ngle product-page: running into some issues probably with the controller" This reverts commit bc30b67.
joshuatf
left a comment
joshuatf
left a comment
There was a problem hiding this comment.
Thanks so much for taking a stab at my initial feedback. This is testing well and LGTM!
Left a comment around the issues we ran into in case we want to update this to a single component in the future. 🚢
| import { ProductFormFooter } from './layout/product-form-footer'; | ||
| import { ProductForm } from './product-form'; | ||
|
|
||
| const AddProductPage: React.FC = () => { |
There was a problem hiding this comment.
@nathanss attempted this but we ran into 2 issues:
getPermalinkPartsdid not seem to get the parts correctly when switching between pages without a full refresh. Possibly related to encoding of slashes in the URL but I'm not entirely sure. cc @louwie17 if you have any context around this- React router did not re-render properly when switching from a created product to a new product form. This may be due to them sharing the same component and caching happening on the routing level.
Since the remainder of the logic has been merged in this PR, I don't think we should let this block us from merging this one.
All Submissions:
Changes proposed in this Pull Request:
Creates a new component ProductForm to merge duplicated UI that existed between AddProductPage and EditProductPage.
No behavior should change.
Closes #35766.
How to test the changes in this Pull Request:
Other information:
pnpm --filter=<project> changelog add?FOR PR REVIEWER ONLY: