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

Itinerary Summary Overlay #1058

Merged
merged 29 commits into from
Jan 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
41e8b73
initial work on new itinerary summary overlay
miles-grant-ibigroup Oct 30, 2023
2d0f298
attempt at itinerary short renderer
miles-grant-ibigroup Nov 1, 2023
4fc120e
Merge branch 'dev' into itin-summary-overlay
miles-grant-ibigroup Nov 1, 2023
9cdf883
attempt at using metro itin summary
miles-grant-ibigroup Nov 1, 2023
4430731
improve itinerary summary overlay spacing
miles-grant-ibigroup Nov 2, 2023
c61f635
Merge branch 'dev' into itin-summary-overlay
miles-grant-ibigroup Nov 3, 2023
164b365
Merge branch 'dev' into itin-summary-overlay
miles-grant-ibigroup Dec 1, 2023
80c8f63
Merge branch 'dev' into itin-summary-overlay
miles-grant-ibigroup Dec 5, 2023
8caf338
clean up
miles-grant-ibigroup Dec 7, 2023
7cd9754
use fancy new math to calculate best point
daniel-heppner-ibigroup Dec 8, 2023
290a5c1
new algorithm to place flags: find furthest distance from all other c…
miles-grant-ibigroup Dec 13, 2023
23d88b5
add comments
miles-grant-ibigroup Dec 13, 2023
06114c6
allow clicking itin flags
miles-grant-ibigroup Dec 13, 2023
6034b3c
Merge branch 'dev' into itin-summary-overlay
miles-grant-ibigroup Dec 26, 2023
bbfa7ef
itinerary preview overlay: complete configurability, cleanup
miles-grant-ibigroup Dec 26, 2023
862fc67
add hover timeout
miles-grant-ibigroup Dec 26, 2023
2a66e3f
correct `previewOverlay` config item
miles-grant-ibigroup Dec 26, 2023
da52973
fix types
daniel-heppner-ibigroup Dec 26, 2023
1e43c6b
use reducer for unique point algo
daniel-heppner-ibigroup Dec 26, 2023
608e15b
simplify logic
daniel-heppner-ibigroup Dec 26, 2023
cda7285
Merge branch 'dev' into itin-summary-overlay
miles-grant-ibigroup Dec 27, 2023
429accd
test: update snapshots
miles-grant-ibigroup Dec 29, 2023
26d2e47
address pr feedback
miles-grant-ibigroup Jan 2, 2024
5e39d1c
update snapshots
miles-grant-ibigroup Jan 2, 2024
8b96c1a
Merge branch 'dev' into itin-summary-overlay
miles-grant-ibigroup Jan 5, 2024
2ecd33c
fix merged itinerary bug
miles-grant-ibigroup Jan 5, 2024
37d5c1e
address pr feedback
miles-grant-ibigroup Jan 5, 2024
aa3c220
Merge branch 'dev' into itin-summary-overlay
miles-grant-ibigroup Jan 5, 2024
bbf4efa
Merge branch 'dev' into itin-summary-overlay
miles-grant-ibigroup Jan 11, 2024
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
10 changes: 5 additions & 5 deletions __tests__/components/viewers/__snapshots__/stop-viewer.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -1437,7 +1437,7 @@ exports[`components > viewers > stop viewer should render countdown times after
title="20"
>
<span
className="sc-edoYdd cUdUnS"
className="sc-edoYdd ckpxJS"
color="333333"
style={
Object {
Expand Down Expand Up @@ -3691,7 +3691,7 @@ exports[`components > viewers > stop viewer should render countdown times for st
title="20"
>
<span
className="sc-edoYdd cUdUnS"
className="sc-edoYdd ckpxJS"
color="333333"
style={
Object {
Expand Down Expand Up @@ -5846,7 +5846,7 @@ exports[`components > viewers > stop viewer should render times after midnight w
title="20"
>
<span
className="sc-edoYdd cUdUnS"
className="sc-edoYdd ckpxJS"
color="333333"
style={
Object {
Expand Down Expand Up @@ -9169,7 +9169,7 @@ exports[`components > viewers > stop viewer should render with OTP transit index
title="36"
>
<span
className="sc-edoYdd cUdUnS"
className="sc-edoYdd ckpxJS"
color="333333"
style={
Object {
Expand Down Expand Up @@ -13547,7 +13547,7 @@ exports[`components > viewers > stop viewer should render with TriMet transit in
title="20"
>
<span
className="sc-edoYdd cUdUnS"
className="sc-edoYdd ckpxJS"
color="333333"
style={
Object {
Expand Down
2 changes: 2 additions & 0 deletions example-config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,8 @@ itinerary:
showScheduleDeviation: true
# Shows the duration of a leg below the leg in the metro itinerary summary
showLegDurations: false
# Whether to show (experimental) itinerary preview overlay on itinerary results on map
previewOverlay: false
# Whether to add a OTP_RR_A11Y_ROUTING_ENABLED error to all itineraries with accessibility scores
displayA11yError: false
# The sort option to use by default
Expand Down
2 changes: 1 addition & 1 deletion lib/components/form/batch-styled.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const buttonTransitionCss = css`
`

export const boxShadowCss = css`
box-shadow: rgba(0, 0, 0, 0.1) 0 0 20px;
box-shadow: rgba(0, 0, 0, 0.15) 0 0 20px;
`

// TODO: this needs to be in line with the mode selector buttons, ideally importing the styles
Expand Down
2 changes: 2 additions & 0 deletions lib/components/map/default-map.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { updateOverlayVisibility } from '../../actions/config'
import ElevationPointMarker from './elevation-point-marker'
import EndpointsOverlay from './connected-endpoints-overlay'
import GeoJsonLayer from './connected-geojson-layer'
import ItinSummaryOverlay from './itinerary-summary-overlay'
import ParkAndRideOverlay from './connected-park-and-ride-overlay'
import PointPopup from './point-popup'
import RoutePreviewOverlay from './route-preview-overlay'
Expand Down Expand Up @@ -304,6 +305,7 @@ class DefaultMap extends Component {
zoom={zoom}
>
<PointPopup />
<ItinSummaryOverlay />
<RoutePreviewOverlay />
{/* The default overlays */}
<EndpointsOverlay />
Expand Down
241 changes: 241 additions & 0 deletions lib/components/map/itinerary-summary-overlay.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,241 @@
import { connect } from 'react-redux'
import { Feature, lineString, LineString, Position } from '@turf/helpers'
import { Itinerary, Location } from '@opentripplanner/types'
import { Marker } from 'react-map-gl'
import centroid from '@turf/centroid'
import distance from '@turf/distance'
import polyline from '@mapbox/polyline'
import React, { useContext, useState } from 'react'
import styled from 'styled-components'

import * as narriativeActions from '../../actions/narrative'
import { AppReduxState } from '../../util/state-types'
import { boxShadowCss } from '../form/batch-styled'
import { ComponentContext } from '../../util/contexts'
import { doMergeItineraries } from '../narrative/narrative-itineraries'
import {
getActiveItinerary,
getActiveSearch,
getVisibleItineraryIndex
} from '../../util/state'
import MetroItineraryRoutes from '../narrative/metro/metro-itinerary-routes'

type ItinWithGeometry = Itinerary & {
allLegGeometry: Feature<LineString>
allStartTimes?: Itinerary[]
index?: number
}

type Props = {
from: Location
itins: Itinerary[]
setActiveItinerary: ({ index }: { index: number | null | undefined }) => void
setVisibleItinerary: ({ index }: { index: number | null | undefined }) => void
to: Location
visible?: boolean
visibleItinerary?: number
}

const Card = styled.div`
${boxShadowCss}
miles-grant-ibigroup marked this conversation as resolved.
Show resolved Hide resolved
miles-grant-ibigroup marked this conversation as resolved.
Show resolved Hide resolved

background: #fffffffa;
border-radius: 5px;
padding: 6px;
align-items: center;
display: flex;
flex-wrap: wrap;


span {
span {
span {
max-height: 28px;
min-height: 20px;
}
}
}
div {
margin-top: -0px!important;
}
.route-block-wrapper span {
padding: 0px;
}
* {
height: 26px;
}
}
`

function addItinLineString(itin: Itinerary): ItinWithGeometry {
return {
...itin,
allLegGeometry: lineString(
itin.legs.flatMap((leg) => polyline.decode(leg.legGeometry.points))
)
}
}
function addTrueIndex(array: ItinWithGeometry[]): ItinWithGeometry[] {
for (let i = 0; i < array.length; i++) {
const prevIndex = array?.[i - 1]?.index
const itin = array[i]
const nextIndex = itin?.allStartTimes?.length ?? 1
array[i] = {
...itin,
index: (prevIndex ?? -1) + nextIndex
}
}
return array
}

type ItinUniquePoint = {
itin: ItinWithGeometry
uniquePoint: Position
}

function getUniquePoint(
miles-grant-ibigroup marked this conversation as resolved.
Show resolved Hide resolved
thisItin: ItinWithGeometry,
otherPoints: ItinUniquePoint[]
): ItinUniquePoint {
const otherMidpoints = otherPoints.map((mp) => mp.uniquePoint)
let maxDistance = -Infinity
const line = thisItin.allLegGeometry
const centerOfLine = centroid(line).geometry.coordinates
let uniquePoint = centerOfLine

line.geometry.coordinates.forEach((point) => {
const totalDistance = otherMidpoints.reduce(
(prev, cur) => (prev += distance(point, cur)),
0
)

const selfDistance = distance(point, centerOfLine)
// maximize distance from all other points while minimizing distance to center of our own line
const averageDistance = totalDistance / otherMidpoints.length - selfDistance

if (averageDistance > maxDistance) {
maxDistance = averageDistance
uniquePoint = point
}
})
return { itin: thisItin, uniquePoint }
}

const ItinerarySummaryOverlay = ({
itins,
setActiveItinerary: setActive,
setVisibleItinerary: setVisible,
visible,
visibleItinerary
}: Props) => {
// @ts-expect-error React context is populated dynamically
const { LegIcon } = useContext(ComponentContext)

const [sharedTimeout, setSharedTimeout] = useState<null | NodeJS.Timeout>(
null
)

if (!itins || !visible) return <></>
const mergedItins: ItinWithGeometry[] = addTrueIndex(
doMergeItineraries(itins).mergedItineraries.map(addItinLineString)
)

const midPoints = mergedItins.reduce<ItinUniquePoint[]>((prev, curItin) => {
prev.push(getUniquePoint(curItin, prev))
return prev
}, [])
// The first point is probably not well placed, so let's run the algorithm again
if (midPoints.length > 1) {
midPoints[0] = getUniquePoint(mergedItins[0], midPoints)
}

try {
return (
<>
{midPoints.map(
(mp) =>
// If no itinerary is hovered, show all of them. If one is selected, show only that one
// TODO: clean up conditionals, move these to a more appropriate place without breaking indexing
(visibleItinerary !== null && visibleItinerary !== undefined
? visibleItinerary === mp.itin.index
: true) &&
mp.uniquePoint && (
<Marker
key={mp.itin.duration}
latitude={mp.uniquePoint[0]}
longitude={mp.uniquePoint[1]}
style={{ cursor: 'pointer' }}
>
<Card
onClick={() => {
setActive({ index: mp.itin.index })
}}
// TODO: useCallback here (getting weird errors?)
onMouseEnter={() => {
setSharedTimeout(
setTimeout(() => {
setVisible({ index: mp.itin.index })
}, 150)
)
}}
onMouseLeave={() => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

One behavior that can be annoying: if you hover this card and it causes the map to recenter and the cursor no longer on the card, the itinerary preview will flicker.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah this was annoying... We tried to fix this with the debounce, but I'm not sure how successful it was

Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't we decide we wanted to remove the map re-centering behavior from this overlay? It's still pretty jarring to try to hover one of these itinerary cards on the map.

sharedTimeout && clearTimeout(sharedTimeout)
setVisible({ index: null })
}}
>
<MetroItineraryRoutes
expanded={false}
itinerary={mp.itin}
LegIcon={LegIcon}
/>
</Card>
</Marker>
)
)}
</>
)
} catch (error) {
console.warn(`Can't create geojson from route: ${error}`)
return <></>
}
}

const mapStateToProps = (state: AppReduxState) => {
const { activeSearchId, config } = state.otp
if (config.itinerary?.previewOverlay !== true) {
return {}
}
if (!activeSearchId) return {}

const visibleItinerary = getVisibleItineraryIndex(state)
const activeItinerary = getActiveItinerary(state)

const activeSearch = getActiveSearch(state)
// @ts-expect-error state is not typed
const itins = activeSearch?.response.flatMap(
(serverResponse: { plan?: { itineraries?: Itinerary[] } }) =>
serverResponse?.plan?.itineraries
)

// @ts-expect-error state is not typed
const query = activeSearch ? activeSearch?.query : state.otp.currentQuery
const { from, to } = query

return {
from,
itins,
to,
visible: activeItinerary === undefined || activeItinerary === null,
visibleItinerary
}
}

const mapDispatchToProps = {
setActiveItinerary: narriativeActions.setActiveItinerary,
setVisibleItinerary: narriativeActions.setVisibleItinerary
}

export default connect(
mapStateToProps,
mapDispatchToProps
)(ItinerarySummaryOverlay)
2 changes: 2 additions & 0 deletions lib/components/narrative/metro/default-route-renderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ const Block = styled.span<{ color: string; isOnColoredBackground?: boolean }>`
display: inline-block;
margin-top: -2px;
padding: 3px 7px;
padding-left: 7px !important; /* TODO: this does not scale well to alternate zoom levels/text sizes */
padding-right: 7px !important;
/* Below is for route names that are too long: cut-off and show ellipsis. */
max-width: 150px;
overflow: hidden;
Expand Down
2 changes: 1 addition & 1 deletion lib/components/narrative/narrative-itineraries.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ function makeStartTime(itinerary) {
}
}

const doMergeItineraries = memoize((itineraries) => {
export const doMergeItineraries = memoize((itineraries) => {
const mergedItineraries = itineraries
.reduce((prev, cur) => {
const updatedItineraries = clone(prev)
Expand Down
1 change: 1 addition & 0 deletions lib/util/config-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ export interface ItineraryConfig {
mergeItineraries?: boolean
mutedErrors?: string[]
onlyShowCountdownForRealtime?: boolean
previewOverlay?: boolean
renderRouteNamesInBlocks?: boolean
showFirstResultByDefault?: boolean
showHeaderText?: boolean
Expand Down
3 changes: 2 additions & 1 deletion lib/util/state-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@ import {
import { AppConfig } from './config-types'

export interface OtpState {
// TODO: Add other OTP states
activeSearchId?: string
config: AppConfig
filter: {
sort: {
type: string
}
}
// TODO: Add other OTP states
ui: any // TODO
}

Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@
"@opentripplanner/vehicle-rental-overlay": "^2.1.3",
"@styled-icons/fa-regular": "^10.34.0",
"@styled-icons/fa-solid": "^10.34.0",
"@turf/centroid": "^6.5.0",
"@turf/helpers": "^6.5.0",
"blob-stream": "^0.1.3",
"bootstrap": "^3.3.7",
"bowser": "^1.9.3",
Expand Down
8 changes: 8 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3326,6 +3326,14 @@
"@turf/helpers" "^6.5.0"
"@turf/invariant" "^6.5.0"

"@turf/centroid@^6.5.0":
version "6.5.0"
resolved "https://registry.yarnpkg.com/@turf/centroid/-/centroid-6.5.0.tgz#ecaa365412e5a4d595bb448e7dcdacfb49eb0009"
integrity sha512-MwE1oq5E3isewPprEClbfU5pXljIK/GUOMbn22UM3IFPDJX0KeoyLNwghszkdmFp/qMGL/M13MMWvU+GNLXP/A==
dependencies:
"@turf/helpers" "^6.5.0"
"@turf/meta" "^6.5.0"

"@turf/circle@^6.5.0":
version "6.5.0"
resolved "https://registry.yarnpkg.com/@turf/circle/-/circle-6.5.0.tgz#dc017d8c0131d1d212b7c06f76510c22bbeb093c"
Expand Down
Loading