Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
5a25d9c
improvement(TripBasicsPane): Scroll to trip name/days on submit if th…
binh-dam-ibigroup Feb 2, 2021
c6cf7dc
refactor(TripNotificationsPane): Update styles.
binh-dam-ibigroup Feb 2, 2021
41084cd
refactor(TripNotificationsPane): Improve semantics.
binh-dam-ibigroup Feb 2, 2021
0acd10d
fix(TripNotificationsPane): Add toggle for advanced trip settings.
binh-dam-ibigroup Feb 2, 2021
a990abc
fix(TripNotificationsPane): Add aria props to advanced settings toggle.
binh-dam-ibigroup Feb 2, 2021
337bb96
refactor(AccountPage): Remove unused prop.
binh-dam-ibigroup Feb 3, 2021
f7efc70
refactor(BackLink): Extract back button link.
binh-dam-ibigroup Feb 3, 2021
6c40335
refactor(BackLink): Move markup into BackLink
binh-dam-ibigroup Feb 3, 2021
77affe0
refactor(BackLink): Replace back to planner with back link in trip ed…
binh-dam-ibigroup Feb 3, 2021
42a08c1
refactor(StackedPaneDisplay): Experiment with Save button on top.
binh-dam-ibigroup Feb 3, 2021
8dc5971
Revert "refactor(StackedPaneDisplay): Experiment with Save button on …
binh-dam-ibigroup Feb 5, 2021
4801a00
refactor(Trip panes): Address PR comments
binh-dam-ibigroup Feb 5, 2021
b30a2ef
refactor(TripNotificationsPane): Target departure or arrival variance…
binh-dam-ibigroup Feb 22, 2021
b2baccf
refactor(TripNotificationsPane): Flush selectors to right margin. Twe…
binh-dam-ibigroup Feb 22, 2021
af09bb5
Merge branch 'dev' into improve-trip-name-validation
binh-dam-ibigroup Feb 22, 2021
2dd1821
refactor(TripNotificationPane): Use delayThreshold to manage departur…
binh-dam-ibigroup Mar 1, 2021
79f1545
Merge branch 'dev' into improve-trip-name-validation
binh-dam-ibigroup Mar 1, 2021
814d9df
refactor: Address PR comments
binh-dam-ibigroup Mar 4, 2021
2487512
refactor(TripNotificationsPane): Move common delayThreshold logic to …
binh-dam-ibigroup Mar 4, 2021
ec90a40
refactor(TripNotificationPane): Add comments, rename vars.
binh-dam-ibigroup Mar 4, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions lib/components/user/account-page.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,7 @@ const mapStateToProps = (state, ownProps) => {
return {
isTermsOrVerifyPage:
currentPath === CREATE_ACCOUNT_TERMS_PATH || currentPath === CREATE_ACCOUNT_VERIFY_PATH,
loggedInUser: state.user.loggedInUser,
trips: state.user.loggedInUserMonitoredTrips
loggedInUser: state.user.loggedInUser
}
}

Expand Down
2 changes: 2 additions & 0 deletions lib/components/user/existing-account-display.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react'

