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: Prompt to save when there are unsaved changes #4945

Open
wants to merge 2 commits into
base: mealie-next
Choose a base branch
from

Conversation

SrGesus
Copy link

@SrGesus SrGesus commented Jan 26, 2025

What this PR does / why we need it:

When you are editing a recipe in RecipePage and close the edit mode, there are now unsaved changes but one is not prompted to save those changes.
The discussion on issue #2268 settled on prompting for saving when the close is clicked, but i think it would make more sense to prompt for unsaved changes on exit, where they are already being prompted (but were previously only if edit mode was on).
So to achieve this the only changes that need to be done are removing the condition of being in editing mode to be prompted for unsaved changes, and to fix an issue regarding the saving not updating the originalRecipe that should be updated whenever the recipe is saved.
Also adding a snackbar when not in edit mode where you can choose to discard the changes, and the original recipe will be displayed.

Which issue(s) this PR fixes:

Fixes #2268

@SrGesus SrGesus force-pushed the fix-recipe-unsaved-settings branch 3 times, most recently from 0524a1b to fb8e020 Compare January 27, 2025 18:14
@SrGesus SrGesus marked this pull request as ready for review January 27, 2025 18:15
@SrGesus SrGesus force-pushed the fix-recipe-unsaved-settings branch 2 times, most recently from 6f5d147 to 9d463fd Compare January 30, 2025 11:42
@SrGesus
Copy link
Author

SrGesus commented Jan 30, 2025

Fixed the linting warnings

@SrGesus SrGesus force-pushed the fix-recipe-unsaved-settings branch from 5fc1c25 to 9d463fd Compare February 25, 2025 09:14
@SrGesus SrGesus force-pushed the fix-recipe-unsaved-settings branch from 9d463fd to 55ff8dd Compare February 25, 2025 09:15
@SrGesus SrGesus force-pushed the fix-recipe-unsaved-settings branch from 55ff8dd to 269706f Compare February 25, 2025 09:31
Copy link
Collaborator

@Kuchenpirat Kuchenpirat left a comment

Choose a reason for hiding this comment

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

Hey, thanks for your contribution, and sorry for me being a bit late to the party. Vacation got in the way.

There are still a few things that need updating around this PR.

When a user is leaving the page recipe edit page (e.g. by clicking the mealie icon top left) and changes were made we have an alert poping up. We should replace the alert with your implementation for consistency and a nicer experience.

:timeout="0"
top
>
{{ snackbarText }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of returning the string from script you can just use this:

Suggested change
{{ snackbarText }}
{{ $tc("general.unsaved-changes") }}

@@ -1,5 +1,20 @@
<template>
<div>
<v-snackbar
Copy link
Collaborator

Choose a reason for hiding this comment

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

We already have a Snackbar that is usable across the app via use-toast.

You should check if you can use the existing one, if not you should style it to one of the existing styles.

@Kuchenpirat Kuchenpirat self-assigned this Feb 25, 2025
@SrGesus
Copy link
Author

SrGesus commented Feb 25, 2025

We already have a Snackbar that is usable across the app via use-toast.
You should check if you can use the existing one, if not you should style it to one of the existing styles.

I had looked into that but would have to change how the toasts works to make it behave the way this snackbar is.
It would be doable if there were another toast like "toastConfirm" with two buttons (and remove the timeout of 2s) but it might be unnecessarily complex, i would somehow need to make it save and discard depending on which button was pressed (I don't know if there is a good vue pattern for this behaviour, or if TheSnackBar from layout would somehow have to run an arbitrary function? which sounds kinda bad).

When a user is leaving the page recipe edit page (e.g. by clicking the mealie icon top left) and changes were made we have an alert poping up. We should replace the alert with your implementation for consistency and a nicer experience.

That pop up previously only appeared when you were in edit mode and prevents you from leaving the page, I did change it to also appear when you have unsaved changes outside of edit mode. (and the snackbar only appears when outside of edit mode to warn the user + gives him the option to easily discard without having to enter edit mode again which i think is a plus)
But so i'm not understanding what you're suggesting with this. The user is prevented from changing page without selecting an option in the snackbar? Or make the pop up only appear when leaving the page in edit mode like it was before and have just the snackbar, no pop up, outside edit mode? or have no pop up at all and don't prevent the user from leaving the page?

@Kuchenpirat
Copy link
Collaborator

Yeah, your argumentation towards the first point makes sense, i would still prefer this to be its own component so this can be reused somewhere else if needed and is not hidden away somewhere within the recipe page.

Towards the seccond point. I want to get rid of the alert style window confirms and replace them with the snackbar you implemented here to have a coherent user experience.
Especially since the two dialogs are showing the same text it feels a bit out of place that they behave differently depending on how and where you leave the page with unsaved changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[v1.0.0b] - Closing a recipe from edit shows cached recipe initially
2 participants