Fix/unsaved prompt #35657
Fix/unsaved prompt #35657
Conversation
github-actions
bot
added
focus: react admin
package: @woocommerce/components
Test Results SummaryCommit SHA: 0605938
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. |
| navigateTo( { | ||
| url: 'admin.php?page=wc-admin&path=/product/' + product.id, | ||
| } ); | ||
| } |
There was a problem hiding this comment.
This logic should also be as part of the onSaveDraft function right after line 58.
There was a problem hiding this comment.
oh!!! you are right. It's done now. Thanks @louwie17
louwie17
left a comment
louwie17
left a comment
There was a problem hiding this comment.
Nice work @mdperez86, this was testing quite well. I did notice a React warning about a component state being changed, which disappeared after passing in the product to the resetForm.
Also let one tiny nit pick comment on the error notice text.
| createProductWithStatus( values, 'publish' ); | ||
| const product = await createProductWithStatus( values, 'publish' ); | ||
| if ( product?.id ) { | ||
| resetForm(); |
There was a problem hiding this comment.
We should also pass in the product into the resetForm here.
The transition happens fast enough that the user doesn't notice, but I do get a uncontrolled component has been changed warning if we call it without any product object.
There was a problem hiding this comment.
Ok, but I don't think the right way is passing the product obj here since we are reseting a creation form not an edition one. Which means that when creating the form starts empty. That why I didn't pass the product obj here. If there is a problem regarding to uncontrolled/controlled is because the invalid initial value of the form at the moment it was mounted. Remember the resetForm takes the form's initialValues by default if we don't pass any obj to the function.
There was a problem hiding this comment.
In that case shouldn't we be resetting it to this object:
{
reviews_allowed: true,
name: '',
sku: '',
stock_quantity: 0,
stock_status: 'instock',
}
Which is the same object we initially start the form with.
| createProductWithStatus( values, 'draft' ); | ||
| const product = await createProductWithStatus( values, 'draft' ); | ||
| if ( product?.id ) { | ||
| resetForm(); |
There was a problem hiding this comment.
Same comment as below, we should pass the product into the resetForm here.
| ( error ) => { | ||
| createNotice( | ||
| 'error', | ||
| __( 'Failed to moved product to Trash.', 'woocommerce' ) |
There was a problem hiding this comment.
This should be Failed to move product to Trash.
moved -> move
louwie17
left a comment
louwie17
left a comment
There was a problem hiding this comment.
Nice work, and thanks for the updates! LGTM 🚀
All Submissions:
Changes proposed in this Pull Request:
Fixes #35194
How to test the changes in this Pull Request:
Products -> Add New (MVP)pageOther information:
pnpm --filter=<project> changelog add?FOR PR REVIEWER ONLY: