Skip to content

Commit

Permalink
Merge pull request #618 from ibi-group/fix-pattern-stop-issue
Browse files Browse the repository at this point in the history
Handle uncaught error and improve add stop by name
  • Loading branch information
landonreed committed Oct 20, 2020
2 parents 3e1b15a + c0257d5 commit 0d2cb4a
Show file tree
Hide file tree
Showing 13 changed files with 358 additions and 124 deletions.
4 changes: 3 additions & 1 deletion lib/common/components/Loading.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ import Icon from '@conveyal/woonerf/components/icon'
import React, {Component} from 'react'
import { Row, Col } from 'react-bootstrap'

import type {Style} from '../../types'

type Props = {
inline?: boolean,
small?: boolean,
style?: {[string]: string | number}
style?: Style
}

export default class Loading extends Component<Props> {
Expand Down
4 changes: 3 additions & 1 deletion lib/editor/components/MinuteSecondInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@ import {
convertMMSSStringToSeconds
} from '../../common/util/date-time'

import type {Style} from '../../types'

type Props = {
disabled?: boolean,
onChange: number => void,
seconds: number,
style?: {[string]: string | number}
style?: Style
}

type State = {
Expand Down
4 changes: 2 additions & 2 deletions lib/editor/components/VirtualizedEntitySelect.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import VirtualizedSelect from 'react-virtualized-select'

import {getEntityName} from '../util/gtfs'

import type {Entity} from '../../types'
import type {Entity, Style} from '../../types'

export type EntityOption = {
entity: Entity,
Expand All @@ -20,7 +20,7 @@ type Props = {
entityKey: string,
onChange: any => void,
optionRenderer?: Function,
style?: {[string]: number | string},
style?: Style,
value?: any
}

Expand Down
41 changes: 8 additions & 33 deletions lib/editor/components/map/AddableStop.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
// @flow

import Icon from '@conveyal/woonerf/components/icon'
import { divIcon } from 'leaflet'
import React, {Component} from 'react'
import {Button, Dropdown, MenuItem} from 'react-bootstrap'
import {Marker, Popup} from 'react-leaflet'

import * as stopStrategiesActions from '../../actions/map/stopStrategies'
import AddPatternStopDropdown from '../pattern/AddPatternStopDropdown'

import type {GtfsStop, Pattern} from '../../../types'

Expand All @@ -28,6 +27,7 @@ export default class AddableStop extends Component<Props> {
render () {
const {
activePattern,
addStopToPattern,
stop
} = this.props
const color = 'blue'
Expand All @@ -40,44 +40,19 @@ export default class AddableStop extends Component<Props> {
className: '',
iconSize: [24, 24]
})
// TODO: Refactor to share code with PatternStopButtons
return (
<Marker
position={[stop.stop_lat, stop.stop_lon]}
icon={transparentBusIcon}>
<Popup>
<div style={{minWidth: '180px'}}>
<h5>{stopName}</h5>
<Dropdown
id={`add-stop-dropdown`}
pullRight
onSelect={this._onSelectStop}>
<Button
bsStyle='success'
onClick={this._onClickAddStopToEnd}>
<Icon type='plus' /> Add stop
</Button>
<Dropdown.Toggle
bsStyle='success' />
<Dropdown.Menu style={{maxHeight: '200px', overflowY: 'scroll'}}>
<MenuItem
value={activePattern.patternStops.length}
eventKey={activePattern.patternStops.length}>
Add to end (default)
</MenuItem>
{activePattern.patternStops && activePattern.patternStops.map((stop, i) => {
const index = activePattern.patternStops.length - i
return (
<MenuItem
value={index - 1}
eventKey={index - 1}
key={i}>
{index === 1 ? 'Add to beginning' : `Insert as stop #${index}`}
</MenuItem>
)
})}
</Dropdown.Menu>
</Dropdown>
<AddPatternStopDropdown
activePattern={activePattern}
addStopToPattern={addStopToPattern}
label='Add stop'
stop={stop}
/>
</div>
</Popup>
</Marker>
Expand Down
24 changes: 11 additions & 13 deletions lib/editor/components/map/pattern-debug-lines.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import {Polyline} from 'react-leaflet'
import lineDistance from 'turf-line-distance'
import lineString from 'turf-linestring'

import {POINT_TYPE} from '../../constants'
import {isValidPoint} from '../../util/map'
import {PATTERN_TO_STOP_DISTANCE_THRESHOLD_METERS} from '../../constants'
import {isValidStopControlPoint} from '../../util/map'

import type {ControlPoint, GtfsStop, Pattern} from '../../../types'
import type {EditSettingsState} from '../../../types/reducers'
Expand All @@ -20,8 +20,6 @@ type Props = {
stops: Array<GtfsStop>
}

const DISTANCE_THRESHOLD = 50

/**
* This react-leaflet component draws connecting lines between a pattern
* geometry's anchor points (that are associated with stops) and their
Expand All @@ -44,33 +42,33 @@ export default class PatternDebugLines extends PureComponent<Props> {
// i.e., whether the line should be rendered.
.map((cp, index) => ({...cp, cpIndex: index}))
// Filter out the user-added anchors
.filter(cp => cp.pointType === POINT_TYPE.STOP)
.filter(isValidStopControlPoint)
// The remaining number should match the number of stops
.map((cp, index) => {
const {cpIndex, point, stopId} = cp
if (!isValidPoint(point)) {
return null
}
// If hiding inactive segments (and this control point is not along
// a visible segment), do not show debug line.
if (editSettings.hideInactiveSegments && (cpIndex > patternSegment + 1 || cpIndex < patternSegment - 1)) {
return null
}
const patternStopIsActive = patternStop.index === index
// Do not render if some other pattern stop is active
// $FlowFixMe
if ((patternStop.index || patternStop.index === 0) && !patternStopIsActive) {
if (typeof patternStop.index === 'number' && !patternStopIsActive) {
return null
}
const {coordinates: cpCoord} = point.geometry
// Find stop entity for control point.
const stop = stops.find(s => s.stop_id === stopId)
if (!stop) {
// console.warn(`Could not find stop for pattern stop index=${index} patternStop#stopId=${stopId}`)
// If no stop entity found, do not attempt to draw a line to the
// missing stop.
return null
}
const coordinates = [[cpCoord[1], cpCoord[0]], [stop.stop_lat, stop.stop_lon]]
const distance: number = lineDistance(lineString(coordinates), 'meters')
const distanceGreaterThanThreshold = distance > DISTANCE_THRESHOLD
const distanceGreaterThanThreshold = distance > PATTERN_TO_STOP_DISTANCE_THRESHOLD_METERS
if (distanceGreaterThanThreshold) {
console.warn(`Distance from pattern stop index=${index} to projected point is greater than ${DISTANCE_THRESHOLD} (${distance}).`)
console.warn(`Distance from pattern stop index=${index} to projected point is greater than ${PATTERN_TO_STOP_DISTANCE_THRESHOLD_METERS} (${distance}).`)
}
return (
<Polyline
Expand Down
136 changes: 136 additions & 0 deletions lib/editor/components/pattern/AddPatternStopDropdown.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
// @flow

import Icon from '@conveyal/woonerf/components/icon'
import React, {Component} from 'react'
import { Button, Dropdown, MenuItem } from 'react-bootstrap'

import * as stopStrategiesActions from '../../actions/map/stopStrategies'

import type {GtfsStop, Pattern, PatternStop, Style} from '../../../types'

type Props = {
activePattern: Pattern,
addStopToPattern: typeof stopStrategiesActions.addStopToPattern,
index?: number, // current pattern stop index (if dropdown shown for current pattern stop)
label?: string,
patternStop?: PatternStop, // optional current pattern stop
size?: string,
stop: GtfsStop,
style?: Style
}

const DISABLED_MESSAGE = 'Cannot have the same stop appear consecutively in list'

/**
* Dropdown button that adds a stop as a new pattern stop to the actively
* selected pattern.
*/
export default class AddPatternStopDropdown extends Component<Props> {
_addStop = (index?: number) => {
const {activePattern, addStopToPattern, stop} = this.props
addStopToPattern(activePattern, stop, index)
}

_matchesStopAtIndex = (index: number) => {
const {activePattern, stop} = this.props
const patternStopAtIndex = activePattern.patternStops[index]
return patternStopAtIndex && patternStopAtIndex.stopId === stop.stop_id
}

_onAddToEnd = () => this._addStop()

_onSelectStop = (key: number) => this._addStop(key)

render () {
const {activePattern, index, label, size, style} = this.props
const {patternStops} = activePattern
const lastIndex = patternStops.length - 1
// Check that first/last stop is not already set to this stop.
let addToEndDisabled = this._matchesStopAtIndex(lastIndex)
let addToBeginningDisabled = this._matchesStopAtIndex(0)
// Also, disable end/beginning if the current pattern stop being viewed
// occupies one of these positions.
if (typeof index === 'number') {
addToEndDisabled = addToEndDisabled || index >= lastIndex
addToBeginningDisabled = addToBeginningDisabled || index === 0
}
return (
<Dropdown
id={`add-stop-dropdown`}
pullRight
onSelect={this._onSelectStop}
style={style}
>
<Button
bsSize={size}
bsStyle='success'
disabled={addToEndDisabled}
title={addToEndDisabled ? DISABLED_MESSAGE : ''}
onClick={this._onAddToEnd}>
<Icon type='plus' /> {label}
</Button>
<Dropdown.Toggle
bsSize={size}
bsStyle='success' />
<Dropdown.Menu style={{maxHeight: '200px', overflowY: 'scroll'}}>
<MenuItem
disabled={addToEndDisabled}
title={addToEndDisabled ? DISABLED_MESSAGE : ''}
value={activePattern.patternStops.length}
eventKey={activePattern.patternStops.length}>
Add to end (default)
</MenuItem>
{activePattern.patternStops && activePattern.patternStops.map((s, i) => {
// addIndex is in "reverse" order. For example:
// - 5 pattern stops
// - rendering MenuItem 'Insert at stop #4'
// - addIndex = 3 = 5 (stops) - 1 (i) - 1
// - nextIndex = 4
// - previousIndex = 2
const addIndex = activePattern.patternStops.length - i - 1
const nextIndex = addIndex + 1
const previousIndex = addIndex - 1
let disableAdjacent = false
// If showing the dropdown for the currently selected pattern stop,
// do not allow adding as an adjacent stop (a pattern should not
// visit the same stop consecutively).
if (typeof index === 'number') {
disableAdjacent = index > previousIndex && index < nextIndex
}
// Disable adding stop to current position or directly before
// current position. nextIndex is OK because inserting the stop at
// addIndex would move the current stop at addIndex into the
// nextIndex position, which would serve as a buffer (and avoid
// having the same stop in consecutive positions).
const addAtIndexDisabled = disableAdjacent ||
this._matchesStopAtIndex(previousIndex) ||
this._matchesStopAtIndex(addIndex)
// Skip MenuItem if index is the same as the currently selected
// pattern stop index or it's the zeroth addIndex (Add to
// beginning handles this case).
if (index === addIndex || addIndex === 0) {
return null
}
return (
<MenuItem
disabled={addAtIndexDisabled}
value={addIndex}
title={addAtIndexDisabled ? DISABLED_MESSAGE : ''}
key={i}
eventKey={addIndex}>
Insert as stop #{addIndex + 1}
</MenuItem>
)
})}
<MenuItem
disabled={addToBeginningDisabled}
title={addToBeginningDisabled ? DISABLED_MESSAGE : ''}
value={0}
eventKey={0}>
Add to beginning
</MenuItem>
</Dropdown.Menu>
</Dropdown>
)
}
}
Loading

0 comments on commit 0d2cb4a

Please sign in to comment.