import BackToTripPlanner from './back-to-trip-planner'
import DeleteUser from './delete-user'
import NotificationPrefsPane from './notification-prefs-pane'
import FavoritePlacesList from './places/favorite-places-list'
Expand Down Expand Up @@ -38,6 +39,7 @@ const ExistingAccountDisplay = props => {
]
return (
<div>
<BackToTripPlanner />
<StackedPaneDisplay
onCancel={onCancel}
paneSequence={paneSequence}
Expand Down
2 changes: 2 additions & 0 deletions lib/components/user/monitored-trip/saved-trip-editor.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react'

import BackLink from '../back-link'
import StackedPaneDisplay from '../stacked-pane-display'

/**
Expand Down Expand Up @@ -32,6 +33,7 @@ const SavedTripEditor = props => {

return (
<div>
<BackLink />
<StackedPaneDisplay
onCancel={onCancel}
paneSequence={paneSequence}
Expand Down
8 changes: 5 additions & 3 deletions lib/components/user/monitored-trip/saved-trip-screen.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ class SavedTripScreen extends Component {
const { itinerary, loggedInUser, queryParams } = this.props
return {
...arrayToDayFields(WEEKDAYS),
arrivalVarianceMinutesThreshold: 5,
departureVarianceMinutesThreshold: 5,
excludeFederalHolidays: true,
isActive: true,
itinerary,
Expand All @@ -80,7 +82,7 @@ class SavedTripScreen extends Component {
* @param {*} monitoredTrip The trip edited state to be saved, provided by Formik.
*/
_updateMonitoredTrip = monitoredTrip => {
const { isCreating, createOrUpdateUserMonitoredTrip } = this.props
const { createOrUpdateUserMonitoredTrip, isCreating } = this.props
createOrUpdateUserMonitoredTrip(monitoredTrip, isCreating)
}

Expand Down Expand Up @@ -146,11 +148,11 @@ class SavedTripScreen extends Component {

screenContents = (
<Formik
// Avoid validating on change as it is annoying. Validating on blur is enough.
enableReinitialize
initialValues={clone(monitoredTrip)}
initialValues={monitoredTrip}
onSubmit={this._updateMonitoredTrip}
validateOnBlur
// Avoid validating on change as it is annoying. Validating on blur is enough.
validateOnChange={false}
validationSchema={validationSchema}
>
Expand Down
37 changes: 24 additions & 13 deletions lib/components/user/monitored-trip/trip-basics-pane.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Field } from 'formik'
import FormikErrorFocus from 'formik-error-focus'
import React, { Component } from 'react'
import {
ControlLabel,
Expand Down Expand Up @@ -52,26 +53,33 @@ const allDays = [
* and lets the user edit the trip name and day.
*/
class TripBasicsPane extends Component {
/**
* For new trips only, update the Formik state to
* uncheck days for which the itinerary is not available.
*/
_updateNewTripItineraryExistence = prevProps => {
const { isCreating, itineraryExistence, setFieldValue } = this.props

if (isCreating &&
itineraryExistence &&
itineraryExistence !== prevProps.itineraryExistence
) {
allDays.forEach(({ name }) => {
if (!itineraryExistence[name].valid) {
setFieldValue(name, false)
}
})
}
}

componentDidMount () {
// Check itinerary availability (existence) for all days.
const { checkItineraryExistence, values: monitoredTrip } = this.props
checkItineraryExistence(monitoredTrip)
}

componentDidUpdate (prevProps) {
const { isCreating, itineraryExistence, setFieldValue } = this.props

if (itineraryExistence !== prevProps.itineraryExistence) {
// For new trips only,
// update the Formik state to uncheck days for which the itinerary is not available.
if (isCreating && itineraryExistence) {
allDays.forEach(({ name }) => {
if (!itineraryExistence[name].valid) {
setFieldValue(name, false)
}
})
}
}
this._updateNewTripItineraryExistence(prevProps)
}

componentWillUnmount () {
Expand Down Expand Up @@ -145,6 +153,9 @@ class TripBasicsPane extends Component {
}
</HelpBlock>
{monitoredDaysValidationState && <HelpBlock>Please select at least one day to monitor.</HelpBlock>}

{/* Scroll to the trip name/days fields if submitting and there is an error on these fields. */}
<FormikErrorFocus align='middle' duration={200} />
</FormGroup>
</div>
)
Expand Down
219 changes: 150 additions & 69 deletions lib/components/user/monitored-trip/trip-notifications-pane.js
Original file line number Diff line number Diff line change
@@ -1,32 +1,107 @@
import { Field } from 'formik'
import React, { Component } from 'react'
import { Alert, Checkbox, ControlLabel, FormControl, FormGroup, Glyphicon, HelpBlock, Radio } from 'react-bootstrap'
import { Alert, FormControl, Glyphicon } from 'react-bootstrap'
import styled from 'styled-components'

import Icon from '../../narrative/icon'

const notificationChannelLabels = {
email: 'email',
sms: 'SMS'
}

// Element styles
const SettingsList = styled.ul`
border-spacing: 0 10px;
display: table;
padding-left: 0;
width: 100%;
label {
font-weight: inherit;
}
& > li {
align-items: center;
display: block;
}
`

// Using table display for this element, so that all dropdowns occupy the same width.
// (Bootstrap already sets them to occupy 100% of the width of the parent, i.e. the logical cell.)
const SettingsListWithAlign = styled(SettingsList)`
& > li {
display: table-row;
& > * {
display: table-cell;
}
& > label {
padding-right: 10px;
}
}
`

const InlineFormControl = styled(FormControl)`
display: inline-block;
margin: 0 0.5em;
width: auto;
`

const SettingsToggle = styled.button`
background: none;
border: none;
padding: 0;
& span.fa {
line-height: 150%;
}
`

/**
* A label followed by a dropdown control.
*/
const Select = ({ children, Control = FormControl, label, name }) => (
// <Field> is kept outside of <label> to accommodate layout in table/grid cells.
<>
<label htmlFor={name}>{label}</label>
<Field
as={Control}
componentClass='select'
id={name}
name={name}
>
{children}
</Field>
</>
)

/**
* This component wraps the elements to edit trip notification settings.
*/
class TripNotificationsPane extends Component {
/**
* Handles the conversion of 'true'/'false' strings in event data into boolean
* to update the Formik state.
*/
_handleIsActiveChange = e => {
const { handleChange } = this.props
handleChange({ target: {
name: 'isActive',
value: e.target.value === 'true'
}})
state = {
showAdvancedSettings: false
}

render () {
const { notificationChannel, values: monitoredTrip } = this.props
_handleToggleAdvancedSettings = () => {
this.setState({ showAdvancedSettings: !this.state.showAdvancedSettings })
}

_handleDelayThresholdChange = e => {
// To spare users the complexity of the departure/arrival delay thresholds,
// set both the arrival and departure variance delays to the selected value.
const { setFieldValue } = this.props
const threshold = e.target.value
setFieldValue('arrivalVarianceMinutesThreshold', threshold)
setFieldValue('departureVarianceMinutesThreshold', threshold)
}

// Set text and visuals based on user's notificationChannel.
render () {
const { notificationChannel, values } = this.props
const areNotificationsDisabled = notificationChannel === 'none'
const notificationChannelText = !areNotificationsDisabled &&
notificationChannelLabels[notificationChannel]
// Define a common trip delay field for simplicity, set to the smallest between the
// retrieved departure/arrival delay attributes.
const commonDelayThreshold = Math.min(
values.arrivalVarianceMinutesThreshold,
values.departureVarianceMinutesThreshold
)

let notificationSettingsContent
if (areNotificationsDisabled) {
Expand All @@ -42,66 +117,72 @@ class TripNotificationsPane extends Component {
</Alert>
)
} else {
const { showAdvancedSettings } = this.state
notificationSettingsContent = (
<>
<FormGroup>
<ControlLabel id='is-active-label'>Would you like to receive notifications about this trip?</ControlLabel>
<div role='group' aria-labelledby='is-active-label'>
<Radio
checked={monitoredTrip.isActive}
name='isActive'
onChange={this._handleIsActiveChange}
value
<h4>Notify me via {notificationChannelLabels[notificationChannel]} when:</h4>
<SettingsListWithAlign>
Copy link
Member

@landonreed landonreed Feb 4, 2021

Choose a reason for hiding this comment

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

After seeing this in action, let's make all these settings flush with the left edge (i.e., no indentation). Same goes for the below item in Advanced Settings.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated in 4801a00.

<li>
<Select
label='There is a realtime alert flagged on my journey'
name='notifyOnAlert'
>
Yes
</Radio>
<Radio
checked={!monitoredTrip.isActive}
name='isActive'
onChange={this._handleIsActiveChange}
value={false}
<option value>Yes (default)</option>
<option value={false}>No</option>
</Select>
</li>
<li>
<Select
label='An alternative route or transfer point is recommended'
name='notifyOnItineraryChange'
>
No
</Radio>
</div>
<HelpBlock>
Note: you will be notified by {notificationChannelText}. This can be changed in
your account settings once the trip has been saved.
</HelpBlock>
</FormGroup>

<FormGroup>
<ControlLabel>When would you like to receive notifications about delays or disruptions to your trip?</ControlLabel>

<FormGroup>
<ControlLabel>Check for delays or disruptions:</ControlLabel>
<Field
as={FormControl}
<option value>Yes (default)</option>
<option value={false}>No</option>
</Select>
</li>
<li>
<label htmlFor='commonDelayThreshold'>There are delays or disruptions of more than</label>
<FormControl
componentClass='select'
name='leadTimeInMinutes'
id='commonDelayThreshold'
onChange={this._handleDelayThresholdChange}
value={commonDelayThreshold}
>
<option value={15}>15 min. prior</option>
<option value={30}>30 min. prior (default)</option>
<option value={45}>45 min. prior</option>
<option value={60}>60 min. prior</option>
</Field>
</FormGroup>
<Alert bsStyle='warning'>
Under construction!
<FormGroup>
<ControlLabel>Notify me if:</ControlLabel>
<Checkbox>A different route or transfer point is recommended</Checkbox>
<Checkbox>There is an alert for a route or stop that is part of my journey</Checkbox>
<option value={5}>5 min (default)</option>
<option value={10}>10 min</option>
<option value={15}>15 min</option>
</FormControl>
</li>
</SettingsListWithAlign>

Your arrival or departure time changes by more than:
<FormControl componentClass='select' defaultValue={5} placeholder='select'>
<option value={5}>5 min. (default)</option>
<option value={10}>10 min.</option>
<option value={15}>15 min.</option>
</FormControl>
</FormGroup>
</Alert>
</FormGroup>
<h4>
<SettingsToggle
aria-expanded={showAdvancedSettings}
aria-label='Toggle advanced settings'
onClick={this._handleToggleAdvancedSettings}
type='button'
>
<Icon type={showAdvancedSettings ? 'caret-down' : 'caret-right'} />
Advanced settings
</SettingsToggle>
</h4>
{showAdvancedSettings && (
<SettingsList>
<li>
<Select
Control={InlineFormControl}
label='Monitor this trip'
name='leadTimeInMinutes'
>
<option value={15}>15 min</option>
<option value={30}>30 min (default)</option>
Copy link
Member

Choose a reason for hiding this comment

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

Because it's not the first option, 30 minutes is not selected by default. For this select and the others, we may want to add a selected prop? I'm not too familiar with how Formik handles <select/> inputs though, so I'm open to alternative suggestions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Forgot about that, glad you pointed it out.

Copy link
Collaborator Author

@binh-dam-ibigroup binh-dam-ibigroup Feb 5, 2021

Choose a reason for hiding this comment

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

That was trickier. The default value was already populated correctly in the trip object, but the I passed the wrong component for styling, so it's fixed now in 4801a00.

<option value={45}>45 min</option>
<option value={60}>1 hour</option>
</Select>
before it begins until it ends.
</li>
</SettingsList>
)}
</>
)
}
Expand Down
Loading