From 87cc55f79aca73ea6a1a9d5e8cce3ffbfe9943fa Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 25 Aug 2023 16:12:04 -0400 Subject: [PATCH 1/7] refactor(route-details): Make light refactors to component. --- lib/components/viewers/route-details.js | 36 +++++++++++-------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/lib/components/viewers/route-details.js b/lib/components/viewers/route-details.js index b3d7099f3..faf32780e 100644 --- a/lib/components/viewers/route-details.js +++ b/lib/components/viewers/route-details.js @@ -3,10 +3,9 @@ import { connect } from 'react-redux' import { FormattedMessage, injectIntl } from 'react-intl' import { getMostReadableTextColor } from '@opentripplanner/core-utils/lib/route' - -import { LinkOpensNewWindow } from '../util/externalLink' import PropTypes from 'prop-types' import React, { Component } from 'react' +import styled from 'styled-components' import { extractHeadsignFromPattern, @@ -14,7 +13,10 @@ import { } from '../../util/viewer' import { findStopsForPattern } from '../../actions/api' import { getOperatorName } from '../../util/state' +import { LinkOpensNewWindow } from '../util/externalLink' import { setHoveredStop, setViewedRoute, setViewedStop } from '../../actions/ui' +import { SortResultsDropdown } from '../util/dropdown' +import { UnstyledButton } from '../util/unstyled-button' import { Container, @@ -26,10 +28,14 @@ import { StopContainer, StopLink } from './styled' -import { SortResultsDropdown } from '../util/dropdown' -import { UnstyledButton } from '../util/unstyled-button' -import styled from 'styled-components' +const PatternSelectButton = styled(UnstyledButton)` + span { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + } +` class RouteDetails extends Component { static propTypes = { @@ -80,24 +86,14 @@ class RouteDetails extends Component { const routeColor = getRouteColorBasedOnSettings(operator, route) - const PatternSelectButton = styled(UnstyledButton)` - span { - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; - } - ` - const headsigns = patterns && Object.entries(patterns) - .map((pattern) => { - return { - geometryLength: pattern[1].geometry?.length, - headsign: extractHeadsignFromPattern(pattern[1], shortName), - id: pattern[0] - } - }) + .map(([id, pattern]) => ({ + geometryLength: pattern.geometry?.length, + headsign: extractHeadsignFromPattern(pattern, shortName), + id + })) // Remove duplicate headsigns. Using a reducer means that the first pattern // with a specific headsign is the accepted one. TODO: is this good behavior? .reduce((prev, cur) => { From 3623041d06558e7bef41a6bfdf735a99cdf4a5bd Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 25 Aug 2023 16:42:17 -0400 Subject: [PATCH 2/7] fix(route-details): Obtain diplayed pattern name from orginal pattern dictionary. --- lib/components/viewers/route-details.js | 101 +++++++++++------------- 1 file changed, 44 insertions(+), 57 deletions(-) diff --git a/lib/components/viewers/route-details.js b/lib/components/viewers/route-details.js index faf32780e..7877f9681 100644 --- a/lib/components/viewers/route-details.js +++ b/lib/components/viewers/route-details.js @@ -80,62 +80,58 @@ class RouteDetails extends Component { } render() { - const { intl, operator, pattern, route, setHoveredStop, viewedRoute } = - this.props - const { agency, patterns, shortName, url } = route + const { intl, operator, pattern, route, setHoveredStop } = this.props + const { agency, patterns = {}, shortName, url } = route const routeColor = getRouteColorBasedOnSettings(operator, route) - const headsigns = - patterns && - Object.entries(patterns) - .map(([id, pattern]) => ({ - geometryLength: pattern.geometry?.length, - headsign: extractHeadsignFromPattern(pattern, shortName), - id - })) - // Remove duplicate headsigns. Using a reducer means that the first pattern - // with a specific headsign is the accepted one. TODO: is this good behavior? - .reduce((prev, cur) => { - const amended = prev - const alreadyExistingIndex = prev.findIndex( - (h) => h.headsign === cur.headsign - ) - // If the item we're replacing has less geometry, replace it! - if (alreadyExistingIndex >= 0) { - // Only replace if new pattern has greater geometry - if ( - amended[alreadyExistingIndex].geometryLength < cur.geometryLength - ) { - amended[alreadyExistingIndex] = cur - } - } else { - amended.push(cur) + const headsigns = Object.entries(patterns) + .map(([id, pat]) => ({ + geometryLength: pat.geometry?.length, + headsign: extractHeadsignFromPattern(pat, shortName), + id + })) + // Remove duplicate headsigns. Using a reducer means that the first pattern + // with a specific headsign is the accepted one. TODO: is this good behavior? + .reduce((prev, cur) => { + const amended = prev + const alreadyExistingIndex = prev.findIndex( + (h) => h.headsign === cur.headsign + ) + // If the item we're replacing has less geometry, replace it! + if (alreadyExistingIndex >= 0) { + // Only replace if new pattern has greater geometry + if ( + amended[alreadyExistingIndex].geometryLength < cur.geometryLength + ) { + amended[alreadyExistingIndex] = cur } - return amended - }, []) - .sort((a, b) => { - // sort by number of vehicles on that pattern - const aVehicleCount = route.vehicles?.filter( - (vehicle) => vehicle.patternId === a.id - ).length - const bVehicleCount = route.vehicles?.filter( - (vehicle) => vehicle.patternId === b.id - ).length - - // if both have the same count, sort by pattern geometry length - if (aVehicleCount === bVehicleCount) { - return b.geometryLength - a.geometryLength - } - return bVehicleCount - aVehicleCount - }) + } else { + amended.push(cur) + } + return amended + }, []) + .sort((a, b) => { + // sort by number of vehicles on that pattern + const aVehicleCount = route.vehicles?.filter( + (vehicle) => vehicle.patternId === a.id + ).length + const bVehicleCount = route.vehicles?.filter( + (vehicle) => vehicle.patternId === b.id + ).length + + // if both have the same count, sort by pattern geometry length + if (aVehicleCount === bVehicleCount) { + return b.geometryLength - a.geometryLength + } + return bVehicleCount - aVehicleCount + }) const patternSelectLabel = intl.formatMessage({ id: 'components.RouteDetails.selectADirection' }) const patternSelectName = - headsigns?.find((h) => h.id === viewedRoute?.patternId)?.headsign || - patternSelectLabel + (pattern && patterns[pattern.id]?.headsign) || patternSelectLabel // if no pattern is set, we are in the routeRow return ( @@ -246,12 +242,6 @@ class RouteDetails extends Component { } // connect to redux store -const mapStateToProps = (state) => { - return { - viewedRoute: state.otp.ui.viewedRoute - } -} - const mapDispatchToProps = { findStopsForPattern, setHoveredStop, @@ -259,7 +249,4 @@ const mapDispatchToProps = { setViewedStop } -export default connect( - mapStateToProps, - mapDispatchToProps -)(injectIntl(RouteDetails)) +export default connect(null, mapDispatchToProps)(injectIntl(RouteDetails)) From 612933a3b6f114ca6445ca81018cb7176f9aa721 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 25 Aug 2023 16:54:01 -0400 Subject: [PATCH 3/7] refactor(Rename props, remove dead code.): --- lib/components/viewers/pattern-viewer.tsx | 6 +---- lib/components/viewers/route-details.js | 33 +++++++---------------- 2 files changed, 10 insertions(+), 29 deletions(-) diff --git a/lib/components/viewers/pattern-viewer.tsx b/lib/components/viewers/pattern-viewer.tsx index 62102089e..1f6204633 100644 --- a/lib/components/viewers/pattern-viewer.tsx +++ b/lib/components/viewers/pattern-viewer.tsx @@ -131,11 +131,7 @@ const PatternViewer = ({ - + ) } diff --git a/lib/components/viewers/route-details.js b/lib/components/viewers/route-details.js index 7877f9681..48ba77c02 100644 --- a/lib/components/viewers/route-details.js +++ b/lib/components/viewers/route-details.js @@ -7,14 +7,13 @@ import PropTypes from 'prop-types' import React, { Component } from 'react' import styled from 'styled-components' +import * as uiActions from '../../actions/ui' import { extractHeadsignFromPattern, getRouteColorBasedOnSettings } from '../../util/viewer' -import { findStopsForPattern } from '../../actions/api' import { getOperatorName } from '../../util/state' import { LinkOpensNewWindow } from '../util/externalLink' -import { setHoveredStop, setViewedRoute, setViewedStop } from '../../actions/ui' import { SortResultsDropdown } from '../util/dropdown' import { UnstyledButton } from '../util/unstyled-button' @@ -39,27 +38,14 @@ const PatternSelectButton = styled(UnstyledButton)` class RouteDetails extends Component { static propTypes = { - findStopsForPattern: findStopsForPattern.type, operator: PropTypes.shape({ defaultRouteColor: PropTypes.string, defaultRouteTextColor: PropTypes.string, longNameSplitter: PropTypes.string }), // There are more items in pattern and route, but none mandatory - pattern: PropTypes.shape({ id: PropTypes.string }), - route: PropTypes.shape({ id: PropTypes.string }), - setHoveredStop: setHoveredStop.type, - setViewedRoute: setViewedRoute.type - } - - /** - * Requests stop list for current pattern - */ - getStops = () => { - const { findStopsForPattern, pattern, route } = this.props - if (pattern && route) { - findStopsForPattern({ patternId: pattern.id, routeId: route.id }) - } + patternId: PropTypes.string, + route: PropTypes.shape({ id: PropTypes.string }) } /** @@ -80,8 +66,9 @@ class RouteDetails extends Component { } render() { - const { intl, operator, pattern, route, setHoveredStop } = this.props + const { intl, operator, patternId, route, setHoveredStop } = this.props const { agency, patterns = {}, shortName, url } = route + const pattern = patterns[patternId] const routeColor = getRouteColorBasedOnSettings(operator, route) @@ -130,8 +117,7 @@ class RouteDetails extends Component { const patternSelectLabel = intl.formatMessage({ id: 'components.RouteDetails.selectADirection' }) - const patternSelectName = - (pattern && patterns[pattern.id]?.headsign) || patternSelectLabel + const patternSelectName = pattern?.headsign || patternSelectLabel // if no pattern is set, we are in the routeRow return ( @@ -243,10 +229,9 @@ class RouteDetails extends Component { // connect to redux store const mapDispatchToProps = { - findStopsForPattern, - setHoveredStop, - setViewedRoute, - setViewedStop + setHoveredStop: uiActions.setHoveredStop, + setViewedRoute: uiActions.setViewedRoute, + setViewedStop: uiActions.setViewedStop } export default connect(null, mapDispatchToProps)(injectIntl(RouteDetails)) From b64304ac57a73cef88bc32909d95ea3ba15de810 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 25 Aug 2023 18:09:52 -0400 Subject: [PATCH 4/7] refactor(route-details): Convert to TypeScript, add missing types. --- lib/components/util/types.ts | 30 ++++---- .../{route-details.js => route-details.tsx} | 72 +++++++++++-------- .../viewers/{styled.js => styled.ts} | 16 +++-- 3 files changed, 69 insertions(+), 49 deletions(-) rename lib/components/viewers/{route-details.js => route-details.tsx} (83%) rename lib/components/viewers/{styled.js => styled.ts} (89%) diff --git a/lib/components/util/types.ts b/lib/components/util/types.ts index d75469556..febaa585d 100644 --- a/lib/components/util/types.ts +++ b/lib/components/util/types.ts @@ -1,3 +1,5 @@ +import { Route } from '@opentripplanner/types' + // TYPESCRIPT TODO: move this to a larger shared types file, preferably within otp-ui export interface StopData { bikeRental: BikeRental @@ -22,15 +24,6 @@ export interface BikeRental { stations: any[] } -export interface Route { - agencyId: string - agencyName: string - id: string - longName: string - mode: string - sortOrder: number -} - // FIXME: incomplete export interface StopTime { departureDelay: number @@ -45,6 +38,11 @@ export interface Pattern { desc: string headsign: string id: string + patternGeometry?: { + length: number + points: string + } + stops?: StopData[] } export interface Time { @@ -80,14 +78,18 @@ export interface ViewedRouteState { routeId: string } +export interface RouteVehicle { + patternId: string +} + // Routes have many properties beside id, but none of these are guaranteed. -export interface ViewedRouteObject { - id: string - longName?: string +export interface ViewedRouteObject extends Route { patterns?: Record pending?: boolean - shortName?: string - textColor?: string + url?: string + vehicles?: RouteVehicle[] } export type SetViewedRouteHandler = (route?: ViewedRouteState) => void + +export type SetViewedStopHandler = (payload: { stopId: string }) => void diff --git a/lib/components/viewers/route-details.js b/lib/components/viewers/route-details.tsx similarity index 83% rename from lib/components/viewers/route-details.js rename to lib/components/viewers/route-details.tsx index 48ba77c02..2dbc4e3fe 100644 --- a/lib/components/viewers/route-details.js +++ b/lib/components/viewers/route-details.tsx @@ -1,9 +1,7 @@ -// FIXME: typescript -/* eslint-disable react/prop-types */ import { connect } from 'react-redux' -import { FormattedMessage, injectIntl } from 'react-intl' +import { FormattedMessage, injectIntl, IntlShape } from 'react-intl' import { getMostReadableTextColor } from '@opentripplanner/core-utils/lib/route' -import PropTypes from 'prop-types' +import { TransitOperator } from '@opentripplanner/types' import React, { Component } from 'react' import styled from 'styled-components' @@ -14,6 +12,11 @@ import { } from '../../util/viewer' import { getOperatorName } from '../../util/state' import { LinkOpensNewWindow } from '../util/externalLink' +import { + SetViewedRouteHandler, + SetViewedStopHandler, + ViewedRouteObject +} from '../util/types' import { SortResultsDropdown } from '../util/dropdown' import { UnstyledButton } from '../util/unstyled-button' @@ -36,23 +39,28 @@ const PatternSelectButton = styled(UnstyledButton)` } ` -class RouteDetails extends Component { - static propTypes = { - operator: PropTypes.shape({ - defaultRouteColor: PropTypes.string, - defaultRouteTextColor: PropTypes.string, - longNameSplitter: PropTypes.string - }), - // There are more items in pattern and route, but none mandatory - patternId: PropTypes.string, - route: PropTypes.shape({ id: PropTypes.string }) - } +interface PatternSummary { + geometryLength: number + headsign: string + id: string +} + +interface Props { + intl: IntlShape + operator: TransitOperator + patternId: string + route: ViewedRouteObject + setHoveredStop: (id: string | null) => void + setViewedRoute: SetViewedRouteHandler + setViewedStop: SetViewedStopHandler +} +class RouteDetails extends Component { /** * If a headsign link is clicked, set that pattern in redux state so that the * view can adjust */ - _headSignButtonClicked = (id) => { + _headSignButtonClicked = (id: string) => { const { route, setViewedRoute } = this.props setViewedRoute({ patternId: id, routeId: route.id }) } @@ -60,7 +68,7 @@ class RouteDetails extends Component { /** * If a stop link is clicked, redirect to stop viewer */ - _stopLinkClicked = (stopId) => { + _stopLinkClicked = (stopId: string) => { const { setViewedStop } = this.props setViewedStop({ stopId }) } @@ -73,14 +81,16 @@ class RouteDetails extends Component { const routeColor = getRouteColorBasedOnSettings(operator, route) const headsigns = Object.entries(patterns) - .map(([id, pat]) => ({ - geometryLength: pat.geometry?.length, - headsign: extractHeadsignFromPattern(pat, shortName), - id - })) + .map( + ([id, pat]): PatternSummary => ({ + geometryLength: pat.patternGeometry?.length || 0, + headsign: extractHeadsignFromPattern(pat, shortName), + id + }) + ) // Remove duplicate headsigns. Using a reducer means that the first pattern // with a specific headsign is the accepted one. TODO: is this good behavior? - .reduce((prev, cur) => { + .reduce((prev: PatternSummary[], cur) => { const amended = prev const alreadyExistingIndex = prev.findIndex( (h) => h.headsign === cur.headsign @@ -100,12 +110,12 @@ class RouteDetails extends Component { }, []) .sort((a, b) => { // sort by number of vehicles on that pattern - const aVehicleCount = route.vehicles?.filter( - (vehicle) => vehicle.patternId === a.id - ).length - const bVehicleCount = route.vehicles?.filter( - (vehicle) => vehicle.patternId === b.id - ).length + const aVehicleCount = + route.vehicles?.filter((vehicle) => vehicle.patternId === a.id) + .length || 0 + const bVehicleCount = + route.vehicles?.filter((vehicle) => vehicle.patternId === b.id) + .length || 0 // if both have the same count, sort by pattern geometry length if (aVehicleCount === bVehicleCount) { @@ -164,7 +174,7 @@ class RouteDetails extends Component { pullRight style={{ color: 'black' }} > - {headsigns.map((h) => ( + {headsigns.map((h: PatternSummary) => (
  • this._headSignButtonClicked(h.id)} @@ -182,7 +192,7 @@ class RouteDetails extends Component {

    diff --git a/lib/components/viewers/styled.js b/lib/components/viewers/styled.ts similarity index 89% rename from lib/components/viewers/styled.js rename to lib/components/viewers/styled.ts index e1a4bd984..16bfbe173 100644 --- a/lib/components/viewers/styled.js +++ b/lib/components/viewers/styled.ts @@ -1,7 +1,14 @@ import styled from 'styled-components' +interface RenderProps { + backgroundColor?: string + full?: boolean + routeColor?: string + textColor?: string +} + /** Route Details */ -export const Container = styled.div` +export const Container = styled.div` background-color: ${(props) => props.full ? props.backgroundColor || '#ddd' : 'inherit'}; color: ${(props) => (props.full ? props.textColor : 'inherit')}; @@ -60,7 +67,7 @@ export const PatternContainer = styled.div` } ` -export const StopContainer = styled.ol` +export const StopContainer = styled.ol` color: ${(props) => props?.textColor || '#333'}; background-color: ${(props) => props?.backgroundColor || '#fff'}; overflow-y: scroll; @@ -69,7 +76,7 @@ export const StopContainer = styled.ol` are shown when browsers don't calculate 100% sensibly */ padding: 15px 0 100px; ` -export const StopLink = styled.button` +export const StopLink = styled.button` color: ${(props) => props?.textColor + 'da' || '#333'}; background-color: transparent; border: none; @@ -82,7 +89,8 @@ export const StopLink = styled.button` text-decoration: underline; } ` -export const Stop = styled.li` + +export const Stop = styled.li` cursor: pointer; display: block; white-space: nowrap; From dba0955402098006461e32b18082a386c9fa498a Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Mon, 28 Aug 2023 10:16:22 -0400 Subject: [PATCH 5/7] refactor(route-details): Use logic to determine headsign with pattern. --- lib/components/viewers/route-details.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/components/viewers/route-details.tsx b/lib/components/viewers/route-details.tsx index 2dbc4e3fe..f5ee0d53c 100644 --- a/lib/components/viewers/route-details.tsx +++ b/lib/components/viewers/route-details.tsx @@ -127,7 +127,9 @@ class RouteDetails extends Component { const patternSelectLabel = intl.formatMessage({ id: 'components.RouteDetails.selectADirection' }) - const patternSelectName = pattern?.headsign || patternSelectLabel + + const patternSelectName = + extractHeadsignFromPattern(pattern, shortName) || patternSelectLabel // if no pattern is set, we are in the routeRow return ( From af56ed50d15a03d667a13000b2fd77e073cdcb2f Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Mon, 28 Aug 2023 10:30:44 -0400 Subject: [PATCH 6/7] fix(route-details): Use index as key in stop list. --- lib/components/viewers/route-details.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/components/viewers/route-details.tsx b/lib/components/viewers/route-details.tsx index f5ee0d53c..4c0524b63 100644 --- a/lib/components/viewers/route-details.tsx +++ b/lib/components/viewers/route-details.tsx @@ -129,7 +129,8 @@ class RouteDetails extends Component { }) const patternSelectName = - extractHeadsignFromPattern(pattern, shortName) || patternSelectLabel + (pattern ? extractHeadsignFromPattern(pattern, shortName) : null) || + patternSelectLabel // if no pattern is set, we are in the routeRow return ( @@ -205,9 +206,10 @@ class RouteDetails extends Component { onMouseLeave={() => setHoveredStop(null)} textColor={getMostReadableTextColor(routeColor, route?.textColor)} > - {pattern?.stops?.map((stop) => ( + {pattern?.stops?.map((stop, index) => ( this._stopLinkClicked(stop.id)} onFocus={() => setHoveredStop(stop.id)} onMouseOver={() => setHoveredStop(stop.id)} From 85527a4296723f9b0de3bf3d80a23ba5b51cd718 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Mon, 28 Aug 2023 11:29:12 -0400 Subject: [PATCH 7/7] refactor(route-details): Tweak a11y and headsign lookup. --- lib/components/viewers/route-details.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/components/viewers/route-details.tsx b/lib/components/viewers/route-details.tsx index 4c0524b63..579e376cc 100644 --- a/lib/components/viewers/route-details.tsx +++ b/lib/components/viewers/route-details.tsx @@ -129,10 +129,9 @@ class RouteDetails extends Component { }) const patternSelectName = - (pattern ? extractHeadsignFromPattern(pattern, shortName) : null) || + headsigns.find((h) => h.id === pattern?.id)?.headsign || patternSelectLabel - // if no pattern is set, we are in the routeRow return ( { // Use array index instead of stop id because a stop can be visited several times. key={index} onClick={() => this._stopLinkClicked(stop.id)} - onFocus={() => setHoveredStop(stop.id)} onMouseOver={() => setHoveredStop(stop.id)} routeColor={ routeColor.includes('ffffff') ? '#333' : routeColor @@ -224,6 +222,7 @@ class RouteDetails extends Component { this._stopLinkClicked(stop.id)} + onFocus={() => setHoveredStop(stop.id)} textColor={getMostReadableTextColor( routeColor, route?.textColor