Skip to content
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

fix: Warn on external navigation during editing #2914

Merged
merged 8 commits into from
Feb 21, 2024

Conversation

ekcom
Copy link
Contributor

@ekcom ekcom commented Jan 6, 2024

What type of PR is this?

(REQUIRED)

  • bug

What this PR does / why we need it:

  • Provide a last-second warning when navigating
  • Covers navigation which the current navigation on unsaved changes alert
    does not handle (before this, only internal redirection gave the alert)

Which issue(s) this PR fixes:

Fixes #2912

@ekcom
Copy link
Contributor Author

ekcom commented Jan 6, 2024

Note: this currently will display the warning message (e.g., "Reload site? Changes you made may not be saved.") on the edit page even if no changes were made.

I am not sure how to efficiently detect any changes. Can't check JSON.stringify(props.recipe) === JSON.stringify(originalRecipe.value) during the beforeunload event, so would probably need an event listener for any changes in the form. This may not be worth the performance cost.

This feature still works and is ready.

@ekcom ekcom marked this pull request as ready for review January 6, 2024 22:44
Copy link
Collaborator

@boc-the-git boc-the-git left a comment

Choose a reason for hiding this comment

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

LGTM. Tested it a bit and does what it says on the tin. Nice work!

I do agree with what you've suggested, that there's some inconsistency with some of the other internal navigation/warnings (e.g. refreshing the page (new warning) is different to navigating to the timeline (old warning)), but importantly this does work and will stop people losing data.

Further enhancements are absolutely welcome! :)

@boc-the-git boc-the-git merged commit 8b88f68 into mealie-recipes:mealie-next Feb 21, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] - Warn when leaving edit page
2 participants