Skip to content

Add a confirmation modal when the user tries to navigate away with unsaved changes #35625

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.

Sign up for GitHub

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

Jump to bottom
Merged
mdperez86 merged 3 commits into from
Nov 18, 2022
Merged

Add a confirmation modal when the user tries to navigate away with unsaved changes #35625

mdperez86 merged 3 commits into from
Nov 18, 2022

Conversation

@mdperez86
Copy link
Contributor

@mdperez86 mdperez86 commented Nov 17, 2022 •
edited
Loading

All Submissions:

Changes proposed in this Pull Request:

Closes #35194

  • This PR is a very minor change/addition and does not require testing instructions (if checked you can ignore/remove the next section).

How to test the changes in this Pull Request:

  1. Go to Products -> Add New (MVP)
  2. Change any field without saving it
  3. Try to leave the page
  4. A confirmation dialog should be shown preventing you from leaving the page without saving the changes first

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you created a changelog file for each project being changed, ie pnpm --filter=<project> changelog add?

FOR PR REVIEWER ONLY:

  • I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

@mdperez86 mdperez86 added Enhancement The issue is a request for an enhancement. focus: product management labels Nov 17, 2022
@mdperez86 mdperez86 self-assigned this Nov 17, 2022
@github-actions github-actions bot added focus: react admin plugin: woocommerce Issues related to the WooCommerce Core plugin. labels Nov 17, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Nov 17, 2022 •
edited
Loading

Test Results Summary

Commit SHA: a83ba72

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests26000202620m 51s
E2E Tests186006019213m 59s
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.

@mdperez86 mdperez86 force-pushed the enhancement/35194-confirm-when-exiting branch 2 times, most recently from a0120aa to fc86cf0 Compare November 17, 2022 17:35
@mdperez86 mdperez86 requested a review from a team November 17, 2022 17:37
@mdperez86 mdperez86 marked this pull request as ready for review November 17, 2022 17:43
@mdperez86 mdperez86 force-pushed the enhancement/35194-confirm-when-exiting branch from fc86cf0 to cca422c Compare November 17, 2022 18:13
louwie17
louwie17 previously approved these changes Nov 18, 2022
View reviewed changes
Copy link
Contributor

@louwie17 louwie17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test well as is, although I did leave one question, but the original issue seems to imply exactly that window, so all good on my end.

I did notice though, that this only works when a user refreshes the page or navigates to another page that isn't a React page.
If I navigate to a React page, let's say from Products > Add New (MVP) to WooCommerce > Home with changes, I don't see a warning any my changes are lost.
Could we have a separate PR to address this @mdperez86 ?

plugins/woocommerce-admin/client/products/product-form-actions.tsx
const { isDirty, isValidForm, values, resetForm } =
useFormContext< Product >();

usePreventLeavingPage( isDirty );
Copy link
Contributor

@louwie17 louwie17 Nov 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes you made may not be saved. is this the default text that shows up in the popup? I was confused where this was coming from.
Also I assume Changes you made may not be saved. will be supported by different languages, as the browser will take care of that? Otherwise I did say we should pass ours in manually.

Copy link
Contributor Author

@mdperez86 mdperez86 Nov 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately changing this message is not fully supported by all browsers you can check here. So the message is managed by the browser in those cases.

@mdperez86 mdperez86 requested a review from louwie17 November 18, 2022 14:03
louwie17
louwie17 previously approved these changes Nov 18, 2022
View reviewed changes
Copy link
Contributor

@louwie17 louwie17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding the React router support as well, this tested very well, nice work @mdperez86 💯

louwie17
louwie17 approved these changes Nov 18, 2022
View reviewed changes
Copy link
Contributor

@louwie17 louwie17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing the tests.

@mdperez86 mdperez86 merged commit dba6d33 into trunk Nov 18, 2022
@mdperez86 mdperez86 deleted the enhancement/35194-confirm-when-exiting branch November 18, 2022 14:55
@github-actions github-actions bot added this to the 7.3.0 milestone Nov 18, 2022
@github-actions github-actions bot mentioned this pull request Jan 13, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@louwie17 louwie17 louwie17 approved these changes

Assignees

@mdperez86 mdperez86

Labels

Enhancement The issue is a request for an enhancement. plugin: woocommerce Issues related to the WooCommerce Core plugin.

Projects

None yet

Milestone

7.3.0

Development

Successfully merging this pull request may close these issues.

[Enhancement]: Add a confirmation modal when the user tries to navigate away with unsaved changes

3 participants

@mdperez86 @louwie17