Merge pull request #91 from opentripplanner/stop-viewer-fixes

Stop viewer fixes

Author: landonreed

lib/actions/api.js

@@ -10,7 +10,7 @@ import { rememberPlace } from './map'
 import { hasCar } from '../util/itinerary'
 import { getTripOptionsFromQuery, getUrlParams } from '../util/query'
 import queryParams from '../util/query-params'
-import { queryIsValid } from '../util/state'
+import { getStopViewerConfig, queryIsValid } from '../util/state'
 import { randId } from '../util/storage'
 if (typeof (fetch) === 'undefined') require('isomorphic-fetch')
 
@@ -288,9 +288,6 @@ export function vehicleRentalQuery (params) {
 }
 
 // Single stop lookup query
-
-// Stop times for stop query
-// TODO: make timeRange and numberOfDepartures configurable
 const findStopResponse = createAction('FIND_STOP_RESPONSE')
 const findStopError = createAction('FIND_STOP_ERROR')
 
@@ -303,7 +300,7 @@ export function findStop (params) {
       serviceId: 'stops',
       postprocess: (payload, dispatch) => {
         dispatch(findRoutesAtStop(params.stopId))
-        dispatch(findStopTimesForStop({ stopId: params.stopId }))
+        dispatch(findStopTimesForStop(params))
       },
       noThrottle: true
     }
@@ -468,27 +465,32 @@ export function findGeometryForTrip (params) {
   )
 }
 
-// Stop times for stop query
-// TODO: make timeRange and numberOfDepartures configurable
-
 const findStopTimesForStopResponse = createAction('FIND_STOP_TIMES_FOR_STOP_RESPONSE')
 const findStopTimesForStopError = createAction('FIND_STOP_TIMES_FOR_STOP_ERROR')
 
+/**
+ * Stop times for stop query (used in stop viewer).
+ */
 export function findStopTimesForStop (params) {
-  return createQueryAction(
-    `index/stops/${params.stopId}/stoptimes?timeRange=345600&numberOfDepartures=5`,
-    findStopTimesForStopResponse,
-    findStopTimesForStopError,
-    {
-      rewritePayload: (payload) => {
-        return {
-          stopId: params.stopId,
-          stopTimes: payload
-        }
-      },
-      noThrottle: true
-    }
-  )
+  return function (dispatch, getState) {
+    let { stopId, ...otherParams } = params
+    // If other params not provided, fall back on defaults from stop viewer config.
+    const queryParams = { ...getStopViewerConfig(getState().otp), ...otherParams }
+    dispatch(createQueryAction(
+      `index/stops/${stopId}/stoptimes?${qs.stringify(queryParams)}`,
+      findStopTimesForStopResponse,
+      findStopTimesForStopError,
+      {
+        rewritePayload: (stopTimes) => {
+          return {
+            stopId,
+            stopTimes
+          }
+        },
+        noThrottle: true
+      }
+    ))
+  }
 }
 
 // Routes lookup query

lib/components/map/vehicle-rental-overlay.js

