From 864cf79cd7160ebe33c81b48be8b74cc323bbefb Mon Sep 17 00:00:00 2001 From: Evan Siroky Date: Thu, 18 Jul 2019 09:39:06 -0700 Subject: [PATCH 1/6] fix(narrative): properly render eScooter legs occurring after forced walking --- .../narrative/line-itin/place-row.js | 56 ++++++++++++------- lib/util/itinerary.js | 4 +- 2 files changed, 38 insertions(+), 22 deletions(-) diff --git a/lib/components/narrative/line-itin/place-row.js b/lib/components/narrative/line-itin/place-row.js index 6667d7990..5ae81d09f 100644 --- a/lib/components/narrative/line-itin/place-row.js +++ b/lib/components/narrative/line-itin/place-row.js @@ -156,6 +156,9 @@ class RentedVehicleLeg extends PureComponent { render () { const { config, leg } = this.props const configCompanies = config.companies || [] + + // Sometimes rented vehicles can be walked over things like stairs or other + // ways that forbid the main mode of travel. if (leg.mode === 'WALK') { return (
@@ -163,35 +166,48 @@ class RentedVehicleLeg extends PureComponent {
) } - if (leg.rentedVehicle || leg.rentedBike || leg.rentedCar) { - let pickUpString = 'Pick up' - if (leg.rentedBike) { - // TODO: Special case for TriMet may need to be refactored. - pickUpString += ` shared bike` - } else { - // Add company and vehicle labels. + + let pickUpString = 'Pick up' + if (leg.rentedBike) { + // TODO: Special case for TriMet may need to be refactored. + pickUpString += ` shared bike` + } else { + // Add company and vehicle labels. + let vehicleName = '' + // TODO allow more flexibility in customizing these mode strings + let modeString = leg.rentedVehicle + ? 'eScooter' + : leg.rentedBike + ? 'bike' + : 'car' + + // The networks attribute of the from data will only appear at the very + // beggining of the rental. It is possible that there will be some forced + // walking that occurs in the middle of the rental, so once the main mode + // resumes there won't be any network info. In that case we simply return + // that the rental is continuing. + if (leg.from.networks) { const companies = leg.from.networks.map(n => getCompanyForNetwork(n, configCompanies)) const companyLabel = companies.map(co => co.label).join('/') pickUpString += ` ${companyLabel}` - const modeString = getModeForPlace(leg.from) // Only show vehicle name for car rentals. For bikes and eScooters, these // IDs/names tend to be less relevant (or entirely useless) in this context. - const vehicleName = leg.rentedCar ? ` ${leg.from.name}` : '' - pickUpString += ` ${modeString}${vehicleName}` + if (leg.rentedCar && leg.from.name) { + vehicleName = leg.from.name + } + modeString = getModeForPlace(leg.from) + } else { + pickUpString = 'Continue using rental' } - // e.g., Pick up REACHNOW rented car XYZNDB OR - // Pick up SPIN eScooter - // Pick up shared bike - return ( -
- {pickUpString} -
- ) + + pickUpString += ` ${modeString}${vehicleName}` } - // FIXME: Under what conditions would this be returned? + // e.g., Pick up REACHNOW rented car XYZNDB OR + // Pick up SPIN eScooter + // Pick up shared bike return (
- Continue riding from {leg.from.name} + {pickUpString}
) } diff --git a/lib/util/itinerary.js b/lib/util/itinerary.js index 7befc25bd..412e1bfae 100644 --- a/lib/util/itinerary.js +++ b/lib/util/itinerary.js @@ -393,9 +393,9 @@ export function getLegIcon (leg, customIcons) { iconStr = leg.from.networks[0] } else if (iconStr === 'CAR' && leg.tncData) { iconStr = leg.tncData.company - } else if (iconStr === 'BICYCLE' && leg.rentedBike) { + } else if (iconStr === 'BICYCLE' && leg.rentedBike && leg.from.networks) { iconStr = leg.from.networks[0] - } else if (iconStr === 'MICROMOBILITY' && leg.rentedVehicle) { + } else if (iconStr === 'MICROMOBILITY' && leg.rentedVehicle && leg.from.networks) { iconStr = leg.from.networks[0] } From 908552571137532314400285f27e26d0d706ee36 Mon Sep 17 00:00:00 2001 From: Landon Reed Date: Thu, 18 Jul 2019 12:50:53 -0400 Subject: [PATCH 2/6] fix(react-router): handle any route w webapp + support otp.js legacy route --- lib/actions/ui.js | 16 ++++++++++++++-- lib/components/app/responsive-webapp.js | 5 +++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/actions/ui.js b/lib/actions/ui.js index 2f3ba5f2a..d12bd09a0 100644 --- a/lib/actions/ui.js +++ b/lib/actions/ui.js @@ -40,13 +40,14 @@ export function matchContentToUrl (location) { // This is a bit of a hack to make up for the fact that react-router does // not always provide the match params as expected. // https://github.com/ReactTraining/react-router/issues/5870#issuecomment-394194338 - const root = location.pathname.split('/')[1] + let root = location.pathname.split('/')[1] const match = matchPath(location.pathname, { path: `/${root}/:id`, exact: true, strict: false }) const id = match && match.params && match.params.id + console.log(location, id) switch (root) { case 'route': if (id) { @@ -65,9 +66,16 @@ export function matchContentToUrl (location) { dispatch(setMainPanelContent(MainPanelContent.STOP_VIEWER)) } break + case 'start': case '@': // Parse comma separated params (ensuring numbers are parsed correctly). - const [lat, lon, zoom, routerId] = id.split(',').map(s => isNaN(s) ? s : +s) + let [lat, lon, zoom, routerId] = id ? idToParams(id) : [] + if (!lat || !lon) { + // Attempt to parse path. (Legacy UI otp.js used slashes in the + // pathname to specify lat, lon, etc.) + [,, lat, lon, zoom, routerId] = idToParams(location.pathname, '/') + } + console.log('Setting start position/zoom/router', lat, lon, zoom, routerId) // Update map location/zoom and optionally override router ID. dispatch(setMapCenter({ lat, lon })) dispatch(setMapZoom({ zoom })) @@ -82,6 +90,10 @@ export function matchContentToUrl (location) { } } +function idToParams (id, delimiter = ',') { + return id.split(delimiter).map(s => isNaN(s) ? s : +s) +} + /** * Event listener for responsive webapp that handles a back button press and * sets the active search and itinerary according to the URL query params. diff --git a/lib/components/app/responsive-webapp.js b/lib/components/app/responsive-webapp.js index d55f862e1..f70787102 100644 --- a/lib/components/app/responsive-webapp.js +++ b/lib/components/app/responsive-webapp.js @@ -181,6 +181,7 @@ class RouterWrapper extends Component { // to a quirk with react-router. // https://github.com/ReactTraining/react-router/issues/5870#issuecomment-394194338 '/@/:latLonZoomRouter', + '/start/:latLonZoomRouter', // Route viewer (and route ID). '/route', '/route/:id', @@ -194,6 +195,10 @@ class RouterWrapper extends Component { path='/print' component={PrintLayout} /> + {/* For any other route, simply return the web app. */} + } + /> From 1e5481e9ff9c8ea73d07d001e378b1184dd05958 Mon Sep 17 00:00:00 2001 From: Landon Reed Date: Thu, 18 Jul 2019 12:55:03 -0400 Subject: [PATCH 3/6] refactor: remove log statement --- lib/actions/ui.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/actions/ui.js b/lib/actions/ui.js index d12bd09a0..b564591db 100644 --- a/lib/actions/ui.js +++ b/lib/actions/ui.js @@ -40,14 +40,13 @@ export function matchContentToUrl (location) { // This is a bit of a hack to make up for the fact that react-router does // not always provide the match params as expected. // https://github.com/ReactTraining/react-router/issues/5870#issuecomment-394194338 - let root = location.pathname.split('/')[1] + const root = location.pathname.split('/')[1] const match = matchPath(location.pathname, { path: `/${root}/:id`, exact: true, strict: false }) const id = match && match.params && match.params.id - console.log(location, id) switch (root) { case 'route': if (id) { From 3639fa8fcd5203a3161c06bdb1874531d811defc Mon Sep 17 00:00:00 2001 From: Evan Siroky Date: Thu, 18 Jul 2019 09:58:02 -0700 Subject: [PATCH 4/6] refactor(narrative): fix comment typo, rename a variable to be more meaningful --- lib/components/narrative/line-itin/place-row.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/components/narrative/line-itin/place-row.js b/lib/components/narrative/line-itin/place-row.js index 5ae81d09f..cd00495e1 100644 --- a/lib/components/narrative/line-itin/place-row.js +++ b/lib/components/narrative/line-itin/place-row.js @@ -167,10 +167,10 @@ class RentedVehicleLeg extends PureComponent { ) } - let pickUpString = 'Pick up' + let rentalDescription = 'Pick up' if (leg.rentedBike) { // TODO: Special case for TriMet may need to be refactored. - pickUpString += ` shared bike` + rentalDescription += ` shared bike` } else { // Add company and vehicle labels. let vehicleName = '' @@ -182,14 +182,14 @@ class RentedVehicleLeg extends PureComponent { : 'car' // The networks attribute of the from data will only appear at the very - // beggining of the rental. It is possible that there will be some forced + // beginning of the rental. It is possible that there will be some forced // walking that occurs in the middle of the rental, so once the main mode // resumes there won't be any network info. In that case we simply return // that the rental is continuing. if (leg.from.networks) { const companies = leg.from.networks.map(n => getCompanyForNetwork(n, configCompanies)) const companyLabel = companies.map(co => co.label).join('/') - pickUpString += ` ${companyLabel}` + rentalDescription += ` ${companyLabel}` // Only show vehicle name for car rentals. For bikes and eScooters, these // IDs/names tend to be less relevant (or entirely useless) in this context. if (leg.rentedCar && leg.from.name) { @@ -197,17 +197,17 @@ class RentedVehicleLeg extends PureComponent { } modeString = getModeForPlace(leg.from) } else { - pickUpString = 'Continue using rental' + rentalDescription = 'Continue using rental' } - pickUpString += ` ${modeString}${vehicleName}` + rentalDescription += ` ${modeString}${vehicleName}` } // e.g., Pick up REACHNOW rented car XYZNDB OR // Pick up SPIN eScooter // Pick up shared bike return (
- {pickUpString} + {rentalDescription}
) } From fe13bceedd1446edbaea7785724102905d0ba41c Mon Sep 17 00:00:00 2001 From: Landon Reed Date: Thu, 18 Jul 2019 13:01:23 -0400 Subject: [PATCH 5/6] chore: remove log statement --- lib/actions/ui.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/actions/ui.js b/lib/actions/ui.js index b564591db..ad04d2805 100644 --- a/lib/actions/ui.js +++ b/lib/actions/ui.js @@ -74,7 +74,6 @@ export function matchContentToUrl (location) { // pathname to specify lat, lon, etc.) [,, lat, lon, zoom, routerId] = idToParams(location.pathname, '/') } - console.log('Setting start position/zoom/router', lat, lon, zoom, routerId) // Update map location/zoom and optionally override router ID. dispatch(setMapCenter({ lat, lon })) dispatch(setMapZoom({ zoom })) From 8cc820eb412db6849361bbb87807219c3c82a6eb Mon Sep 17 00:00:00 2001 From: Landon Reed Date: Thu, 18 Jul 2019 13:46:45 -0400 Subject: [PATCH 6/6] fix(example): fix example app to work with new routing fix #72 --- example.js | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/example.js b/example.js index 9d648baea..56060cac7 100644 --- a/example.js +++ b/example.js @@ -1,7 +1,9 @@ // import necessary React/Redux libraries +import { createHashHistory } from 'history' +import { connectRouter, routerMiddleware } from 'connected-react-router' import React, { Component } from 'react' import { render } from 'react-dom' -import { createStore, combineReducers, applyMiddleware } from 'redux' +import { createStore, combineReducers, applyMiddleware, compose } from 'redux' import { Provider } from 'react-redux' import thunk from 'redux-thunk' import createLogger from 'redux-logger' @@ -38,19 +40,29 @@ const initialQuery = { type: 'ITINERARY' } +const history = createHashHistory() +const middleware = [ + thunk, + routerMiddleware(history) // for dispatching history actions +] + +// check if app is being run in development mode. If so, enable redux-logger +if (process.env.NODE_ENV === 'development') { + middleware.push(createLogger()) +} + // set up the Redux store const store = createStore( combineReducers({ - otp: createOtpReducer(otpConfig) // add optional initial query here - // add your own reducers if you want + otp: createOtpReducer(otpConfig), + router: connectRouter(history) }), - applyMiddleware(thunk, createLogger()) + compose(applyMiddleware(...middleware)) ) // define a simple responsive UI using Bootstrap and OTP-RR class OtpRRExample extends Component { render () { - /** desktop view **/ const desktopView = (