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

Trip page: Add warning about navigating away with unsaved changes #400

Merged
merged 9 commits into from
Jul 7, 2021
25 changes: 24 additions & 1 deletion lib/components/user/monitored-trip/trip-basics-pane.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@ import {
ProgressBar
} from 'react-bootstrap'
import { connect } from 'react-redux'
import { Prompt } from 'react-router'
import styled from 'styled-components'

import * as userActions from '../../../actions/user'
import { getErrorStates } from '../../../util/ui'

import TripStatus from './trip-status'
import TripSummary from './trip-summary'

Expand Down Expand Up @@ -87,9 +89,25 @@ class TripBasicsPane extends Component {
}

render () {
const { errors, isCreating, itineraryExistence, values: monitoredTrip } = this.props
const {
canceled,
dirty,
errors,
isCreating,
isSubmitting,
itineraryExistence,
values: monitoredTrip
} = this.props
const { itinerary } = monitoredTrip

// Prevent user from leaving when form has been changed,
// 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 ${
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const unsavedChangesMessage = `You have haven't saved your ${
const unsavedChangesMessage = `You haven't saved your ${

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

isCreating ? 'new ' : ''
}trip yet. If you leave, ${isCreating ? 'it' : 'changes'} will be lost.`

if (!itinerary) {
return <div>No itinerary to display.</div>
} else {
Expand All @@ -107,6 +125,11 @@ class TripBasicsPane extends Component {

return (
<div>
<Prompt
when={unsavedChanges}
message={unsavedChangesMessage}
/>

{/* Do not show trip status when saving trip for the first time
(it doesn't exist in backend yet). */}
{!isCreating && <TripStatus monitoredTrip={monitoredTrip} />}
Expand Down
52 changes: 28 additions & 24 deletions lib/components/user/stacked-pane-display.js
Original file line number Diff line number Diff line change
@@ -1,36 +1,40 @@
import PropTypes from 'prop-types'
import React from 'react'
import React, {useState} from 'react'

import FormNavigationButtons from './form-navigation-buttons'
import { PageHeading, StackedPaneContainer } from './styled'

/**
* This component handles the flow between screens for new OTP user accounts.
*/
const StackedPaneDisplay = ({ onCancel, paneSequence, title }) => (
<>
{title && <PageHeading>{title}</PageHeading>}
{
paneSequence.map(({ pane: Pane, props, title }, index) => (
<StackedPaneContainer key={index}>
<h3>{title}</h3>
<div><Pane {...props} /></div>
</StackedPaneContainer>
))
}
const StackedPaneDisplay = ({ onCancel, paneSequence, title }) => {
// Create indicator of if cancel button was clicked so that child components can know
const [isBeingCanceled, updateBeingCanceled] = useState(false)

<FormNavigationButtons
backButton={{
onClick: onCancel,
text: 'Cancel'
}}
okayButton={{
text: 'Save Preferences',
type: 'submit'
}}
/>
</>
)
return (
<>
{title && <PageHeading>{title}</PageHeading>}
{
paneSequence.map(({ pane: Pane, props, title }, index) => (
<StackedPaneContainer key={index}>
<h3>{title}</h3>
<div><Pane canceled={isBeingCanceled} {...props} /></div>
</StackedPaneContainer>
))
}

<FormNavigationButtons
backButton={{
onClick: () => { updateBeingCanceled(true); onCancel() },
Copy link
Collaborator

Choose a reason for hiding this comment

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

What formatting do @landonreed and @evansiroky prefer regarding the semicolon? Spread over multiple lines?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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).

Copy link
Member

Choose a reason for hiding this comment

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

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.

text: 'Cancel'
}}
okayButton={{
text: 'Save Preferences',
type: 'submit'
}}
/>
</>)
}

StackedPaneDisplay.propTypes = {
onCancel: PropTypes.func.isRequired,
Expand Down