-
Notifications
You must be signed in to change notification settings - Fork 17
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
[IN-57][Reviews] Do not show warning on decision submit #112
[IN-57][Reviews] Do not show warning on decision submit #112
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me
@@ -124,6 +124,7 @@ export default Controller.extend({ | |||
}, | |||
submitDecision(trigger, comment, filter) { | |||
this.toggleProperty('savingAction'); | |||
this.set('userHasEnteredReview', false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ember-onbeforeunload
has a function that you can call in the router called shouldCheckIsPageDirty
, where you can exclude certain routes from the isPageDirty
check. In normal preprints, we have the submit route/button exempt from the warning modal on click.
Would it be a better practice to use shouldCheckIsPageDirty
here too instead of changing userHasEnteredReview
in the controller?
@fabmiz See Baylee's comment. Worth exploring as an option, kicking back to AD while you investigate. |
244cc4b
to
d073763
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Nice way to use the shouldCheckIsPageDirty
function!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🥇
Purpose
This is a follow-up PR for #108
Fix issue where the user gets a warning modal even after they submit
the decision form.
Summary of Changes
userHasEnteredReview
tofalse
onsubmitDecision
action, hence preventingthe
showWarning
to be set totrue
inwillTransition
action.Side Effects / Testing Notes
On the preprint detail page:
stay
orleave
and see if they behave as expected.Ticket
https://openscience.atlassian.net/browse/IN-57
Reviewer Checklist
CHANGELOG.md