Warn before leaving recipe editor with changes #464
Conversation
Signed-off-by: Sebastian Fey <[email protected]>
…d prompting user to confirm Signed-off-by: Sebastian Fey <[email protected]>
seyfeb
changed the title
Signed-off-by: Sebastian Fey <[email protected]>
seyfeb
force-pushed
the
feature/warnBeforeLeavingRecipeEdit
branch
from
43fad70 to
3d12202
Compare
Codecov Report
@@ Coverage Diff @@
## master #464 +/- ##
========================================
Coverage 0.91% 0.91%
Complexity 414 414
========================================
Files 13 13
Lines 1307 1307
========================================
Hits 12 12
Misses 1295 1295
Flags with carried forward coverage won't be shown. Click here to find out more. |
christianlupus
left a comment
christianlupus
left a comment
There was a problem hiding this comment.
I am considering if it would be more clear to set the saving flag inside the save function. Also, we could reset it on the done()/fail() functions in order to have a clear idea if an operation is in progress at the moment.
One case I am thinking a bit but that might be off: What happens on a slow network connection when a user clicks save but does not wait for at least the termination of the HTTP request. As the saving flag allows to reload/navigate to another URL, the changes might get lost if the user is too fast. Or am I getting things here wrong?
| return window.confirm('You have unsaved changes! Do you still want to leave?') | ||
| }, | ||
| confirmStayInEditedForm() { | ||
| return !this.savingRecipe && this.formDirty && !this.confirmLeavingPage() |
There was a problem hiding this comment.
This will continue to new URL when a saving action is just running, right?
There was a problem hiding this comment.
The idea was: If the user clicked the save button (saving is running), the browser continues to the new URL without checking if the form is dirty (this.formDirty) and (if it is dirty) asking for confirmation (!this.confirmLeavingPage())
There was a problem hiding this comment.
Yes, I see that. This is more related to the overall comment below about the abort of a saving action.
src/components/RecipeEdit.vue
Outdated
| } | ||
| }, | ||
| setup: function() { | ||
| let $this = this |
There was a problem hiding this comment.
May I ask, mainly out of curiosity, why we need a $this variable when we have the this one?
There was a problem hiding this comment.
Good spot :o I introduced it for the $nextTick callback. But actually it’s not necessary since $nextTick is invoked in the this context
Signed-off-by: Sebastian Fey <[email protected]>
Actually I’m not sure about this. I tried to provoke this problem but wasn’t successful. |
|
I tried to provoke this behavior as well but was neither successful. Either one is online and the save is possible or one is offline and one cannot navigate elsewhere. So it should be save to merge this, |
2 participants
When changes in the recipe form are done by the user, the form is marked dirty. When navigating away from a dirty form, the user is prompted for confirmation.
Closes #310
@sam-19 If you‘d like to share your opinion, you‘re welcome!