Trip page: Add warning about navigating away with unsaved changes #400
Conversation
| when={this.props.dirty && !this.props.isSubmitting && !this.props.canceled} | ||
| message={`You have haven't saved your ${this.props.isCreating ? 'new ' : ''}trip yet. If you leave, ${this.props.isCreating ? 'it' : 'changes'} will be lost.`} |
There was a problem hiding this comment.
Lines are too long. Please split up these long lines. A boolean variable could be created for the first one. The second one could also be calculated separately before returning this JSX. Also, please restructure these props at the very beginning of the render method.
There was a problem hiding this comment.
Thanks! I'm not entirely happy with the unsavedChangesMessage line break, but that's the only way I could think of that doesn't create a break in the popup.
| // but don't show it when they click submit or cancel. | ||
| const unsavedChanges = dirty && !isSubmitting && !canceled | ||
| // Message changes depending on if the new or existing trip is being edited | ||
| const unsavedChangesMessage = `You have haven't saved your ${ |
There was a problem hiding this comment.
| const unsavedChangesMessage = `You have haven't saved your ${ | |
| const unsavedChangesMessage = `You haven't saved your ${ |
There was a problem hiding this comment.
For internationalization, we should stay away from these string concats. I recommend we just display one string for when you create a trip and one other string for when you modify a trip.
There was a problem hiding this comment.
This looks great. Just one tweak for a typo. Also, I noticed an issue when clicking the <- Back button, where it updated the URL even if I press "Cancel" in the prompt, but that may be more related to #376
| // but don't show it when they click submit or cancel. | ||
| const unsavedChanges = dirty && !isSubmitting && !canceled | ||
| // Message changes depending on if the new or existing trip is being edited | ||
| const unsavedChangesMessage = `You have haven't saved your ${ |
There was a problem hiding this comment.
For internationalization, we should stay away from these string concats. I recommend we just display one string for when you create a trip and one other string for when you modify a trip.
|
|
||
| <FormNavigationButtons | ||
| backButton={{ | ||
| onClick: () => { updateBeingCanceled(true); onCancel() }, |
There was a problem hiding this comment.
What formatting do @landonreed and @evansiroky prefer regarding the semicolon? Spread over multiple lines?
There was a problem hiding this comment.
This was created by prettier -- ambiguities like this should be resolved in the future once we get the new version of mastarm up and running (which should include auto formatting).
There was a problem hiding this comment.
Yea, I was stuck on this a bit also, @binh-dam-ibigroup. Let's just go ahead and move to multiple lines, since we tend not to include semicolons anywhere in this code base.
…ed trip changes resolve typos, remove string assembly for internationalization
|
@landonreed have played around with this and I definitely think it is to do with #376. I can register the Should I try to address this in this pr or should we just expand #376? |
|
@miles-grant-ibigroup, can you document your findings in #376? We can handle the back button stuff altogether in that issue (so we can continue with the merge for this one). |
|
🎉 This PR is included in version 3.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
When a user is adding or editing a trip, they will now be prompted to confirm they really want to leave if they try to navigate away.
If they haven't made any changes or if they haven't made any changes, they aren't prompted.