@@ -32,7 +32,9 @@ class VehicleRentalOverlay extends MapLayer {
   }
 
   componentDidMount () {
-    if (this.props.visible) this._startRefreshing()
+    const {companies, mapSymbols, name, visible} = this.props
+    if (visible) this._startRefreshing()
+    if (!mapSymbols) console.warn(`No map symbols provided for layer ${name}`, companies)
   }
 
   componentWillUnmount () {
@@ -214,8 +216,7 @@ class VehicleRentalOverlay extends MapLayer {
   }
 
   render () {
-    const { mapSymbols, stations, companies } = this.props
-    if (!mapSymbols) console.warn(`No map symbols provided for layer ${this.props.name}`, companies)
+    const { stations, companies } = this.props
     let filteredStations = stations
     if (companies) {
       filteredStations = stations.filter(

lib/components/viewers/stop-viewer.js

@@ -13,7 +13,7 @@ import { setMainPanelContent, toggleAutoRefresh } from '../../actions/ui'
 import { findStop, findStopTimesForStop } from '../../actions/api'
 import { forgetStop, rememberStop, setLocation } from '../../actions/map'
 import { routeComparator } from '../../util/itinerary'
-import { getShowUserSettings } from '../../util/state'
+import { getShowUserSettings, getStopViewerConfig } from '../../util/state'
 import { formatDuration, formatStopTime, getTimeFormat } from '../../util/time'
 
 class StopViewer extends Component {
@@ -116,6 +116,7 @@ class StopViewer extends Component {
       showUserSettings,
       stopData,
       stopViewerArriving,
+      stopViewerConfig,
       timeFormat
     } = this.props
     const { spin } = this.state
@@ -237,6 +238,7 @@ class StopViewer extends Component {
                         pattern={patternTimes.pattern}
                         route={patternTimes.route}
                         stopTimes={patternTimes.times}
+                        stopViewerConfig={stopViewerConfig}
                         key={patternTimes.id}
                         stopViewerArriving={stopViewerArriving}
                         homeTimezone={homeTimezone}
@@ -282,23 +284,28 @@ class PatternRow extends Component {
       stopTimes,
       homeTimezone,
       stopViewerArriving,
+      stopViewerConfig,
       timeFormat
     } = this.props
     // sort stop times by next departure
-    let sortedStopTimes = null
-    if (stopTimes) {
-      sortedStopTimes = stopTimes.sort((a, b) => {
-        const aTime = a.serviceDay + a.realtimeDeparture
-        const bTime = b.serviceDay + b.realtimeDeparture
-        return aTime - bTime
-      })
-      // Cap the number of times shown for any Route at 5. TODO: make configurable
-      if (sortedStopTimes.length > 0) sortedStopTimes = sortedStopTimes.slice(0, 5)
-      // Do not show any patterns with no departures happening soon.
-      const timeIsOverThreshold = sortedStopTimes[0].realtimeDeparture - getHomeTime(homeTimezone) > ONE_HOUR_IN_SECONDS * 3
-      if (sortedStopTimes[0] && timeIsOverThreshold) {
-        return null
-      }
+    let sortedStopTimes = []
+    const hasStopTimes = stopTimes && stopTimes.length > 0
+    if (hasStopTimes) {
+      sortedStopTimes = stopTimes
+        .concat()
+        .sort((a, b) => {
+          const aTime = a.serviceDay + a.realtimeDeparture
+          const bTime = b.serviceDay + b.realtimeDeparture
+          return aTime - bTime
+        })
+        // We request only x departures per pattern, but the patterns are merged
+        // according to shared headsigns, so we need to slice the stop times
+        // here as well to ensure only x times are shown per route/headsign combo.
+        // This is applied after the sort, so we're keeping the soonest departures.
+        .slice(0, stopViewerConfig.numberOfDepartures)
+    } else {
+      // Do not include pattern row if it has no stop times.
+      return null
     }
     const routeName = route.shortName ? route.shortName : route.longName
 
@@ -314,7 +321,7 @@ class PatternRow extends Component {
           </div>
 
           {/* next departure preview */}
-          {stopTimes && stopTimes.length > 0 && (
+          {hasStopTimes && (
             <div className='next-trip-preview'>
               {getFormattedStopTime(sortedStopTimes[0], homeTimezone, stopViewerArriving, timeFormat)}
             </div>
@@ -343,7 +350,7 @@ class PatternRow extends Component {
                 </div>
 
                 {/* list of upcoming trips */}
-                {stopTimes && (
+                {hasStopTimes && (
                   sortedStopTimes.map((stopTime, i) => {
                     return (
                       <div
@@ -485,6 +492,7 @@ function getStatusLabel (delay) {
 
 const mapStateToProps = (state, ownProps) => {
   const showUserSettings = getShowUserSettings(state.otp)
+  const stopViewerConfig = getStopViewerConfig(state.otp)
   return {
     autoRefreshStopTimes: state.otp.user.autoRefreshStopTimes,
     favoriteStops: state.otp.user.favoriteStops,
@@ -493,6 +501,7 @@ const mapStateToProps = (state, ownProps) => {
     showUserSettings,
     stopData: state.otp.transitIndex.stops[state.otp.ui.viewedStop.stopId],
     stopViewerArriving: state.otp.config.language.stopViewerArriving,
+    stopViewerConfig,
     timeFormat: getTimeFormat(state.otp.config)
   }
 }

lib/reducers/create-otp-reducer.js

@@ -20,7 +20,14 @@ const defaultConfig = {
   language: {},
   operators: [],
   realtimeEffectsDisplayThreshold: 120,
-  routingTypes: []
+  routingTypes: [],
+  stopViewer: {
+    numberOfDepartures: 3, // per pattern
+    // This is set to 345600 (four days) so that, for example, if it is Friday and
+    // a route does not begin service again until Monday, we are showing its next
+    // departure and it is not entirely excluded from display.
+    timeRange: 345600 // four days in seconds
+  }
 }
 
 // Load user settings from local storage.

lib/util/state.js

@@ -172,3 +172,7 @@ export function getRealtimeEffects (otpState) {
 export function getShowUserSettings (otpState) {
   return otpState.config.persistence && otpState.config.persistence.enabled
 }
+
+export function getStopViewerConfig (otpState) {
+  return otpState.config.stopViewer
+}