Do not close trip settings screen on error saving trip. #232
Conversation
… a trip (new/existing).
binh-dam-ibigroup
changed the title
| const savedTrip = this.props.monitoredTrips.find(trip => { | ||
| const tripQueryParams = qs.parse(trip.queryParams.split('?')[1]) | ||
| return tripQueryParams.ui_activeSearch === activeSearchId | ||
| }) |
There was a problem hiding this comment.
This approach seems flimsy. I would prefer that this logic be contained within the action -- that would remove all of this inference that is being applied here. I think the routing logic for these handlers (goToTripPlanner, goToSavedTrips, etc.) should also be contained within those actions.
landonreed
left a comment
landonreed
left a comment
There was a problem hiding this comment.
I think we ought to move this logic to the actions. That would remove the need for the somewhat flimsy checks and instead just check the result of the fetch.
There was a problem hiding this comment.
_handleSaveTrip does not need to exist (just use _updateMonitoredTrip). Also, _updateMonitoredTrip does not need to be async (no need to await createOrUpdateUserMonitoredTrip).
There was a problem hiding this comment.
Good catch. Updated in 3fd0a0a along with missing import in actions/user.
landonreed
left a comment
landonreed
left a comment
There was a problem hiding this comment.
Wow, that's a lot simpler. Nice work. Just one final condition for approval (see comment).
|
🎉 This PR is included in version 1.4.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
If there is an error while saving a new or existing trip, the trip settings screen will close after the user dismisses the error message. I find this annoying, and this PR prevents the trip settings screen from closing in such situations. In some cases, you want the user to be able to make changes and let them attempt to change again.
To test: