From ec0662d3ca7d37ae76a4eaf634a8ff4bf1959e16 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 3 Mar 2021 16:40:50 -0500 Subject: [PATCH 01/48] feat(mobile/batch-results-screen): Add mobile batch results screen. --- example.js | 5 + lib/components/mobile/batch-results-screen.js | 254 ++++++++++++++++++ lib/components/mobile/container.js | 3 +- lib/components/mobile/main.js | 3 +- .../narrative/narrative-itineraries.js | 11 +- lib/index.js | 4 + 6 files changed, 275 insertions(+), 5 deletions(-) create mode 100644 lib/components/mobile/batch-results-screen.js diff --git a/example.js b/example.js index 0f4484e8c..8bc805c1b 100644 --- a/example.js +++ b/example.js @@ -13,6 +13,7 @@ import createLogger from 'redux-logger' // import OTP-RR components import { + BatchResultsScreen, BatchRoutingPanel, BatchSearchScreen, CallTakerControls, @@ -20,6 +21,7 @@ import { CallTakerWindows, DefaultItinerary, DefaultMainPanel, + MobileResultsScreen, MobileSearchScreen, ResponsiveWebapp, createCallTakerReducer, @@ -71,6 +73,9 @@ const components = { ? BatchRoutingPanel : DefaultMainPanel, MapWindows: isCallTakerModuleEnabled ? CallTakerWindows : null, + MobileResultsScreen: isBatchRoutingEnabled + ? BatchResultsScreen + : MobileResultsScreen, MobileSearchScreen: isBatchRoutingEnabled ? BatchSearchScreen : MobileSearchScreen, diff --git a/lib/components/mobile/batch-results-screen.js b/lib/components/mobile/batch-results-screen.js new file mode 100644 index 000000000..5f327bce5 --- /dev/null +++ b/lib/components/mobile/batch-results-screen.js @@ -0,0 +1,254 @@ +import coreUtils from '@opentripplanner/core-utils' +import LocationIcon from '@opentripplanner/location-icon' +import PropTypes from 'prop-types' +import React, { Component } from 'react' +import { Button, Col, Row } from 'react-bootstrap' +import { connect } from 'react-redux' +import styled from 'styled-components' + +import Map from '../map/map' +import ErrorMessage from '../form/error-message' +import Icon from '../narrative/icon' +import NarrativeItineraries from '../narrative/narrative-itineraries' + +import MobileContainer from './container' +import MobileNavigationBar from './navigation-bar' + +import { MobileScreens, setMobileScreen } from '../../actions/ui' +import { setUseRealtimeResponse } from '../../actions/narrative' +import { clearActiveSearch } from '../../actions/form' +import { + getActiveError, + getActiveItineraries, + getActiveSearch, + getRealtimeEffects +} from '../../util/state' + +const LocationContainer = styled.div` + font-weight: 300; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +` + +const LocationSummaryContainer = styled.div` + height: 50px; + padding-right: 10px; +` + +const LocationsSummaryColFromTo = styled(Col)` + font-size: 1.1em; + line-height: 1.2em; +` + +const LocationsSummaryRow = styled(Row)` + padding: 4px 8px; +` + +const StyledLocationIcon = styled(LocationIcon)` + margin: 3px; +` + +class BatchMobileResultsScreen extends Component { + static propTypes = { + activeItineraryIndex: PropTypes.number, + query: PropTypes.object, + resultCount: PropTypes.number, + setMobileScreen: PropTypes.func + } + + constructor () { + super() + this.state = { + expanded: true, + itineraryExpanded: false, + mapExpanded: false + } + } + + componentDidMount () { + // Get the target element that we want to persist scrolling for + // FIXME Do we need to add something that removes the listeners when + // component unmounts? + coreUtils.ui.enableScrollForSelector('.mobile-narrative-container') + } + + componentDidUpdate (prevProps) { + // Check if the active leg changed + if (this.props.activeLeg !== prevProps.activeLeg) { + this._setExpanded(false) + } + } + + _setExpanded (expanded) { + this.setState({ expanded }) + this.refs['narrative-container'].scrollTop = 0 + } + + _editSearchClicked = () => { + this.props.clearActiveSearch() + this.props.setMobileScreen(MobileScreens.SEARCH_FORM) + } + + _handleToggleDetailedItinerary = itineraryExpanded => { + this.setState({ itineraryExpanded }) + } + + _optionClicked = () => { + this._setExpanded(!this.state.expanded) + } + + _toggleMapExpanded = () => { + this.setState({ mapExpanded: !this.state.mapExpanded }) + // Also find a way to recenter (and rerender) the map. + } + + renderError = () => { + const { error } = this.props + + return ( + + + {this.renderLocationsSummary()} +
+
+ +
+ +
+
+
+ ) + } + + renderLocationsSummary = () => { + const { query } = this.props + + return ( + + + + + { query.from ? query.from.name : '' } + + + { query.to ? query.to.name : '' } + + + + + + + + ) + } + + render () { + const { + error, + resultCount + } = this.props + const { itineraryExpanded, mapExpanded } = this.state + + const narrativeContainerStyle = itineraryExpanded + ? { flexGrow: 1, overflowY: 'auto', padding: 0, position: 'unset' } + : { flex: '0 0 60%', overflowY: 'auto', padding: 0, position: 'unset' } + + // Ensure that narrative covers map. + narrativeContainerStyle.backgroundColor = 'white' + + let headerAction = null + + if (error) { + return this.renderError() + } + + return ( + + 1 ? 's' : ''}` + : 'Waiting...' + } + /> + + {this.renderLocationsSummary()} + {!itineraryExpanded && ( +
+ {/* Extra container for positioning the expand map button relative to the map. */} +
+ + +
+
+ )} + {!mapExpanded && ( +
+ +
+ )} + +
+ ) + } +} + +// connect to the redux store + +const mapStateToProps = (state, ownProps) => { + const activeSearch = getActiveSearch(state.otp) + const {useRealtime} = state.otp + const response = !activeSearch + ? null + : useRealtime ? activeSearch.response : activeSearch.nonRealtimeResponse + + const realtimeEffects = getRealtimeEffects(state.otp) + const itineraries = getActiveItineraries(state.otp) + return { + query: state.otp.currentQuery, + realtimeEffects, + error: getActiveError(state.otp), + resultCount: + response + ? activeSearch.query.routingType === 'ITINERARY' + ? itineraries.length + : response.otp.profile.length + : null, + useRealtime, + activeItineraryIndex: activeSearch ? activeSearch.activeItinerary : null, + activeLeg: activeSearch ? activeSearch.activeLeg : null + } +} + +const mapDispatchToProps = { + clearActiveSearch, + setMobileScreen, + setUseRealtimeResponse +} + +export default connect(mapStateToProps, mapDispatchToProps)(BatchMobileResultsScreen) diff --git a/lib/components/mobile/container.js b/lib/components/mobile/container.js index fc79f524e..1928c1458 100644 --- a/lib/components/mobile/container.js +++ b/lib/components/mobile/container.js @@ -2,8 +2,9 @@ import React, { Component } from 'react' export default class MobileContainer extends Component { render () { + const { className, style } = this.props return ( -
+
{this.props.children}
) diff --git a/lib/components/mobile/main.js b/lib/components/mobile/main.js index dc9eb2344..db28eafd4 100644 --- a/lib/components/mobile/main.js +++ b/lib/components/mobile/main.js @@ -6,7 +6,6 @@ import MobileDateTimeScreen from './date-time-screen' import MobileOptionsScreen from './options-screen' import MobileLocationSearch from './location-search' import MobileWelcomeScreen from './welcome-screen' -import MobileResultsScreen from './results-screen' import MobileStopViewer from './stop-viewer' import MobileTripViewer from './trip-viewer' import MobileRouteViewer from './route-viewer' @@ -45,7 +44,7 @@ class MobileMain extends Component { } render () { - const { MobileSearchScreen } = this.context + const { MobileResultsScreen, MobileSearchScreen } = this.context const { uiState } = this.props // check for route viewer diff --git a/lib/components/narrative/narrative-itineraries.js b/lib/components/narrative/narrative-itineraries.js index 904dd7e0e..219d0aaba 100644 --- a/lib/components/narrative/narrative-itineraries.js +++ b/lib/components/narrative/narrative-itineraries.js @@ -36,10 +36,11 @@ function humanReadableMode (modeStr) { class NarrativeItineraries extends Component { static propTypes = { + activeItinerary: PropTypes.number, containerStyle: PropTypes.object, itineraries: PropTypes.array, + onToggleDetailedItinerary: PropTypes.func, pending: PropTypes.bool, - activeItinerary: PropTypes.number, setActiveItinerary: PropTypes.func, setActiveLeg: PropTypes.func, setActiveStep: PropTypes.func, @@ -52,7 +53,13 @@ class NarrativeItineraries extends Component { state = {} _toggleDetailedItinerary = () => { - this.setState({showDetails: !this.state.showDetails}) + const showDetails = !this.state.showDetails + this.setState({ showDetails }) + + const { onToggleDetailedItinerary } = this.props + if (onToggleDetailedItinerary) { + onToggleDetailedItinerary(showDetails) + } } _onFilterChange = evt => { diff --git a/lib/index.js b/lib/index.js index da6c458e6..f51a2d2f2 100644 --- a/lib/index.js +++ b/lib/index.js @@ -29,6 +29,7 @@ import TripTools from './components/narrative/trip-tools' import LineItinerary from './components/narrative/line-itin/line-itinerary' import MobileMain from './components/mobile/main' +import MobileResultsScreen from './components/mobile/results-screen' import MobileSearchScreen from './components/mobile/search-screen' import NavLoginButton from './components/user/nav-login-button' @@ -46,6 +47,7 @@ import DesktopNav from './components/app/desktop-nav' import DefaultMainPanel from './components/app/default-main-panel' import BatchRoutingPanel from './components/app/batch-routing-panel' +import BatchResultsScreen from './components/mobile/batch-results-screen' import BatchSearchScreen from './components/mobile/batch-search-screen' import { setAutoPlan, setMapCenter } from './actions/config' @@ -95,6 +97,7 @@ export { // mobile components MobileMain, + MobileResultsScreen, MobileSearchScreen, // viewer components @@ -115,6 +118,7 @@ export { DefaultMainPanel, // batch routing components + BatchResultsScreen, BatchRoutingPanel, BatchSearchScreen, From d35ce7f3758381d43835113f3db247743368a9a9 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 3 Mar 2021 17:32:11 -0500 Subject: [PATCH 02/48] refactor(mobile/BatchResultsScreen): Clean up code. --- lib/components/mobile/batch-results-screen.js | 51 +++++++------------ lib/components/mobile/container.js | 2 +- 2 files changed, 19 insertions(+), 34 deletions(-) diff --git a/lib/components/mobile/batch-results-screen.js b/lib/components/mobile/batch-results-screen.js index 5f327bce5..85694b826 100644 --- a/lib/components/mobile/batch-results-screen.js +++ b/lib/components/mobile/batch-results-screen.js @@ -15,13 +15,11 @@ import MobileContainer from './container' import MobileNavigationBar from './navigation-bar' import { MobileScreens, setMobileScreen } from '../../actions/ui' -import { setUseRealtimeResponse } from '../../actions/narrative' import { clearActiveSearch } from '../../actions/form' import { getActiveError, getActiveItineraries, - getActiveSearch, - getRealtimeEffects + getActiveSearch } from '../../util/state' const LocationContainer = styled.div` @@ -49,9 +47,15 @@ const StyledLocationIcon = styled(LocationIcon)` margin: 3px; ` +const ExpandMapButton = styled(Button)` + bottom: 16px; + position: absolute; + right: 10px; + zIndex: 999999; +` + class BatchMobileResultsScreen extends Component { static propTypes = { - activeItineraryIndex: PropTypes.number, query: PropTypes.object, resultCount: PropTypes.number, setMobileScreen: PropTypes.func @@ -60,7 +64,6 @@ class BatchMobileResultsScreen extends Component { constructor () { super() this.state = { - expanded: true, itineraryExpanded: false, mapExpanded: false } @@ -76,13 +79,12 @@ class BatchMobileResultsScreen extends Component { componentDidUpdate (prevProps) { // Check if the active leg changed if (this.props.activeLeg !== prevProps.activeLeg) { - this._setExpanded(false) + this._setItineraryExpanded(false) } } - _setExpanded (expanded) { - this.setState({ expanded }) - this.refs['narrative-container'].scrollTop = 0 + _setItineraryExpanded (itineraryExpanded) { + this.setState({ itineraryExpanded }) } _editSearchClicked = () => { @@ -90,14 +92,6 @@ class BatchMobileResultsScreen extends Component { this.props.setMobileScreen(MobileScreens.SEARCH_FORM) } - _handleToggleDetailedItinerary = itineraryExpanded => { - this.setState({ itineraryExpanded }) - } - - _optionClicked = () => { - this._setExpanded(!this.state.expanded) - } - _toggleMapExpanded = () => { this.setState({ mapExpanded: !this.state.mapExpanded }) // Also find a way to recenter (and rerender) the map. @@ -162,8 +156,6 @@ class BatchMobileResultsScreen extends Component { // Ensure that narrative covers map. narrativeContainerStyle.backgroundColor = 'white' - let headerAction = null - if (error) { return this.renderError() } @@ -171,7 +163,6 @@ class BatchMobileResultsScreen extends Component { return ( 1 ? 's' : ''}` : 'Waiting...' @@ -184,14 +175,13 @@ class BatchMobileResultsScreen extends Component { {/* Extra container for positioning the expand map button relative to the map. */}
- +
)} @@ -208,7 +198,7 @@ class BatchMobileResultsScreen extends Component { height: '100%', width: '100%' }} - onToggleDetailedItinerary={this._handleToggleDetailedItinerary} + onToggleDetailedItinerary={this._setItineraryExpanded} /> )} @@ -227,28 +217,23 @@ const mapStateToProps = (state, ownProps) => { ? null : useRealtime ? activeSearch.response : activeSearch.nonRealtimeResponse - const realtimeEffects = getRealtimeEffects(state.otp) const itineraries = getActiveItineraries(state.otp) return { - query: state.otp.currentQuery, - realtimeEffects, + activeLeg: activeSearch ? activeSearch.activeLeg : null, error: getActiveError(state.otp), + query: state.otp.currentQuery, resultCount: response ? activeSearch.query.routingType === 'ITINERARY' ? itineraries.length : response.otp.profile.length - : null, - useRealtime, - activeItineraryIndex: activeSearch ? activeSearch.activeItinerary : null, - activeLeg: activeSearch ? activeSearch.activeLeg : null + : null } } const mapDispatchToProps = { clearActiveSearch, - setMobileScreen, - setUseRealtimeResponse + setMobileScreen } export default connect(mapStateToProps, mapDispatchToProps)(BatchMobileResultsScreen) diff --git a/lib/components/mobile/container.js b/lib/components/mobile/container.js index 1928c1458..021d536f4 100644 --- a/lib/components/mobile/container.js +++ b/lib/components/mobile/container.js @@ -4,7 +4,7 @@ export default class MobileContainer extends Component { render () { const { className, style } = this.props return ( -
+
{this.props.children}
) From b1a84b6a583f5a7950feac17f1d22fe514d64ec5 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Thu, 4 Mar 2021 17:31:44 -0500 Subject: [PATCH 03/48] refactor(BatchResultsScreen): Fix fitting itinerary after expanding map. --- lib/components/mobile/batch-results-screen.js | 91 ++++++++++--------- 1 file changed, 50 insertions(+), 41 deletions(-) diff --git a/lib/components/mobile/batch-results-screen.js b/lib/components/mobile/batch-results-screen.js index 85694b826..401f8e07b 100644 --- a/lib/components/mobile/batch-results-screen.js +++ b/lib/components/mobile/batch-results-screen.js @@ -10,18 +10,17 @@ import Map from '../map/map' import ErrorMessage from '../form/error-message' import Icon from '../narrative/icon' import NarrativeItineraries from '../narrative/narrative-itineraries' - -import MobileContainer from './container' -import MobileNavigationBar from './navigation-bar' - -import { MobileScreens, setMobileScreen } from '../../actions/ui' import { clearActiveSearch } from '../../actions/form' +import { MobileScreens, setMobileScreen } from '../../actions/ui' import { getActiveError, getActiveItineraries, getActiveSearch } from '../../util/state' +import MobileNavigationBar from './navigation-bar' +import MobileContainer from './container' + const LocationContainer = styled.div` font-weight: 300; overflow: hidden; @@ -51,7 +50,7 @@ const ExpandMapButton = styled(Button)` bottom: 16px; position: absolute; right: 10px; - zIndex: 999999; + z-index: 999999; ` class BatchMobileResultsScreen extends Component { @@ -83,7 +82,7 @@ class BatchMobileResultsScreen extends Component { } } - _setItineraryExpanded (itineraryExpanded) { + _setItineraryExpanded = itineraryExpanded => { this.setState({ itineraryExpanded }) } @@ -94,7 +93,6 @@ class BatchMobileResultsScreen extends Component { _toggleMapExpanded = () => { this.setState({ mapExpanded: !this.state.mapExpanded }) - // Also find a way to recenter (and rerender) the map. } renderError = () => { @@ -142,6 +140,25 @@ class BatchMobileResultsScreen extends Component { ) } + renderMap () { + const { mapExpanded } = this.state + return ( +
+ {/* Extra container for positioning the expand map button relative to the map. */} +
+ + + + {mapExpanded ? 'Show results' : 'Expand map'} + +
+
+ ) + } + render () { const { error, @@ -170,39 +187,32 @@ class BatchMobileResultsScreen extends Component { /> {this.renderLocationsSummary()} - {!itineraryExpanded && ( -
- {/* Extra container for positioning the expand map button relative to the map. */} -
- - - - {mapExpanded ? 'Show results' : 'Expand map'} - -
-
- )} - {!mapExpanded && ( -
- -
- )} + {mapExpanded + // Set up two separate renderings of the map according to map expanded state, + // so that it is properly sized and itineraries fit under either conditions. + // (Otherwise, if just the narrative is added/removed, the map doesn't resize properly.) + ? this.renderMap() + : ( + <> + {!itineraryExpanded && this.renderMap()} +
+ +
+ + )} ) } @@ -219,7 +229,6 @@ const mapStateToProps = (state, ownProps) => { const itineraries = getActiveItineraries(state.otp) return { - activeLeg: activeSearch ? activeSearch.activeLeg : null, error: getActiveError(state.otp), query: state.otp.currentQuery, resultCount: From 9c21e13a74aa04281f3ef589a26029c675d0b011 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Thu, 4 Mar 2021 18:05:08 -0500 Subject: [PATCH 04/48] refactor(BatchResultsScreen): Tweak styles. --- lib/components/mobile/batch-results-screen.js | 44 +++++++++++++++---- lib/components/mobile/container.js | 6 +-- 2 files changed, 39 insertions(+), 11 deletions(-) diff --git a/lib/components/mobile/batch-results-screen.js b/lib/components/mobile/batch-results-screen.js index 401f8e07b..127a384dd 100644 --- a/lib/components/mobile/batch-results-screen.js +++ b/lib/components/mobile/batch-results-screen.js @@ -21,6 +21,21 @@ import { import MobileNavigationBar from './navigation-bar' import MobileContainer from './container' +const StyledMobileContainer = styled(MobileContainer)` + bottom: 0; + display: flex; + flex-direction: column; + left: 0; + padding-top: 50px; + position: fixed; + right: 0; + top: 0; + + .options > .header { + margin: 10px; + } +` + const LocationContainer = styled.div` font-weight: 300; overflow: hidden; @@ -46,8 +61,19 @@ const StyledLocationIcon = styled(LocationIcon)` margin: 3px; ` +const ResultsMap = styled.div` + flex-grow: 1; + position: unset; +` + +const MapContainer = styled.div` + height: 100%; + position: relative; + width: 100%; +` + const ExpandMapButton = styled(Button)` - bottom: 16px; + bottom: 25px; position: absolute; right: 10px; z-index: 999999; @@ -143,9 +169,11 @@ class BatchMobileResultsScreen extends Component { renderMap () { const { mapExpanded } = this.state return ( -
- {/* Extra container for positioning the expand map button relative to the map. */} -
+ + {/* Extra container for positioning the expand map button relative to the map. + (ResultsMap above has position =) + */} + {mapExpanded ? 'Show results' : 'Expand map'} -
-
+ + ) } @@ -178,7 +206,7 @@ class BatchMobileResultsScreen extends Component { } return ( - + 1 ? 's' : ''}` @@ -213,7 +241,7 @@ class BatchMobileResultsScreen extends Component {
)} - + ) } } diff --git a/lib/components/mobile/container.js b/lib/components/mobile/container.js index 021d536f4..0795ae7c7 100644 --- a/lib/components/mobile/container.js +++ b/lib/components/mobile/container.js @@ -2,10 +2,10 @@ import React, { Component } from 'react' export default class MobileContainer extends Component { render () { - const { className, style } = this.props + const { children, className } = this.props return ( -
- {this.props.children} +
+ {children}
) } From 90be15adc7545c80397c06ec4d2881cd76a61605 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 5 Mar 2021 14:32:07 -0500 Subject: [PATCH 05/48] refactor(mobile/ResultsHeaderAndError): Extract component. Refactors. --- lib/components/mobile/batch-results-screen.js | 213 +++--------------- .../mobile/results-header-and-error.js | 142 ++++++++++++ lib/components/mobile/results-screen.js | 167 +++----------- 3 files changed, 208 insertions(+), 314 deletions(-) create mode 100644 lib/components/mobile/results-header-and-error.js diff --git a/lib/components/mobile/batch-results-screen.js b/lib/components/mobile/batch-results-screen.js index 127a384dd..b26bd9ef0 100644 --- a/lib/components/mobile/batch-results-screen.js +++ b/lib/components/mobile/batch-results-screen.js @@ -1,75 +1,30 @@ -import coreUtils from '@opentripplanner/core-utils' -import LocationIcon from '@opentripplanner/location-icon' import PropTypes from 'prop-types' import React, { Component } from 'react' -import { Button, Col, Row } from 'react-bootstrap' +import { Button } from 'react-bootstrap' import { connect } from 'react-redux' import styled from 'styled-components' import Map from '../map/map' -import ErrorMessage from '../form/error-message' import Icon from '../narrative/icon' import NarrativeItineraries from '../narrative/narrative-itineraries' -import { clearActiveSearch } from '../../actions/form' -import { MobileScreens, setMobileScreen } from '../../actions/ui' -import { - getActiveError, - getActiveItineraries, - getActiveSearch -} from '../../util/state' +import { getActiveError } from '../../util/state' -import MobileNavigationBar from './navigation-bar' import MobileContainer from './container' +import ResultsHeaderAndError from './results-header-and-error' const StyledMobileContainer = styled(MobileContainer)` - bottom: 0; - display: flex; - flex-direction: column; - left: 0; - padding-top: 50px; - position: fixed; - right: 0; - top: 0; - .options > .header { margin: 10px; } -` - -const LocationContainer = styled.div` - font-weight: 300; - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; -` - -const LocationSummaryContainer = styled.div` - height: 50px; - padding-right: 10px; -` - -const LocationsSummaryColFromTo = styled(Col)` - font-size: 1.1em; - line-height: 1.2em; -` - -const LocationsSummaryRow = styled(Row)` - padding: 4px 8px; -` -const StyledLocationIcon = styled(LocationIcon)` - margin: 3px; -` - -const ResultsMap = styled.div` - flex-grow: 1; - position: unset; -` - -const MapContainer = styled.div` - height: 100%; - position: relative; - width: 100%; + &.otp.mobile .mobile-narrative-container { + bottom: 0; + left: 0; + overflow-y: auto; + padding: 0; + position: fixed; + right: 0; + } ` const ExpandMapButton = styled(Button)` @@ -81,9 +36,7 @@ const ExpandMapButton = styled(Button)` class BatchMobileResultsScreen extends Component { static propTypes = { - query: PropTypes.object, - resultCount: PropTypes.number, - setMobileScreen: PropTypes.func + error: PropTypes.object } constructor () { @@ -94,13 +47,6 @@ class BatchMobileResultsScreen extends Component { } } - componentDidMount () { - // Get the target element that we want to persist scrolling for - // FIXME Do we need to add something that removes the listeners when - // component unmounts? - coreUtils.ui.enableScrollForSelector('.mobile-narrative-container') - } - componentDidUpdate (prevProps) { // Check if the active leg changed if (this.props.activeLeg !== prevProps.activeLeg) { @@ -112,111 +58,34 @@ class BatchMobileResultsScreen extends Component { this.setState({ itineraryExpanded }) } - _editSearchClicked = () => { - this.props.clearActiveSearch() - this.props.setMobileScreen(MobileScreens.SEARCH_FORM) - } - _toggleMapExpanded = () => { this.setState({ mapExpanded: !this.state.mapExpanded }) } - renderError = () => { - const { error } = this.props - - return ( - - - {this.renderLocationsSummary()} -
-
- -
- -
-
-
- ) - } - - renderLocationsSummary = () => { - const { query } = this.props - - return ( - - - - - { query.from ? query.from.name : '' } - - - { query.to ? query.to.name : '' } - - - - - - - - ) - } - renderMap () { const { mapExpanded } = this.state return ( - - {/* Extra container for positioning the expand map button relative to the map. - (ResultsMap above has position =) - */} - - - - - {mapExpanded ? 'Show results' : 'Expand map'} - - - +
+ + + + {mapExpanded ? 'Show results' : 'Expand map'} + +
) } render () { - const { - error, - resultCount - } = this.props + const { error } = this.props const { itineraryExpanded, mapExpanded } = this.state - const narrativeContainerStyle = itineraryExpanded - ? { flexGrow: 1, overflowY: 'auto', padding: 0, position: 'unset' } - : { flex: '0 0 60%', overflowY: 'auto', padding: 0, position: 'unset' } - - // Ensure that narrative covers map. - narrativeContainerStyle.backgroundColor = 'white' - - if (error) { - return this.renderError() - } - return ( - 1 ? 's' : ''}` - : 'Waiting...' - } - /> - - {this.renderLocationsSummary()} - - {mapExpanded + + {!error && (mapExpanded // Set up two separate renderings of the map according to map expanded state, // so that it is properly sized and itineraries fit under either conditions. // (Otherwise, if just the narrative is added/removed, the map doesn't resize properly.) @@ -224,11 +93,7 @@ class BatchMobileResultsScreen extends Component { : ( <> {!itineraryExpanded && this.renderMap()} -
+
- )} + )) + } ) } @@ -249,28 +115,9 @@ class BatchMobileResultsScreen extends Component { // connect to the redux store const mapStateToProps = (state, ownProps) => { - const activeSearch = getActiveSearch(state.otp) - const {useRealtime} = state.otp - const response = !activeSearch - ? null - : useRealtime ? activeSearch.response : activeSearch.nonRealtimeResponse - - const itineraries = getActiveItineraries(state.otp) return { - error: getActiveError(state.otp), - query: state.otp.currentQuery, - resultCount: - response - ? activeSearch.query.routingType === 'ITINERARY' - ? itineraries.length - : response.otp.profile.length - : null + error: getActiveError(state.otp) } } -const mapDispatchToProps = { - clearActiveSearch, - setMobileScreen -} - -export default connect(mapStateToProps, mapDispatchToProps)(BatchMobileResultsScreen) +export default connect(mapStateToProps)(BatchMobileResultsScreen) diff --git a/lib/components/mobile/results-header-and-error.js b/lib/components/mobile/results-header-and-error.js new file mode 100644 index 000000000..1f05c0e23 --- /dev/null +++ b/lib/components/mobile/results-header-and-error.js @@ -0,0 +1,142 @@ +import LocationIcon from '@opentripplanner/location-icon' +import PropTypes from 'prop-types' +import React, { Component } from 'react' +import { Button, Col, Row } from 'react-bootstrap' +import { connect } from 'react-redux' +import styled from 'styled-components' + +import Map from '../map/map' +import ErrorMessage from '../form/error-message' +import { clearActiveSearch } from '../../actions/form' +import { MobileScreens, setMobileScreen } from '../../actions/ui' +import { + getActiveError, + getActiveItineraries, + getActiveSearch +} from '../../util/state' + +import MobileNavigationBar from './navigation-bar' + +const LocationContainer = styled.div` + font-weight: 300; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +` + +const LocationSummaryContainer = styled.div` + height: 50px; + left: 0; + padding-right: 10px; + position: fixed; + right: 0; + top: 50px; +` + +const LocationsSummaryColFromTo = styled(Col)` + font-size: 1.1em; + line-height: 1.2em; +` + +const LocationsSummaryRow = styled(Row)` + padding: 4px 8px; +` + +const StyledLocationIcon = styled(LocationIcon)` + margin: 3px; +` + +class ResultsHeaderAndError extends Component { + static propTypes = { + query: PropTypes.object, + resultCount: PropTypes.number, + setMobileScreen: PropTypes.func + } + + _editSearchClicked = () => { + this.props.clearActiveSearch() + this.props.setMobileScreen(MobileScreens.SEARCH_FORM) + } + + render () { + const { error, query, resultCount } = this.props + const headerText = error + ? 'No Trip Found' + : (resultCount + ? `We Found ${resultCount} Option${resultCount > 1 ? 's' : ''}` + : 'Waiting...' + ) + + return ( + <> + + + + + + + { query.from ? query.from.name : '' } + + + { query.to ? query.to.name : '' } + + + + + + + + + {error && ( + <> +
+
+ +
+ +
+
+ + )} + + ) + } +} + +// connect to the redux store + +const mapStateToProps = (state, ownProps) => { + const activeSearch = getActiveSearch(state.otp) + const {useRealtime} = state.otp + const response = !activeSearch + ? null + : useRealtime ? activeSearch.response : activeSearch.nonRealtimeResponse + + const itineraries = getActiveItineraries(state.otp) + return { + error: getActiveError(state.otp), + query: state.otp.currentQuery, + resultCount: + response + ? activeSearch.query.routingType === 'ITINERARY' + ? itineraries.length + : response.otp.profile.length + : null + } +} + +const mapDispatchToProps = { + clearActiveSearch, + setMobileScreen +} + +export default connect(mapStateToProps, mapDispatchToProps)(ResultsHeaderAndError) diff --git a/lib/components/mobile/results-screen.js b/lib/components/mobile/results-screen.js index 93bb9bb61..3ad65961d 100644 --- a/lib/components/mobile/results-screen.js +++ b/lib/components/mobile/results-screen.js @@ -1,21 +1,11 @@ import coreUtils from '@opentripplanner/core-utils' -import LocationIcon from '@opentripplanner/location-icon' import PropTypes from 'prop-types' import React, { Component } from 'react' -import { Button, Col, Row } from 'react-bootstrap' import { connect } from 'react-redux' -import styled from 'styled-components' import Map from '../map/map' -import ErrorMessage from '../form/error-message' import ItineraryCarousel from '../narrative/itinerary-carousel' - -import MobileContainer from './container' -import MobileNavigationBar from './navigation-bar' - -import { MobileScreens, setMobileScreen } from '../../actions/ui' import { setUseRealtimeResponse } from '../../actions/narrative' -import { clearActiveSearch } from '../../actions/form' import { getActiveError, getActiveItineraries, @@ -23,41 +13,18 @@ import { getRealtimeEffects } from '../../util/state' -const LocationContainer = styled.div` - font-weight: 300; - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; -` - -const LocationSummaryContainer = styled.div` - height: 50px; - left: 0; - padding-right: 10px; - position: fixed; - right: 0; - top: 50px; -` - -const LocationsSummaryColFromTo = styled(Col)` - font-size: 1.1em; - line-height: 1.2em; -` - -const LocationsSummaryRow = styled(Row)` - padding: 4px 8px; -` - -const StyledLocationIcon = styled(LocationIcon)` - margin: 3px; -` +import ResultsHeaderAndError from './results-header-and-error' +import MobileContainer from './container' class MobileResultsScreen extends Component { static propTypes = { activeItineraryIndex: PropTypes.number, + activeLeg: PropTypes.number, + error: PropTypes.object, query: PropTypes.object, + realtimeEffects: PropTypes.object, resultCount: PropTypes.number, - setMobileScreen: PropTypes.func + useRealtime: PropTypes.bool } constructor () { @@ -86,11 +53,6 @@ class MobileResultsScreen extends Component { this.refs['narrative-container'].scrollTop = 0 } - _editSearchClicked = () => { - this.props.clearActiveSearch() - this.props.setMobileScreen(MobileScreens.SEARCH_FORM) - } - _optionClicked = () => { this._setExpanded(!this.state.expanded) } @@ -107,8 +69,8 @@ class MobileResultsScreen extends Component { for (let i = 0; i < resultCount; i++) { dots.push(
) } @@ -118,57 +80,11 @@ class MobileResultsScreen extends Component { ) } - renderError = () => { - const { error } = this.props - - return ( - - - {this.renderLocationsSummary()} -
-
- -
- -
-
-
- ) - } - - renderLocationsSummary = () => { - const { query } = this.props - - return ( - - - - - { query.from ? query.from.name : '' } - - - { query.to ? query.to.name : '' } - - - - - - - - ) - } - render () { const { activeItineraryIndex, error, realtimeEffects, - resultCount, useRealtime } = this.props const { expanded } = this.state @@ -180,54 +96,45 @@ class MobileResultsScreen extends Component { // Ensure that narrative covers map. narrativeContainerStyle.backgroundColor = 'white' - let headerAction = null const showRealtimeAnnotation = realtimeEffects.isAffectedByRealtimeData && ( realtimeEffects.exceedsThreshold || realtimeEffects.routesDiffer || !useRealtime ) - if (error) { - return this.renderError() - } - return ( - 1 ? 's' : ''}` - : 'Waiting...' - } - headerAction={headerAction} - /> - {this.renderLocationsSummary()} - -
- -
- -
- Option {activeItineraryIndex + 1} - -
+ + {!error && ( + <> +
+ +
-
- -
- {this.renderDots()} + style={{ bottom: expanded ? null : 100, top: expanded ? 100 : null }} + > + Option {activeItineraryIndex + 1} + +
+ +
+ +
+ {this.renderDots()} + + )} ) } @@ -261,8 +168,6 @@ const mapStateToProps = (state, ownProps) => { } const mapDispatchToProps = { - clearActiveSearch, - setMobileScreen, setUseRealtimeResponse } From 50d8d23b62953c0d0bfe9eb0135d3616bb539459 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 5 Mar 2021 14:48:53 -0500 Subject: [PATCH 06/48] style(mobile/*results*): Tweak comments and sort props and imports. --- lib/components/mobile/batch-results-screen.js | 2 +- lib/components/mobile/results-header-and-error.js | 12 ++++++------ lib/components/mobile/results-screen.js | 14 +++++++------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/components/mobile/batch-results-screen.js b/lib/components/mobile/batch-results-screen.js index b26bd9ef0..2067894ca 100644 --- a/lib/components/mobile/batch-results-screen.js +++ b/lib/components/mobile/batch-results-screen.js @@ -86,7 +86,7 @@ class BatchMobileResultsScreen extends Component { {!error && (mapExpanded - // Set up two separate renderings of the map according to map expanded state, + // Set up two separate renderings of the map according mapExpanded, // so that it is properly sized and itineraries fit under either conditions. // (Otherwise, if just the narrative is added/removed, the map doesn't resize properly.) ? this.renderMap() diff --git a/lib/components/mobile/results-header-and-error.js b/lib/components/mobile/results-header-and-error.js index 1f05c0e23..da6b4c78a 100644 --- a/lib/components/mobile/results-header-and-error.js +++ b/lib/components/mobile/results-header-and-error.js @@ -5,10 +5,10 @@ import { Button, Col, Row } from 'react-bootstrap' import { connect } from 'react-redux' import styled from 'styled-components' -import Map from '../map/map' +import * as formActions from '../../actions/form' +import * as uiActions from '../../actions/ui' import ErrorMessage from '../form/error-message' -import { clearActiveSearch } from '../../actions/form' -import { MobileScreens, setMobileScreen } from '../../actions/ui' +import Map from '../map/map' import { getActiveError, getActiveItineraries, @@ -55,7 +55,7 @@ class ResultsHeaderAndError extends Component { _editSearchClicked = () => { this.props.clearActiveSearch() - this.props.setMobileScreen(MobileScreens.SEARCH_FORM) + this.props.setMobileScreen(uiActions.MobileScreens.SEARCH_FORM) } render () { @@ -135,8 +135,8 @@ const mapStateToProps = (state, ownProps) => { } const mapDispatchToProps = { - clearActiveSearch, - setMobileScreen + clearActiveSearch: formActions.clearActiveSearch, + setMobileScreen: uiActions.setMobileScreen } export default connect(mapStateToProps, mapDispatchToProps)(ResultsHeaderAndError) diff --git a/lib/components/mobile/results-screen.js b/lib/components/mobile/results-screen.js index 3ad65961d..b65db3215 100644 --- a/lib/components/mobile/results-screen.js +++ b/lib/components/mobile/results-screen.js @@ -3,9 +3,9 @@ import PropTypes from 'prop-types' import React, { Component } from 'react' import { connect } from 'react-redux' +import * as narrativeActions from '../../actions/narrative' import Map from '../map/map' import ItineraryCarousel from '../narrative/itinerary-carousel' -import { setUseRealtimeResponse } from '../../actions/narrative' import { getActiveError, getActiveItineraries, @@ -13,8 +13,8 @@ import { getRealtimeEffects } from '../../util/state' -import ResultsHeaderAndError from './results-header-and-error' import MobileContainer from './container' +import ResultsHeaderAndError from './results-header-and-error' class MobileResultsScreen extends Component { static propTypes = { @@ -152,23 +152,23 @@ const mapStateToProps = (state, ownProps) => { const realtimeEffects = getRealtimeEffects(state.otp) const itineraries = getActiveItineraries(state.otp) return { + activeItineraryIndex: activeSearch ? activeSearch.activeItinerary : null, + activeLeg: activeSearch ? activeSearch.activeLeg : null, + error: getActiveError(state.otp), query: state.otp.currentQuery, realtimeEffects, - error: getActiveError(state.otp), resultCount: response ? activeSearch.query.routingType === 'ITINERARY' ? itineraries.length : response.otp.profile.length : null, - useRealtime, - activeItineraryIndex: activeSearch ? activeSearch.activeItinerary : null, - activeLeg: activeSearch ? activeSearch.activeLeg : null + useRealtime } } const mapDispatchToProps = { - setUseRealtimeResponse + setUseRealtimeResponse: narrativeActions.setUseRealtimeResponse } export default connect(mapStateToProps, mapDispatchToProps)(MobileResultsScreen) From e802be61940f6d6454d6c6291b61e310bdf9ae21 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 12 Mar 2021 10:27:17 -0500 Subject: [PATCH 07/48] refactor(batch results): Mention component is required, fix typo. --- example.js | 1 + lib/components/mobile/batch-results-screen.js | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/example.js b/example.js index 8bc805c1b..57355139d 100644 --- a/example.js +++ b/example.js @@ -60,6 +60,7 @@ if (useCustomIcons) { // - MainControls (optional) // - MainPanel (required) // - MapWindows (optional) +// - MobileResultsScreen (required) // - MobileSearchScreen (required) // - ModeIcon (required) const components = { diff --git a/lib/components/mobile/batch-results-screen.js b/lib/components/mobile/batch-results-screen.js index 2067894ca..20cb9fa52 100644 --- a/lib/components/mobile/batch-results-screen.js +++ b/lib/components/mobile/batch-results-screen.js @@ -86,8 +86,8 @@ class BatchMobileResultsScreen extends Component { {!error && (mapExpanded - // Set up two separate renderings of the map according mapExpanded, - // so that it is properly sized and itineraries fit under either conditions. + // Set up two separate renderings of the map according to mapExpanded, + // so that the map is properly sized and itineraries fit under either conditions. // (Otherwise, if just the narrative is added/removed, the map doesn't resize properly.) ? this.renderMap() : ( From c10d7ca6b05b877a62cce375389246a2f2d3a5ea Mon Sep 17 00:00:00 2001 From: Rob Gregg Date: Thu, 11 Mar 2021 15:15:33 +0000 Subject: [PATCH 08/48] fix(narrative-itineraries.js, default-itinerary.js): Added setActiveLeg as a prop to ItineraryBody c Both files were not passing setActiveLeg as a prop so I imported where necessary and passed in the prop. fix #338 --- lib/components/narrative/default/default-itinerary.js | 4 +++- lib/components/narrative/narrative-itineraries.js | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/components/narrative/default/default-itinerary.js b/lib/components/narrative/default/default-itinerary.js index 36170108e..bd72789bc 100644 --- a/lib/components/narrative/default/default-itinerary.js +++ b/lib/components/narrative/default/default-itinerary.js @@ -6,6 +6,7 @@ import ItineraryBody from '../line-itin/connected-itinerary-body' import ItinerarySummary from './itinerary-summary' import SimpleRealtimeAnnotation from '../simple-realtime-annotation' import { getTotalFareAsString } from '../../../util/state' +import { setActiveLeg } from "../../../actions/narrative"; const { isBicycle, isTransit } = coreUtils.itinerary const { formatDuration, formatTime } = coreUtils.time @@ -121,6 +122,7 @@ export default class DefaultItinerary extends NarrativeItinerary { expanded, itinerary, LegIcon, + setActiveLeg, showRealtimeAnnotation, timeFormat } = this.props @@ -184,7 +186,7 @@ export default class DefaultItinerary extends NarrativeItinerary { {(active && expanded) && <> {showRealtimeAnnotation && } - + }
diff --git a/lib/components/narrative/narrative-itineraries.js b/lib/components/narrative/narrative-itineraries.js index 219d0aaba..ce5377d3d 100644 --- a/lib/components/narrative/narrative-itineraries.js +++ b/lib/components/narrative/narrative-itineraries.js @@ -15,11 +15,12 @@ import { import Icon from '../narrative/icon' import { ComponentContext } from '../../util/contexts' import { - getResponsesWithErrors, getActiveItineraries, getActiveSearch, - getRealtimeEffects + getRealtimeEffects, + getResponsesWithErrors } from '../../util/state' + import SaveTripButton from './save-trip-button' // TODO: move to utils? @@ -196,6 +197,7 @@ class NarrativeItineraries extends Component { itinerary={itinerary} key={index} LegIcon={LegIcon} + setActiveLeg={setActiveLeg} onClick={active ? this._toggleDetailedItinerary : undefined} routingType='ITINERARY' showRealtimeAnnotation={showRealtimeAnnotation} From edea628d0568bf4d3c6f753241acdf89c5be9edd Mon Sep 17 00:00:00 2001 From: Rob Gregg Date: Thu, 11 Mar 2021 17:19:11 +0000 Subject: [PATCH 09/48] fix(default-itinerary.js): Removed some unused code. The previous commit had a line of unused code. 338 --- lib/components/narrative/default/default-itinerary.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/components/narrative/default/default-itinerary.js b/lib/components/narrative/default/default-itinerary.js index bd72789bc..abd1eac24 100644 --- a/lib/components/narrative/default/default-itinerary.js +++ b/lib/components/narrative/default/default-itinerary.js @@ -6,7 +6,6 @@ import ItineraryBody from '../line-itin/connected-itinerary-body' import ItinerarySummary from './itinerary-summary' import SimpleRealtimeAnnotation from '../simple-realtime-annotation' import { getTotalFareAsString } from '../../../util/state' -import { setActiveLeg } from "../../../actions/narrative"; const { isBicycle, isTransit } = coreUtils.itinerary const { formatDuration, formatTime } = coreUtils.time From 100c1d62160bc53210f50732875aa911ec4ed982 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 23 Mar 2021 12:05:43 -0400 Subject: [PATCH 10/48] fix(BatchResultsScreen): Fix leg focus, address some PR comments. --- lib/components/mobile/batch-results-screen.js | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/components/mobile/batch-results-screen.js b/lib/components/mobile/batch-results-screen.js index 20cb9fa52..fce24aa04 100644 --- a/lib/components/mobile/batch-results-screen.js +++ b/lib/components/mobile/batch-results-screen.js @@ -7,7 +7,7 @@ import styled from 'styled-components' import Map from '../map/map' import Icon from '../narrative/icon' import NarrativeItineraries from '../narrative/narrative-itineraries' -import { getActiveError } from '../../util/state' +import { getActiveError, getActiveSearch } from '../../util/state' import MobileContainer from './container' import ResultsHeaderAndError from './results-header-and-error' @@ -28,12 +28,15 @@ const StyledMobileContainer = styled(MobileContainer)` ` const ExpandMapButton = styled(Button)` - bottom: 25px; + bottom: 10px; + left: 10px; position: absolute; - right: 10px; z-index: 999999; ` +/** + * This component renders the mobile view of itinerary results from batch routing. + */ class BatchMobileResultsScreen extends Component { static propTypes = { error: PropTypes.object @@ -48,8 +51,10 @@ class BatchMobileResultsScreen extends Component { } componentDidUpdate (prevProps) { - // Check if the active leg changed if (this.props.activeLeg !== prevProps.activeLeg) { + // Check if the active leg has changed. If a different leg is selected, + // unexpand the itinerary to show the map focused on the selected leg + // (similar to the behavior of LineItinerary). this._setItineraryExpanded(false) } } @@ -71,7 +76,7 @@ class BatchMobileResultsScreen extends Component { bsSize='small' onClick={this._toggleMapExpanded} > - + {' '} {mapExpanded ? 'Show results' : 'Expand map'}
@@ -115,7 +120,9 @@ class BatchMobileResultsScreen extends Component { // connect to the redux store const mapStateToProps = (state, ownProps) => { + const activeSearch = getActiveSearch(state.otp) return { + activeLeg: activeSearch ? activeSearch.activeLeg : null, error: getActiveError(state.otp) } } From df664e04cae78655b5e26907567b2b81004ebd72 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 23 Mar 2021 15:31:56 -0400 Subject: [PATCH 11/48] refactor(BatchResultsScreen): Show result screen with correct #itins if errors. --- lib/components/mobile/batch-results-screen.js | 22 ++++-------- lib/components/mobile/main.js | 36 +++++++++++++------ .../mobile/results-header-and-error.js | 24 ++++++------- 3 files changed, 44 insertions(+), 38 deletions(-) diff --git a/lib/components/mobile/batch-results-screen.js b/lib/components/mobile/batch-results-screen.js index fce24aa04..b22fd4b10 100644 --- a/lib/components/mobile/batch-results-screen.js +++ b/lib/components/mobile/batch-results-screen.js @@ -38,16 +38,9 @@ const ExpandMapButton = styled(Button)` * This component renders the mobile view of itinerary results from batch routing. */ class BatchMobileResultsScreen extends Component { - static propTypes = { - error: PropTypes.object - } - - constructor () { - super() - this.state = { - itineraryExpanded: false, - mapExpanded: false - } + state = { + itineraryExpanded: false, + mapExpanded: false } componentDidUpdate (prevProps) { @@ -84,13 +77,12 @@ class BatchMobileResultsScreen extends Component { } render () { - const { error } = this.props const { itineraryExpanded, mapExpanded } = this.state return ( - {!error && (mapExpanded + {mapExpanded // Set up two separate renderings of the map according to mapExpanded, // so that the map is properly sized and itineraries fit under either conditions. // (Otherwise, if just the narrative is added/removed, the map doesn't resize properly.) @@ -101,6 +93,7 @@ class BatchMobileResultsScreen extends Component {
- )) + ) }
) @@ -122,8 +115,7 @@ class BatchMobileResultsScreen extends Component { const mapStateToProps = (state, ownProps) => { const activeSearch = getActiveSearch(state.otp) return { - activeLeg: activeSearch ? activeSearch.activeLeg : null, - error: getActiveError(state.otp) + activeLeg: activeSearch ? activeSearch.activeLeg : null } } diff --git a/lib/components/mobile/main.js b/lib/components/mobile/main.js index db28eafd4..805615a41 100644 --- a/lib/components/mobile/main.js +++ b/lib/components/mobile/main.js @@ -12,11 +12,13 @@ import MobileRouteViewer from './route-viewer' import { MobileScreens, MainPanelContent, setMobileScreen } from '../../actions/ui' import { ComponentContext } from '../../util/contexts' -import { getActiveItinerary } from '../../util/state' +import { getActiveItinerary, getResponsesWithErrors } from '../../util/state' class MobileMain extends Component { static propTypes = { + currentPosition: PropTypes.object, currentQuery: PropTypes.object, + errors: PropTypes.array, map: PropTypes.element, setMobileScreen: PropTypes.func, title: PropTypes.element, @@ -26,20 +28,30 @@ class MobileMain extends Component { static contextType = ComponentContext componentDidUpdate (prevProps) { + const { + activeItinerary, + currentPosition, + currentQuery, + errors, + setMobileScreen + } = this.props + // Check if we are in the welcome screen and both locations have been set OR // auto-detect is denied and one location is set if ( prevProps.uiState.mobileScreen === MobileScreens.WELCOME_SCREEN && ( - (this.props.currentQuery.from && this.props.currentQuery.to) || - (!this.props.currentPosition.coords && (this.props.currentQuery.from || this.props.currentQuery.to)) + (currentQuery.from && currentQuery.to) || + (!currentPosition.coords && (currentQuery.from || currentQuery.to)) ) ) { // If so, advance to main search screen - this.props.setMobileScreen(MobileScreens.SEARCH_FORM) + setMobileScreen(MobileScreens.SEARCH_FORM) } - if (!prevProps.activeItinerary && this.props.activeItinerary) { - this.props.setMobileScreen(MobileScreens.RESULTS_SUMMARY) + // Display the results screen if results have been returned + // (and an active itinerary has been set) or if there are errors. + if (!prevProps.activeItinerary && (activeItinerary || errors.length > 0)) { + setMobileScreen(MobileScreens.RESULTS_SUMMARY) } } @@ -110,12 +122,14 @@ class MobileMain extends Component { // connect to the redux store const mapStateToProps = (state, ownProps) => { + const { config, currentQuery, location, ui } = state.otp return { - config: state.otp.config, - uiState: state.otp.ui, - currentQuery: state.otp.currentQuery, - currentPosition: state.otp.location.currentPosition, - activeItinerary: getActiveItinerary(state.otp) + activeItinerary: getActiveItinerary(state.otp), + config, + currentPosition: location.currentPosition, + currentQuery, + errors: getResponsesWithErrors(state.otp), + uiState: ui } } diff --git a/lib/components/mobile/results-header-and-error.js b/lib/components/mobile/results-header-and-error.js index da6b4c78a..2a50db2c2 100644 --- a/lib/components/mobile/results-header-and-error.js +++ b/lib/components/mobile/results-header-and-error.js @@ -12,7 +12,8 @@ import Map from '../map/map' import { getActiveError, getActiveItineraries, - getActiveSearch + getActiveSearch, + getResponsesWithErrors } from '../../util/state' import MobileNavigationBar from './navigation-bar' @@ -48,6 +49,7 @@ const StyledLocationIcon = styled(LocationIcon)` class ResultsHeaderAndError extends Component { static propTypes = { + errors: PropTypes.array, query: PropTypes.object, resultCount: PropTypes.number, setMobileScreen: PropTypes.func @@ -59,8 +61,9 @@ class ResultsHeaderAndError extends Component { } render () { - const { error, query, resultCount } = this.props - const headerText = error + const { errors, query, resultCount } = this.props + const hasNoResult = resultCount === 0 && errors.length > 0 + const headerText = hasNoResult ? 'No Trip Found' : (resultCount ? `We Found ${resultCount} Option${resultCount > 1 ? 's' : ''}` @@ -90,11 +93,11 @@ class ResultsHeaderAndError extends Component { - {error && ( + {hasNoResult && ( <>
- +
+
+
+ ) + } +} + +// connect to the redux store + +const mapStateToProps = (state, ownProps) => { + return {} +} + +const mapDispatchToProps = { + clearActiveSearch: formActions.clearActiveSearch, + setMobileScreen: uiActions.setMobileScreen +} + +export default connect(mapStateToProps, mapDispatchToProps)(ResultsError) diff --git a/lib/components/mobile/results-header-and-error.js b/lib/components/mobile/results-header.js similarity index 80% rename from lib/components/mobile/results-header-and-error.js rename to lib/components/mobile/results-header.js index 2a50db2c2..9c498da3d 100644 --- a/lib/components/mobile/results-header-and-error.js +++ b/lib/components/mobile/results-header.js @@ -7,10 +7,7 @@ import styled from 'styled-components' import * as formActions from '../../actions/form' import * as uiActions from '../../actions/ui' -import ErrorMessage from '../form/error-message' -import Map from '../map/map' import { - getActiveError, getActiveItineraries, getActiveSearch, getResponsesWithErrors @@ -47,7 +44,11 @@ const StyledLocationIcon = styled(LocationIcon)` margin: 3px; ` -class ResultsHeaderAndError extends Component { +/** + * This component renders the results header and an error message + * if no itinerary was found. + */ +class ResultsHeader extends Component { static propTypes = { errors: PropTypes.array, query: PropTypes.object, @@ -92,24 +93,6 @@ class ResultsHeaderAndError extends Component { - - {hasNoResult && ( - <> -
-
- -
- -
-
- - )} ) } @@ -139,4 +122,4 @@ const mapDispatchToProps = { setMobileScreen: uiActions.setMobileScreen } -export default connect(mapStateToProps, mapDispatchToProps)(ResultsHeaderAndError) +export default connect(mapStateToProps, mapDispatchToProps)(ResultsHeader) diff --git a/lib/components/mobile/results-screen.js b/lib/components/mobile/results-screen.js index b65db3215..9ff918b83 100644 --- a/lib/components/mobile/results-screen.js +++ b/lib/components/mobile/results-screen.js @@ -14,7 +14,8 @@ import { } from '../../util/state' import MobileContainer from './container' -import ResultsHeaderAndError from './results-header-and-error' +import ResultsError from './results-error' +import ResultsHeader from './results-header' class MobileResultsScreen extends Component { static propTypes = { @@ -104,37 +105,39 @@ class MobileResultsScreen extends Component { return ( - - {!error && ( - <> -
- -
- -
- Option {activeItineraryIndex + 1} - -
- -
- -
- {this.renderDots()} - - )} + +
+ +
+ {error + ? + : ( + <> +
+ Option {activeItineraryIndex + 1} + +
+ +
+ +
+ {this.renderDots()} + + ) + }
) } From d615fd8acd12952e040676e53ec2e10013013b01 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 23 Mar 2021 17:49:05 -0400 Subject: [PATCH 14/48] style(ResultsScreen): Commit indents --- lib/components/mobile/batch-results-screen.js | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/lib/components/mobile/batch-results-screen.js b/lib/components/mobile/batch-results-screen.js index 80b377730..f239b6f3a 100644 --- a/lib/components/mobile/batch-results-screen.js +++ b/lib/components/mobile/batch-results-screen.js @@ -1,4 +1,3 @@ -import PropTypes from 'prop-types' import React, { Component } from 'react' import { Button } from 'react-bootstrap' import { connect } from 'react-redux' @@ -11,7 +10,7 @@ import { getActiveItineraries, getActiveSearch, getResponsesWithErrors - } from '../../util/state' +} from '../../util/state' import MobileContainer from './container' import ResultsError from './results-error' @@ -100,19 +99,19 @@ class BatchMobileResultsScreen extends Component { {hasNoResult ? : ( -
- -
- ) +
+ +
+ ) } ) From 22beb2cc5596033f8e308ae9e54271aa51103daa Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 23 Mar 2021 17:56:39 -0400 Subject: [PATCH 15/48] style(mobile/main): Fix indent --- lib/components/mobile/main.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/components/mobile/main.js b/lib/components/mobile/main.js index fb8bf5278..4027f295e 100644 --- a/lib/components/mobile/main.js +++ b/lib/components/mobile/main.js @@ -14,7 +14,7 @@ import * as uiActions from '../../actions/ui' import { ComponentContext } from '../../util/contexts' import { getActiveSearch } from '../../util/state' -const { MobileScreens, MainPanelContent } = uiActions +const { MainPanelContent, MobileScreens } = uiActions class MobileMain extends Component { static propTypes = { @@ -33,7 +33,7 @@ class MobileMain extends Component { currentPosition, currentQuery, setMobileScreen - } = this.props + } = this.props // Check if we are in the welcome screen and both locations have been set OR // auto-detect is denied and one location is set From 15329feb4bfef8f2aa9e75faa63ba87b0e39d55b Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 24 Mar 2021 12:28:37 -0400 Subject: [PATCH 16/48] refactor(BoundsUpdatingOverlay): Remove fit bounds mobile restriction. --- lib/components/map/bounds-updating-overlay.js | 35 +++++++------------ 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/lib/components/map/bounds-updating-overlay.js b/lib/components/map/bounds-updating-overlay.js index 230ae68c8..72e8695ad 100644 --- a/lib/components/map/bounds-updating-overlay.js +++ b/lib/components/map/bounds-updating-overlay.js @@ -87,29 +87,18 @@ class BoundsUpdatingOverlay extends MapLayer { // If no itinerary update but from/to locations are present, fit to those } else if (newFrom && newTo && (fromChanged || toChanged)) { - // On certain mobile devices (e.g., Android + Chrome), setting from and to - // locations via the location search component causes issues for this - // fitBounds invocation. The map does not appear to be visible when these - // prop changes are detected, so for now we should perhaps just skip this - // fitBounds on mobile. - // See https://github.com/opentripplanner/otp-react-redux/issues/133 for - // more info. - // TODO: Fix this so mobile devices will also update the bounds to the - // from/to locations. - if (!coreUtils.ui.isMobile()) { - const bounds = L.bounds([ - [newFrom.lat, newFrom.lon], - [newTo.lat, newTo.lon] - ]) - // Ensure bounds extend to include intermediatePlaces - extendBoundsByPlaces(bounds, newIntermediate) - const { x: left, y: bottom } = bounds.getBottomLeft() - const { x: right, y: top } = bounds.getTopRight() - map.fitBounds([ - [left, bottom], - [right, top] - ], { padding }) - } + const bounds = L.bounds([ + [newFrom.lat, newFrom.lon], + [newTo.lat, newTo.lon] + ]) + // Ensure bounds extend to include intermediatePlaces + extendBoundsByPlaces(bounds, newIntermediate) + const { x: left, y: bottom } = bounds.getBottomLeft() + const { x: right, y: top } = bounds.getTopRight() + map.fitBounds([ + [left, bottom], + [right, top] + ], { padding }) // If only from or to is set, pan to that } else if (newFrom && fromChanged) { From fc76d3ec47dc0ef419ee8a2e9e990aa8d1a6b1b0 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 24 Mar 2021 16:00:45 -0400 Subject: [PATCH 17/48] refactor(BatchResultsScreen): Investigate imperative resizing. --- lib/components/map/bounds-updating-overlay.js | 21 ++++++ .../map/connected-transitive-overlay.js | 5 ++ lib/components/map/default-map.js | 5 +- lib/components/map/map.js | 7 +- lib/components/mobile/batch-results-screen.js | 64 ++++++++++++++----- 5 files changed, 80 insertions(+), 22 deletions(-) diff --git a/lib/components/map/bounds-updating-overlay.js b/lib/components/map/bounds-updating-overlay.js index 72e8695ad..083547d2e 100644 --- a/lib/components/map/bounds-updating-overlay.js +++ b/lib/components/map/bounds-updating-overlay.js @@ -69,6 +69,10 @@ class BoundsUpdatingOverlay extends MapLayer { const oldIntermediate = oldProps.query && oldProps.query.intermediatePlaces const newIntermediate = newProps.query && newProps.query.intermediatePlaces const intermediateChanged = !isEqual(oldIntermediate, newIntermediate) + + // Also refit map if stateData prop has changed + const stateDataChanged = !isEqual(oldProps.stateData, newProps.stateData) + if ( (!oldItinBounds && newItinBounds) || (oldItinBounds && newItinBounds && !oldItinBounds.equals(newItinBounds)) @@ -122,6 +126,23 @@ class BoundsUpdatingOverlay extends MapLayer { const leg = newProps.itinerary.legs[newProps.activeLeg] const step = leg.steps[newProps.activeStep] map.panTo([step.lat, step.lon]) + } else if (stateDataChanged) { + // If a new stateData prop is passed, + // force a resize of the map, then refit the active itinerary or active leg. + map.invalidateSize({ animate: true, debounceMoveEnd: true }) + + if ( + newProps.itinerary && + newProps.activeLeg !== null + ) { + // Fit to active leg if set. + map.fitBounds( + getLeafletLegBounds(newProps.itinerary.legs[newProps.activeLeg]), + { padding } + ) + } else { + map.fitBounds(newItinBounds, { padding }) + } } } } diff --git a/lib/components/map/connected-transitive-overlay.js b/lib/components/map/connected-transitive-overlay.js index 9b2eed104..fbd5c7cf2 100644 --- a/lib/components/map/connected-transitive-overlay.js +++ b/lib/components/map/connected-transitive-overlay.js @@ -31,6 +31,11 @@ const mapStateToProps = (state, ownProps) => { transitiveData = activeSearch.response.otp } + if (transitiveData) { + // HACK: pass a prop change from map to transitive to force a render. + transitiveData.stateData = ownProps.stateData + } + return { activeItinerary: activeSearch && activeSearch.activeItinerary, routingType: activeSearch && activeSearch.query && activeSearch.query.routingType, diff --git a/lib/components/map/default-map.js b/lib/components/map/default-map.js index 3ce357508..bba144d83 100644 --- a/lib/components/map/default-map.js +++ b/lib/components/map/default-map.js @@ -121,6 +121,7 @@ class DefaultMap extends Component { carRentalStations, mapConfig, mapPopupLocation, + stateData, vehicleRentalQuery, vehicleRentalStations } = this.props @@ -151,11 +152,11 @@ class DefaultMap extends Component { zoom={mapConfig.initZoom || 13} > {/* The default overlays */} - + - + diff --git a/lib/components/map/map.js b/lib/components/map/map.js index c3474437d..2eaf50b52 100644 --- a/lib/components/map/map.js +++ b/lib/components/map/map.js @@ -14,11 +14,12 @@ class Map extends Component { } } - getComponentForView (view) { + getComponentForView = view => { + const { stateData } = this.props // TODO: allow a 'CUSTOM' type switch (view.type) { - case 'DEFAULT': return - case 'STYLIZED': return + case 'DEFAULT': return + case 'STYLIZED': return } } diff --git a/lib/components/mobile/batch-results-screen.js b/lib/components/mobile/batch-results-screen.js index f239b6f3a..e5a9b0056 100644 --- a/lib/components/mobile/batch-results-screen.js +++ b/lib/components/mobile/batch-results-screen.js @@ -84,37 +84,67 @@ class BatchMobileResultsScreen extends Component { const { errors, itineraries } = this.props const hasNoResult = itineraries.length === 0 && errors.length > 0 const { itineraryExpanded, mapExpanded } = this.state - +/* + const narrative = ( +
+ +
+ ) +*/ return ( - {mapExpanded + {/*mapExpanded // Set up two separate renderings of the map according to mapExpanded, // so that the map is properly sized and itineraries fit under either conditions. // (Otherwise, if just the narrative is added/removed, the map doesn't resize properly.) - ? this.renderMap() + ? <>{this.renderMap()} : ( <> {!itineraryExpanded && this.renderMap()} {hasNoResult ? - : ( -
- -
- ) + : narrative } ) + */} + +
+ + + {' '} + {mapExpanded ? 'Show results' : 'Expand map'} + +
+ {hasNoResult + ? + : ( +
+ +
+ ) }
) From 2e7757c9365880a36f3bb154f1beaa1fb3d16556 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 24 Mar 2021 16:21:05 -0400 Subject: [PATCH 18/48] refactor(map/default-map): Use key prop to force transitive remount on resize. --- lib/components/map/connected-transitive-overlay.js | 5 ----- lib/components/map/default-map.js | 9 ++++++++- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/components/map/connected-transitive-overlay.js b/lib/components/map/connected-transitive-overlay.js index fbd5c7cf2..9b2eed104 100644 --- a/lib/components/map/connected-transitive-overlay.js +++ b/lib/components/map/connected-transitive-overlay.js @@ -31,11 +31,6 @@ const mapStateToProps = (state, ownProps) => { transitiveData = activeSearch.response.otp } - if (transitiveData) { - // HACK: pass a prop change from map to transitive to force a render. - transitiveData.stateData = ownProps.stateData - } - return { activeItinerary: activeSearch && activeSearch.activeItinerary, routingType: activeSearch && activeSearch.query && activeSearch.query.routingType, diff --git a/lib/components/map/default-map.js b/lib/components/map/default-map.js index bba144d83..eb13ba0cc 100644 --- a/lib/components/map/default-map.js +++ b/lib/components/map/default-map.js @@ -156,7 +156,14 @@ class DefaultMap extends Component { - + {/* + HACK: Use the key prop to force a remount and full resizing of transitive + if the map container size changes, + per https://linguinecode.com/post/4-methods-to-re-render-react-component + Without it, transitive resolution will not match the map, + and transitive will appear blurry after e.g. the narrative is expanded. + */} + From 557bfc7387d771ce1a21c3c36fe7e0f65a1e28c4 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 24 Mar 2021 17:03:02 -0400 Subject: [PATCH 19/48] refactor(BatchResultsScreen): Refactor, tweak split height. --- lib/components/mobile/batch-results-screen.js | 63 ++++--------------- 1 file changed, 13 insertions(+), 50 deletions(-) diff --git a/lib/components/mobile/batch-results-screen.js b/lib/components/mobile/batch-results-screen.js index e5a9b0056..9e99aa550 100644 --- a/lib/components/mobile/batch-results-screen.js +++ b/lib/components/mobile/batch-results-screen.js @@ -38,6 +38,8 @@ const ExpandMapButton = styled(Button)` z-index: 999999; ` +const NARRATIVE_SPLIT_TOP_PERCENT = 45 + /** * This component renders the mobile view of itinerary results from batch routing. */ @@ -64,62 +66,20 @@ class BatchMobileResultsScreen extends Component { this.setState({ mapExpanded: !this.state.mapExpanded }) } - renderMap () { - const { mapExpanded } = this.state - return ( -
- - - {' '} - {mapExpanded ? 'Show results' : 'Expand map'} - -
- ) - } - render () { const { errors, itineraries } = this.props const hasNoResult = itineraries.length === 0 && errors.length > 0 const { itineraryExpanded, mapExpanded } = this.state -/* - const narrative = ( -
- -
- ) -*/ + const narrativeTop = mapExpanded ? '100%' : (itineraryExpanded ? '100px' : `${NARRATIVE_SPLIT_TOP_PERCENT}%`) + const mapBottom = mapExpanded ? 0 : `${100 - NARRATIVE_SPLIT_TOP_PERCENT}%` + return ( - {/*mapExpanded - // Set up two separate renderings of the map according to mapExpanded, - // so that the map is properly sized and itineraries fit under either conditions. - // (Otherwise, if just the narrative is added/removed, the map doesn't resize properly.) - ? <>{this.renderMap()} - : ( - <> - {!itineraryExpanded && this.renderMap()} - {hasNoResult - ? - : narrative - } - - ) - */} - -
+
: ( -
+
Date: Wed, 24 Mar 2021 18:20:10 -0400 Subject: [PATCH 20/48] refactor(BatchResultScreen): Move itin state to URL param, remove stateData prop. --- lib/actions/ui.js | 24 +++++++++++++-- lib/components/map/bounds-updating-overlay.js | 13 +++++---- lib/components/map/default-map.js | 12 +++++--- lib/components/map/map.js | 5 ++-- lib/components/mobile/batch-results-screen.js | 29 +++++++++++-------- 5 files changed, 57 insertions(+), 26 deletions(-) diff --git a/lib/actions/ui.js b/lib/actions/ui.js index 27cca104a..c24a5a324 100644 --- a/lib/actions/ui.js +++ b/lib/actions/ui.js @@ -3,9 +3,14 @@ import coreUtils from '@opentripplanner/core-utils' import { createAction } from 'redux-actions' import { matchPath } from 'react-router' -import { findRoute } from './api' +import { findRoute, setUrlSearch } from './api' import { setMapCenter, setMapZoom, setRouterId } from './config' -import { clearActiveSearch, parseUrlQueryString, setActiveSearch } from './form' +import { + clearActiveSearch, + parseUrlQueryString, + setActiveSearch, + settingQueryParam +} from './form' import { clearLocation } from './map' import { setActiveItinerary } from './narrative' import { getUiUrlParams } from '../util/state' @@ -145,6 +150,21 @@ export function handleBackButtonPress (e) { } } +/** + * Sets the itinerary view state ('full', 'split', or 'hidden') in: + * - the currentQuery otp redux state, + * - in the URL params. + */ +export function setItineraryView (value) { + return function (dispatch, getState) { + dispatch(settingQueryParam({ ui_itineraryView: value })) + + const urlParams = coreUtils.query.getUrlParams() + urlParams.ui_itineraryView = value + dispatch(setUrlSearch(urlParams)) + } +} + export const setMobileScreen = createAction('SET_MOBILE_SCREEN') /** diff --git a/lib/components/map/bounds-updating-overlay.js b/lib/components/map/bounds-updating-overlay.js index 083547d2e..6f2f550ae 100644 --- a/lib/components/map/bounds-updating-overlay.js +++ b/lib/components/map/bounds-updating-overlay.js @@ -70,8 +70,8 @@ class BoundsUpdatingOverlay extends MapLayer { const newIntermediate = newProps.query && newProps.query.intermediatePlaces const intermediateChanged = !isEqual(oldIntermediate, newIntermediate) - // Also refit map if stateData prop has changed - const stateDataChanged = !isEqual(oldProps.stateData, newProps.stateData) + // Also refit map if itineraryView prop has changed. + const itineraryViewChanged = oldProps.itineraryView !== newProps.itineraryView if ( (!oldItinBounds && newItinBounds) || @@ -126,10 +126,10 @@ class BoundsUpdatingOverlay extends MapLayer { const leg = newProps.itinerary.legs[newProps.activeLeg] const step = leg.steps[newProps.activeStep] map.panTo([step.lat, step.lon]) - } else if (stateDataChanged) { - // If a new stateData prop is passed, + } else if (itineraryViewChanged) { + // If itineraryView has changed, // force a resize of the map, then refit the active itinerary or active leg. - map.invalidateSize({ animate: true, debounceMoveEnd: true }) + map.invalidateSize(true) if ( newProps.itinerary && @@ -151,10 +151,13 @@ class BoundsUpdatingOverlay extends MapLayer { const mapStateToProps = (state, ownProps) => { const activeSearch = getActiveSearch(state.otp) + const urlParams = coreUtils.query.getUrlParams() + return { activeLeg: activeSearch && activeSearch.activeLeg, activeStep: activeSearch && activeSearch.activeStep, itinerary: getActiveItinerary(state.otp), + itineraryView: urlParams.ui_itineraryView, popupLocation: state.otp.ui.mapPopupLocation, query: state.otp.currentQuery } diff --git a/lib/components/map/default-map.js b/lib/components/map/default-map.js index eb13ba0cc..9a3157701 100644 --- a/lib/components/map/default-map.js +++ b/lib/components/map/default-map.js @@ -1,4 +1,5 @@ import BaseMap from '@opentripplanner/base-map' +import coreUtils from '@opentripplanner/core-utils' import React, { Component } from 'react' import { connect } from 'react-redux' import styled from 'styled-components' @@ -119,9 +120,9 @@ class DefaultMap extends Component { bikeRentalStations, carRentalQuery, carRentalStations, + itineraryView, mapConfig, mapPopupLocation, - stateData, vehicleRentalQuery, vehicleRentalStations } = this.props @@ -147,12 +148,12 @@ class DefaultMap extends Component { center={center} maxZoom={mapConfig.maxZoom} onClick={this.onMapClick} - popup={popup} onPopupClosed={this.onPopupClosed} + popup={popup} zoom={mapConfig.initZoom || 13} > {/* The default overlays */} - + @@ -163,7 +164,7 @@ class DefaultMap extends Component { Without it, transitive resolution will not match the map, and transitive will appear blurry after e.g. the narrative is expanded. */} - + @@ -214,9 +215,12 @@ const mapStateToProps = (state, ownProps) => { const overlays = state.otp.config.map && state.otp.config.map.overlays ? state.otp.config.map.overlays : [] + const urlParams = coreUtils.query.getUrlParams() + return { bikeRentalStations: state.otp.overlay.bikeRental.stations, carRentalStations: state.otp.overlay.carRental.stations, + itineraryView: urlParams.ui_itineraryView, mapConfig: state.otp.config.map, mapPopupLocation: state.otp.ui.mapPopupLocation, overlays, diff --git a/lib/components/map/map.js b/lib/components/map/map.js index 2eaf50b52..4edf47a0e 100644 --- a/lib/components/map/map.js +++ b/lib/components/map/map.js @@ -15,11 +15,10 @@ class Map extends Component { } getComponentForView = view => { - const { stateData } = this.props // TODO: allow a 'CUSTOM' type switch (view.type) { - case 'DEFAULT': return - case 'STYLIZED': return + case 'DEFAULT': return + case 'STYLIZED': return } } diff --git a/lib/components/mobile/batch-results-screen.js b/lib/components/mobile/batch-results-screen.js index 9e99aa550..592bf06ac 100644 --- a/lib/components/mobile/batch-results-screen.js +++ b/lib/components/mobile/batch-results-screen.js @@ -1,8 +1,10 @@ +import coreUtils from '@opentripplanner/core-utils' import React, { Component } from 'react' import { Button } from 'react-bootstrap' import { connect } from 'react-redux' import styled from 'styled-components' +import * as uiActions from '../../actions/ui' import Map from '../map/map' import Icon from '../narrative/icon' import NarrativeItineraries from '../narrative/narrative-itineraries' @@ -44,11 +46,6 @@ const NARRATIVE_SPLIT_TOP_PERCENT = 45 * This component renders the mobile view of itinerary results from batch routing. */ class BatchMobileResultsScreen extends Component { - state = { - itineraryExpanded: false, - mapExpanded: false - } - componentDidUpdate (prevProps) { if (this.props.activeLeg !== prevProps.activeLeg) { // Check if the active leg has changed. If a different leg is selected, @@ -59,17 +56,18 @@ class BatchMobileResultsScreen extends Component { } _setItineraryExpanded = itineraryExpanded => { - this.setState({ itineraryExpanded }) + this.props.setItineraryView(itineraryExpanded ? 'full' : 'split') } _toggleMapExpanded = () => { - this.setState({ mapExpanded: !this.state.mapExpanded }) + this.props.setItineraryView(this.props.itineraryView === 'hidden' ? 'split' : 'hidden') } render () { - const { errors, itineraries } = this.props + const { errors, itineraries, itineraryView } = this.props const hasNoResult = itineraries.length === 0 && errors.length > 0 - const { itineraryExpanded, mapExpanded } = this.state + const mapExpanded = itineraryView === 'hidden' + const itineraryExpanded = itineraryView === 'full' const narrativeTop = mapExpanded ? '100%' : (itineraryExpanded ? '100px' : `${NARRATIVE_SPLIT_TOP_PERCENT}%`) const mapBottom = mapExpanded ? 0 : `${100 - NARRATIVE_SPLIT_TOP_PERCENT}%` @@ -80,7 +78,7 @@ class BatchMobileResultsScreen extends Component { className='results-map' style={{bottom: mapBottom, display: itineraryExpanded ? 'none' : 'inherit'}} > - + { const activeSearch = getActiveSearch(state.otp) + const urlParams = coreUtils.query.getUrlParams() + return { activeLeg: activeSearch ? activeSearch.activeLeg : null, errors: getResponsesWithErrors(state.otp), - itineraries: getActiveItineraries(state.otp) + itineraries: getActiveItineraries(state.otp), + itineraryView: urlParams.ui_itineraryView } } -export default connect(mapStateToProps)(BatchMobileResultsScreen) +const mapDispatchToProps = { + setItineraryView: uiActions.setItineraryView +} + +export default connect(mapStateToProps, mapDispatchToProps)(BatchMobileResultsScreen) From 9e71d59fefc84fcd2ec47dd83929dfcefd63353c Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 24 Mar 2021 20:03:16 -0400 Subject: [PATCH 21/48] refactor(mobile/BatchResultsScreen): Tweak layout for no-results. --- lib/components/map/bounds-updating-overlay.js | 24 +++++++++---------- lib/components/mobile/batch-results-screen.js | 8 ++++++- lib/components/mobile/results-error.js | 4 ++-- 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/lib/components/map/bounds-updating-overlay.js b/lib/components/map/bounds-updating-overlay.js index 6f2f550ae..48afe3f61 100644 --- a/lib/components/map/bounds-updating-overlay.js +++ b/lib/components/map/bounds-updating-overlay.js @@ -128,20 +128,20 @@ class BoundsUpdatingOverlay extends MapLayer { map.panTo([step.lat, step.lon]) } else if (itineraryViewChanged) { // If itineraryView has changed, - // force a resize of the map, then refit the active itinerary or active leg. + // force a resize of the map before re-fitting the active itinerary or active leg. map.invalidateSize(true) - if ( - newProps.itinerary && - newProps.activeLeg !== null - ) { - // Fit to active leg if set. - map.fitBounds( - getLeafletLegBounds(newProps.itinerary.legs[newProps.activeLeg]), - { padding } - ) - } else { - map.fitBounds(newItinBounds, { padding }) + if (newProps.itinerary) { + if (newProps.activeLeg !== null) { + // Fit to active leg if set. + map.fitBounds( + getLeafletLegBounds(newProps.itinerary.legs[newProps.activeLeg]), + { padding } + ) + } else { + // Fit to whole itinerary otherwise. + map.fitBounds(newItinBounds, { padding }) + } } } } diff --git a/lib/components/mobile/batch-results-screen.js b/lib/components/mobile/batch-results-screen.js index 592bf06ac..ff655e57e 100644 --- a/lib/components/mobile/batch-results-screen.js +++ b/lib/components/mobile/batch-results-screen.js @@ -88,7 +88,13 @@ class BatchMobileResultsScreen extends Component {
{hasNoResult - ? + ? : (
+
) diff --git a/lib/components/narrative/narrative-itineraries.js b/lib/components/narrative/narrative-itineraries.js index 0dd79bbee..c61edce1a 100644 --- a/lib/components/narrative/narrative-itineraries.js +++ b/lib/components/narrative/narrative-itineraries.js @@ -12,7 +12,7 @@ import { setVisibleItinerary, updateItineraryFilter } from '../../actions/narrative' -import { ItineraryView } from '../../actions/ui' +import { ItineraryView, setItineraryView } from '../../actions/ui' import Icon from '../narrative/icon' import { ComponentContext } from '../../util/contexts' import { @@ -41,33 +41,20 @@ class NarrativeItineraries extends Component { activeItinerary: PropTypes.number, containerStyle: PropTypes.object, itineraries: PropTypes.array, - onToggleDetailedItinerary: PropTypes.func, pending: PropTypes.bool, setActiveItinerary: PropTypes.func, setActiveLeg: PropTypes.func, setActiveStep: PropTypes.func, + setItineraryView: PropTypes.func, setUseRealtimeResponse: PropTypes.func, useRealtime: PropTypes.bool } static contextType = ComponentContext - constructor (props) { - super(props) - this.state = { - // If URL indicates full (expanded) itinerary view, then show itinerary details. - showDetails: props.itineraryView === ItineraryView.FULL - } - } - _toggleDetailedItinerary = () => { - const showDetails = !this.state.showDetails - this.setState({ showDetails }) - - const { onToggleDetailedItinerary } = this.props - if (onToggleDetailedItinerary) { - onToggleDetailedItinerary(showDetails) - } + const { itineraryView, setItineraryView } = this.props + setItineraryView(itineraryView === ItineraryView.FULL ? ItineraryView.SPLIT : ItineraryView.FULL) } _onFilterChange = evt => { @@ -118,6 +105,7 @@ class NarrativeItineraries extends Component { containerStyle, errors, itineraries, + itineraryView, pending, realtimeEffects, sort, @@ -126,7 +114,8 @@ class NarrativeItineraries extends Component { const { ItineraryBody, LegIcon } = this.context if (!activeSearch) return null - const itineraryIsExpanded = activeItinerary !== undefined && activeItinerary !== null && this.state.showDetails + const showDetails = itineraryView === ItineraryView.FULL + const itineraryIsExpanded = activeItinerary !== undefined && activeItinerary !== null && showDetails const showRealtimeAnnotation = realtimeEffects.isAffectedByRealtimeData && ( realtimeEffects.exceedsThreshold || @@ -199,7 +188,7 @@ class NarrativeItineraries extends Component { return ( { setActiveStep: (index, step) => { dispatch(setActiveStep({index, step})) }, + setItineraryView: payload => dispatch(setItineraryView(payload)), setUseRealtimeResponse: payload => dispatch(setUseRealtimeResponse(payload)), setVisibleItinerary: payload => dispatch(setVisibleItinerary(payload)), updateItineraryFilter: payload => dispatch(updateItineraryFilter(payload)) From 7c82cd75c4bfd45860644e82e9371b260019d75e Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 30 Mar 2021 13:10:00 -0400 Subject: [PATCH 29/48] refactor(actions/ui): Rename ItineraryView.SPLIT>LIST --- lib/actions/ui.js | 4 ++-- lib/components/narrative/narrative-itineraries.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/actions/ui.js b/lib/actions/ui.js index a80f44239..ae0d9fe61 100644 --- a/lib/actions/ui.js +++ b/lib/actions/ui.js @@ -242,9 +242,9 @@ export const MobileScreens = { export const ItineraryView = { FULL: 'full', HIDDEN: 'hidden', - SPLIT: 'split' + LIST: 'list' } -const DEFAULT_ITINERARY_VIEW = ItineraryView.SPLIT +const DEFAULT_ITINERARY_VIEW = ItineraryView.LIST /** * Sets the itinerary view state (see values above) in the URL params diff --git a/lib/components/narrative/narrative-itineraries.js b/lib/components/narrative/narrative-itineraries.js index c61edce1a..d6eef1d72 100644 --- a/lib/components/narrative/narrative-itineraries.js +++ b/lib/components/narrative/narrative-itineraries.js @@ -54,7 +54,7 @@ class NarrativeItineraries extends Component { _toggleDetailedItinerary = () => { const { itineraryView, setItineraryView } = this.props - setItineraryView(itineraryView === ItineraryView.FULL ? ItineraryView.SPLIT : ItineraryView.FULL) + setItineraryView(itineraryView === ItineraryView.FULL ? ItineraryView.LIST : ItineraryView.FULL) } _onFilterChange = evt => { From d0a4f3531f62c34c9fad61e26e6a02c1b3e287fc Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 30 Mar 2021 15:07:15 -0400 Subject: [PATCH 30/48] refactor(BatchResultsScreen): Add uncommited refactors from 7c82cd7 --- lib/components/mobile/batch-results-screen.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/components/mobile/batch-results-screen.js b/lib/components/mobile/batch-results-screen.js index e49f8d631..036fe4895 100644 --- a/lib/components/mobile/batch-results-screen.js +++ b/lib/components/mobile/batch-results-screen.js @@ -59,11 +59,11 @@ class BatchMobileResultsScreen extends Component { } _setItineraryExpanded = itineraryExpanded => { - this.props.setItineraryView(itineraryExpanded ? VIEW.FULL : VIEW.SPLIT) + this.props.setItineraryView(itineraryExpanded ? VIEW.FULL : VIEW.LIST) } _toggleMapExpanded = () => { - this.props.setItineraryView(this.props.itineraryView === VIEW.HIDDEN ? VIEW.SPLIT : VIEW.HIDDEN) + this.props.setItineraryView(this.props.itineraryView === VIEW.HIDDEN ? VIEW.LIST : VIEW.HIDDEN) } render () { From df927a8f852ee3ec599d8f14bdd44406cf9376af Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 30 Mar 2021 16:05:30 -0400 Subject: [PATCH 31/48] refactor(mobile/ResultsHeader): Reset itineraryView state when clicking Edit. --- lib/components/mobile/results-header.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/components/mobile/results-header.js b/lib/components/mobile/results-header.js index 9c498da3d..588982134 100644 --- a/lib/components/mobile/results-header.js +++ b/lib/components/mobile/results-header.js @@ -59,6 +59,7 @@ class ResultsHeader extends Component { _editSearchClicked = () => { this.props.clearActiveSearch() this.props.setMobileScreen(uiActions.MobileScreens.SEARCH_FORM) + this.props.setItineraryView(uiActions.ItineraryView.LIST) } render () { @@ -119,6 +120,7 @@ const mapStateToProps = (state, ownProps) => { const mapDispatchToProps = { clearActiveSearch: formActions.clearActiveSearch, + setItineraryView: uiActions.setItineraryView, setMobileScreen: uiActions.setMobileScreen } From 7678132763b027dc21ff3d7784230b7fe5577913 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 31 Mar 2021 11:37:29 -0400 Subject: [PATCH 32/48] refactor(NarrativeItineraries): Toggle focus between leg and itinerary when clicking leg. --- lib/actions/ui.js | 5 +-- lib/components/map/bounds-updating-overlay.js | 2 +- lib/components/mobile/batch-results-screen.js | 18 ++++------ lib/components/mobile/results-header.js | 1 + .../narrative/narrative-itineraries.js | 35 +++++++++++++++---- 5 files changed, 41 insertions(+), 20 deletions(-) diff --git a/lib/actions/ui.js b/lib/actions/ui.js index ae0d9fe61..f3463eb49 100644 --- a/lib/actions/ui.js +++ b/lib/actions/ui.js @@ -240,11 +240,12 @@ export const MobileScreens = { * (currently only used in batch results). */ export const ItineraryView = { + DEFAULT: 'list', FULL: 'full', HIDDEN: 'hidden', + LEG: 'leg', LIST: 'list' } -const DEFAULT_ITINERARY_VIEW = ItineraryView.LIST /** * Sets the itinerary view state (see values above) in the URL params @@ -257,7 +258,7 @@ export function setItineraryView (value) { // If the itinerary value is changed, // set the desired ui query param, or remove it if same as default. if (value !== urlParams.ui_itineraryView) { - if (value !== DEFAULT_ITINERARY_VIEW) { + if (value !== ItineraryView.DEFAULT) { urlParams.ui_itineraryView = value } else if (urlParams.ui_itineraryView) { delete urlParams.ui_itineraryView diff --git a/lib/components/map/bounds-updating-overlay.js b/lib/components/map/bounds-updating-overlay.js index 4ba59ab06..e4ce308e6 100644 --- a/lib/components/map/bounds-updating-overlay.js +++ b/lib/components/map/bounds-updating-overlay.js @@ -125,7 +125,7 @@ class BoundsUpdatingOverlay extends MapLayer { // See https://github.com/opentripplanner/otp-react-redux/issues/133 for // more info. // TODO: Fix this so mobile devices will also update the bounds to the - // from/to locations. + // from/to locations. if (!coreUtils.ui.isMobile()) { const bounds = L.bounds([ [newFrom.lat, newFrom.lon], diff --git a/lib/components/mobile/batch-results-screen.js b/lib/components/mobile/batch-results-screen.js index 036fe4895..58f68c81b 100644 --- a/lib/components/mobile/batch-results-screen.js +++ b/lib/components/mobile/batch-results-screen.js @@ -49,21 +49,17 @@ const NARRATIVE_SPLIT_TOP_PERCENT = 45 * and features a split view between the map and itinerary results or narratives. */ class BatchMobileResultsScreen extends Component { - componentDidUpdate (prevProps) { - if (this.props.activeLeg !== prevProps.activeLeg) { - // Check if the active leg has changed. If a different leg is selected, - // unexpand the itinerary to show the map focused on the selected leg - // (similar to the behavior of LineItinerary). - this._setItineraryExpanded(false) - } + state = { + previousItineraryView: null } - _setItineraryExpanded = itineraryExpanded => { - this.props.setItineraryView(itineraryExpanded ? VIEW.FULL : VIEW.LIST) + _setItineraryView = view => { + this.setState({ previousItineraryView: this.props.itineraryView }) + this.props.setItineraryView(view) } _toggleMapExpanded = () => { - this.props.setItineraryView(this.props.itineraryView === VIEW.HIDDEN ? VIEW.LIST : VIEW.HIDDEN) + this._setItineraryView(this.props.itineraryView === VIEW.HIDDEN ? this.state.previousItineraryView : VIEW.HIDDEN) } render () { @@ -130,7 +126,7 @@ const mapStateToProps = (state, ownProps) => { activeLeg: activeSearch ? activeSearch.activeLeg : null, errors: getResponsesWithErrors(state.otp), itineraries: getActiveItineraries(state.otp), - itineraryView: urlParams.ui_itineraryView + itineraryView: urlParams.ui_itineraryView || VIEW.DEFAULT } } diff --git a/lib/components/mobile/results-header.js b/lib/components/mobile/results-header.js index 588982134..b615e007c 100644 --- a/lib/components/mobile/results-header.js +++ b/lib/components/mobile/results-header.js @@ -59,6 +59,7 @@ class ResultsHeader extends Component { _editSearchClicked = () => { this.props.clearActiveSearch() this.props.setMobileScreen(uiActions.MobileScreens.SEARCH_FORM) + // Reset itinerary view state to show the list of results. this.props.setItineraryView(uiActions.ItineraryView.LIST) } diff --git a/lib/components/narrative/narrative-itineraries.js b/lib/components/narrative/narrative-itineraries.js index d6eef1d72..d2e7fd9e9 100644 --- a/lib/components/narrative/narrative-itineraries.js +++ b/lib/components/narrative/narrative-itineraries.js @@ -52,9 +52,32 @@ class NarrativeItineraries extends Component { static contextType = ComponentContext + _setActiveLeg = (index, leg) => { + const { activeLeg, setActiveLeg, setItineraryView } = this.props + const isSameLeg = activeLeg === index + if (isSameLeg) { + // If clicking on the same leg again, reset it to null, + // and show the full itinerary (both desktop and mobile view) + setActiveLeg(null, null) + setItineraryView(ItineraryView.FULL) + } else { + // Focus on the newly selected leg. + setActiveLeg(index, leg) + setItineraryView(ItineraryView.LEG) + } + } + + _isShowingDetails = () => { + const { itineraryView } = this.props + return itineraryView === ItineraryView.FULL || itineraryView === ItineraryView.LEG + } + _toggleDetailedItinerary = () => { - const { itineraryView, setItineraryView } = this.props - setItineraryView(itineraryView === ItineraryView.FULL ? ItineraryView.LIST : ItineraryView.FULL) + const { setActiveLeg, setItineraryView } = this.props + const newView = this._isShowingDetails() ? ItineraryView.LIST : ItineraryView.FULL + setItineraryView(newView) + // Reset the active leg. + setActiveLeg(null, null) } _onFilterChange = evt => { @@ -105,7 +128,6 @@ class NarrativeItineraries extends Component { containerStyle, errors, itineraries, - itineraryView, pending, realtimeEffects, sort, @@ -114,7 +136,7 @@ class NarrativeItineraries extends Component { const { ItineraryBody, LegIcon } = this.context if (!activeSearch) return null - const showDetails = itineraryView === ItineraryView.FULL + const showDetails = this._isShowingDetails() const itineraryIsExpanded = activeItinerary !== undefined && activeItinerary !== null && showDetails const showRealtimeAnnotation = realtimeEffects.isAffectedByRealtimeData && ( @@ -193,12 +215,13 @@ class NarrativeItineraries extends Component { itinerary={itinerary} key={index} LegIcon={LegIcon} - setActiveLeg={setActiveLeg} onClick={active ? this._toggleDetailedItinerary : undefined} routingType='ITINERARY' showRealtimeAnnotation={showRealtimeAnnotation} sort={sort} {...this.props} + // Override setActiveLeg from props spreading + setActiveLeg={this._setActiveLeg} /> ) })} @@ -239,7 +262,7 @@ const mapStateToProps = (state, ownProps) => { errors: getResponsesWithErrors(state.otp), // swap out realtime itineraries with non-realtime depending on boolean itineraries, - itineraryView: urlParams.ui_itineraryView, + itineraryView: urlParams.ui_itineraryView || ItineraryView.DEFAULT, pending, realtimeEffects, activeItinerary: activeSearch && activeSearch.activeItinerary, From dffebbf445cca179f9b2043ab56cc499d05a330a Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 31 Mar 2021 11:49:42 -0400 Subject: [PATCH 33/48] refactor(NarrativeItineraries): Fix lint --- lib/components/narrative/narrative-itineraries.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/components/narrative/narrative-itineraries.js b/lib/components/narrative/narrative-itineraries.js index d2e7fd9e9..77738aa27 100644 --- a/lib/components/narrative/narrative-itineraries.js +++ b/lib/components/narrative/narrative-itineraries.js @@ -207,6 +207,11 @@ class NarrativeItineraries extends Component { const active = index === activeItinerary // Hide non-active itineraries. if (!active && itineraryIsExpanded) return null + const itineraryBodyProps = { + ...this.props, + // Override setActiveLeg from props spreading + setActiveLeg: this._setActiveLeg + } return ( ) })} From 77bfc8f678830e18fa9ed828d9d6e6652969747a Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 31 Mar 2021 15:28:49 -0400 Subject: [PATCH 34/48] refactor(mobile/results-screen): Convert options header into button+styles. --- lib/components/mobile/mobile.css | 1 - lib/components/mobile/results-screen.js | 19 +++++++++++++++---- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/lib/components/mobile/mobile.css b/lib/components/mobile/mobile.css index 1e6475c6e..4b5409dc8 100644 --- a/lib/components/mobile/mobile.css +++ b/lib/components/mobile/mobile.css @@ -159,7 +159,6 @@ text-align: center; font-size: 20px; font-weight: 500; - padding-top: 5px; } .otp.mobile .mobile-narrative-container { diff --git a/lib/components/mobile/results-screen.js b/lib/components/mobile/results-screen.js index 9ff918b83..d2e20518e 100644 --- a/lib/components/mobile/results-screen.js +++ b/lib/components/mobile/results-screen.js @@ -2,6 +2,7 @@ import coreUtils from '@opentripplanner/core-utils' import PropTypes from 'prop-types' import React, { Component } from 'react' import { connect } from 'react-redux' +import styled from 'styled-components' import * as narrativeActions from '../../actions/narrative' import Map from '../map/map' @@ -17,6 +18,16 @@ import MobileContainer from './container' import ResultsError from './results-error' import ResultsHeader from './results-header' +const OptionExpanderButton = styled.button` + border: none; + bottom: ${props => props.expanded ? 'inherit' : '100px'}; + outline: none; + padding-bottom: 3px; + display: block; + top: ${props => props.expanded ? '100px' : 'inherit'}; + width: 100%; +` + class MobileResultsScreen extends Component { static propTypes = { activeItineraryIndex: PropTypes.number, @@ -113,14 +124,14 @@ class MobileResultsScreen extends Component { ? : ( <> -
- Option {activeItineraryIndex + 1} + Option {activeItineraryIndex + 1} -
+
Date: Wed, 31 Mar 2021 16:11:11 -0400 Subject: [PATCH 35/48] refactor(mobile/ResultsError): Convert some components to styled- per PR comments --- lib/components/mobile/batch-results-screen.js | 39 ++++++++++++------- lib/components/mobile/mobile.css | 1 - lib/components/mobile/results-error.js | 11 ++++-- lib/components/mobile/results-screen.js | 26 ++++++------- 4 files changed, 47 insertions(+), 30 deletions(-) diff --git a/lib/components/mobile/batch-results-screen.js b/lib/components/mobile/batch-results-screen.js index 58f68c81b..5a82e1131 100644 --- a/lib/components/mobile/batch-results-screen.js +++ b/lib/components/mobile/batch-results-screen.js @@ -40,10 +40,26 @@ const ExpandMapButton = styled(Button)` z-index: 999999; ` -const VIEW = uiActions.ItineraryView - const NARRATIVE_SPLIT_TOP_PERCENT = 45 +// Styles for the results map also include prop-independent styles copied from mobile.css. +const ResultsMap = styled.div` + bottom: ${props => props.expanded ? '0' : `${100 - NARRATIVE_SPLIT_TOP_PERCENT}%`}; + display: ${props => props.visible ? 'inherit' : 'none'}; + left: 0; + position: fixed; + right: 0; + top: 100px; +` + +const StyledResultsError = styled(ResultsError)` + display: ${props => props.visible ? 'inherit' : 'none'}; + top: ${props => props.visible ? (props.expanded ? '100px' : `${NARRATIVE_SPLIT_TOP_PERCENT}%`) : '100%'}; + transition: top 300ms; +` + +const VIEW = uiActions.ItineraryView + /** * This component renders the mobile view of itinerary results from batch routing, * and features a split view between the map and itinerary results or narratives. @@ -68,14 +84,13 @@ class BatchMobileResultsScreen extends Component { const mapExpanded = itineraryView === VIEW.HIDDEN const itineraryExpanded = itineraryView === VIEW.FULL const narrativeTop = mapExpanded ? '100%' : (itineraryExpanded ? '100px' : `${NARRATIVE_SPLIT_TOP_PERCENT}%`) - const mapBottom = mapExpanded ? 0 : `${100 - NARRATIVE_SPLIT_TOP_PERCENT}%` return ( -
{' '} {mapExpanded ? 'Show results' : 'Expand map'} -
+ {hasNoResult - ? + expanded={itineraryExpanded} + visible={!mapExpanded} + /> : (
+
+ ) + } +} + +// connect to the redux store + +const mapDispatchToProps = { + clearActiveSearch: formActions.clearActiveSearch, + setItineraryView: uiActions.setItineraryView, + setMobileScreen: uiActions.setMobileScreen +} + +export default connect(null, mapDispatchToProps)(EditSearchButton) diff --git a/lib/components/mobile/results-error.js b/lib/components/mobile/results-error.js index 2114ed00e..eb8728a1e 100644 --- a/lib/components/mobile/results-error.js +++ b/lib/components/mobile/results-error.js @@ -1,65 +1,35 @@ import PropTypes from 'prop-types' -import React, { Component } from 'react' -import { Button } from 'react-bootstrap' -import { connect } from 'react-redux' +import React from 'react' import styled from 'styled-components' -import * as formActions from '../../actions/form' -import * as uiActions from '../../actions/ui' import ErrorMessage from '../form/error-message' +import EditSearchButton from './edit-search-button' + /** * This component is used on mobile views to * render an error message if no results are found. */ -class ResultsError extends Component { - static propTypes = { - error: PropTypes.object, - setItineraryView: PropTypes.func, - setMobileScreen: PropTypes.func - } - - _editSearchClicked = () => { - // Reset itinerary view state to show the list of results *before* clearing the search. - // (Otherwise, if the map is expanded, the search is not cleared.) - this.props.setItineraryView(uiActions.ItineraryView.LIST) - this.props.clearActiveSearch() - this.props.setMobileScreen(uiActions.MobileScreens.SEARCH_FORM) - } - - render () { - const { className, error } = this.props - return ( -
- -
- -
-
- ) - } +const ResultsError = ({ className, error }) => ( +
+ +
+ + Back to Search + +
+
+) + +ResultsError.propTypes = { + error: PropTypes.object } const StyledResultsError = styled(ResultsError)` top: 300px; ` -// connect to the redux store - -const mapStateToProps = (state, ownProps) => { - return {} -} - -const mapDispatchToProps = { - clearActiveSearch: formActions.clearActiveSearch, - setItineraryView: uiActions.setItineraryView, - setMobileScreen: uiActions.setMobileScreen -} - -export default connect(mapStateToProps, mapDispatchToProps)(StyledResultsError) +export default StyledResultsError diff --git a/lib/components/mobile/results-header.js b/lib/components/mobile/results-header.js index 14b478e49..d0aa51c4b 100644 --- a/lib/components/mobile/results-header.js +++ b/lib/components/mobile/results-header.js @@ -1,18 +1,17 @@ import LocationIcon from '@opentripplanner/location-icon' import PropTypes from 'prop-types' import React, { Component } from 'react' -import { Button, Col, Row } from 'react-bootstrap' +import { Col, Row } from 'react-bootstrap' import { connect } from 'react-redux' import styled from 'styled-components' -import * as formActions from '../../actions/form' -import * as uiActions from '../../actions/ui' import { getActiveItineraries, getActiveSearch, getResponsesWithErrors } from '../../util/state' +import EditSearchButton from './edit-search-button' import MobileNavigationBar from './navigation-bar' const LocationContainer = styled.div` @@ -52,17 +51,7 @@ class ResultsHeader extends Component { static propTypes = { errors: PropTypes.array, query: PropTypes.object, - resultCount: PropTypes.number, - setItineraryView: PropTypes.func, - setMobileScreen: PropTypes.func - } - - _editSearchClicked = () => { - // Reset itinerary view state to show the list of results *before* clearing the search. - // (Otherwise, if the map is expanded, the search is not cleared.) - this.props.setItineraryView(uiActions.ItineraryView.LIST) - this.props.clearActiveSearch() - this.props.setMobileScreen(uiActions.MobileScreens.SEARCH_FORM) + resultCount: PropTypes.number } render () { @@ -90,10 +79,9 @@ class ResultsHeader extends Component { - + + Edit + @@ -121,10 +109,4 @@ const mapStateToProps = (state, ownProps) => { } } -const mapDispatchToProps = { - clearActiveSearch: formActions.clearActiveSearch, - setItineraryView: uiActions.setItineraryView, - setMobileScreen: uiActions.setMobileScreen -} - -export default connect(mapStateToProps, mapDispatchToProps)(ResultsHeader) +export default connect(mapStateToProps)(ResultsHeader) From af9f653f1d0dec934913ef1ec57fb42eeb995f42 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 9 Apr 2021 15:10:27 -0400 Subject: [PATCH 44/48] fix(actions/ui): Split ItineraryView hidden state. --- lib/actions/ui.js | 8 +++++--- lib/components/mobile/batch-results-screen.js | 9 ++++++--- lib/components/narrative/narrative-itineraries.js | 4 +++- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/lib/actions/ui.js b/lib/actions/ui.js index a435fe027..7379dfeee 100644 --- a/lib/actions/ui.js +++ b/lib/actions/ui.js @@ -243,12 +243,14 @@ export const ItineraryView = { DEFAULT: 'list', /** One itinerary is shown. (In mobile view, the map is hidden.) */ FULL: 'full', - /** Itinerary is hidden. (In mobile view, the map is expanded.) */ - HIDDEN: 'hidden', /** One itinerary is shown, itinerary and map are focused on a leg. (The mobile view is split.) */ LEG: 'leg', + /** One itinerary leg is hidden. (In mobile view, the map is expanded.) */ + LEG_HIDDEN: 'leg-hidden', /** The list of itineraries is shown. (The mobile view is split.) */ - LIST: 'list' + LIST: 'list', + /** The list of itineraries is hidden. (In mobile view, the map is expanded.) */ + LIST_HIDDEN: 'list-hidden' } /** diff --git a/lib/components/mobile/batch-results-screen.js b/lib/components/mobile/batch-results-screen.js index 45b505e20..da1750bb3 100644 --- a/lib/components/mobile/batch-results-screen.js +++ b/lib/components/mobile/batch-results-screen.js @@ -85,9 +85,12 @@ class BatchMobileResultsScreen extends Component { _toggleMapExpanded = () => { const { itineraryView, setItineraryView } = this.props - if (itineraryView !== ItineraryView.HIDDEN) { + if (itineraryView === ItineraryView.LEG) { this.setState({ previousSplitView: itineraryView }) - setItineraryView(ItineraryView.HIDDEN) + setItineraryView(ItineraryView.LEG_HIDDEN) + } else if (itineraryView === ItineraryView.LIST) { + this.setState({ previousSplitView: itineraryView }) + setItineraryView(ItineraryView.LIST_HIDDEN) } else { setItineraryView(this.state.previousSplitView) } @@ -96,7 +99,7 @@ class BatchMobileResultsScreen extends Component { render () { const { errors, itineraries, itineraryView } = this.props const hasErrorsAndNoResult = itineraries.length === 0 && errors.length > 0 - const mapExpanded = itineraryView === ItineraryView.HIDDEN + const mapExpanded = itineraryView === ItineraryView.LEG_HIDDEN || itineraryView === ItineraryView.LIST_HIDDEN const itineraryExpanded = itineraryView === ItineraryView.FULL return ( diff --git a/lib/components/narrative/narrative-itineraries.js b/lib/components/narrative/narrative-itineraries.js index 12beb7af9..cdcaec0ff 100644 --- a/lib/components/narrative/narrative-itineraries.js +++ b/lib/components/narrative/narrative-itineraries.js @@ -71,7 +71,9 @@ class NarrativeItineraries extends Component { _isShowingDetails = () => { const { itineraryView } = this.props - return itineraryView === ItineraryView.FULL || itineraryView === ItineraryView.LEG + return itineraryView === ItineraryView.FULL || + itineraryView === ItineraryView.LEG || + itineraryView === ItineraryView.LEG_HIDDEN } _toggleDetailedItinerary = () => { From 7a8b67e5e5555bb641af54e4415b83bfb99f767d Mon Sep 17 00:00:00 2001 From: Landon Reed Date: Fri, 9 Apr 2021 16:23:28 -0400 Subject: [PATCH 45/48] refactor(state.js): remove noisy/unneeded log statement --- lib/util/state.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/util/state.js b/lib/util/state.js index a3459b805..4caab3f7f 100644 --- a/lib/util/state.js +++ b/lib/util/state.js @@ -99,7 +99,6 @@ export function getActiveItineraries (otpState) { }) return hasCar default: - console.warn(`Filter (${filter}) not supported`) return true } }) From 3f138bb57c187373a0dea19cc90aea90f8fb92bd Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Mon, 12 Apr 2021 09:36:31 -0400 Subject: [PATCH 46/48] refactor(NarrativeItineraries): Remove unused prop. --- lib/components/narrative/narrative-itineraries.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/components/narrative/narrative-itineraries.js b/lib/components/narrative/narrative-itineraries.js index cdcaec0ff..b241ac753 100644 --- a/lib/components/narrative/narrative-itineraries.js +++ b/lib/components/narrative/narrative-itineraries.js @@ -279,7 +279,6 @@ const mapStateToProps = (state, ownProps) => { activeSearch, activeStep: activeSearch && activeSearch.activeStep, errors: getResponsesWithErrors(state.otp), - filter: state.otp.filter, itineraries, itineraryView: urlParams.ui_itineraryView || ItineraryView.DEFAULT, modes, From 43fef77367b6d788c401198500108f49fff1f355 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 13 Apr 2021 11:39:22 -0400 Subject: [PATCH 47/48] refactor(actions/ui): Extract batch results and edit search actions. --- lib/actions/ui.js | 40 +++++- lib/components/mobile/batch-results-screen.js | 124 +++++++----------- lib/components/mobile/edit-search-button.js | 42 +----- lib/reducers/create-otp-reducer.js | 2 + 4 files changed, 97 insertions(+), 111 deletions(-) diff --git a/lib/actions/ui.js b/lib/actions/ui.js index 7379dfeee..955515798 100644 --- a/lib/actions/ui.js +++ b/lib/actions/ui.js @@ -253,6 +253,8 @@ export const ItineraryView = { LIST_HIDDEN: 'list-hidden' } +const setPreviousItineraryView = createAction('SET_PREVIOUS_ITINERARY_VIEW') + /** * Sets the itinerary view state (see values above) in the URL params * (currently only used in batch results). @@ -260,9 +262,11 @@ export const ItineraryView = { export function setItineraryView (value) { return function (dispatch, getState) { const urlParams = coreUtils.query.getUrlParams() + const prevItineraryView = urlParams.ui_itineraryView || ItineraryView.DEFAULT // If the itinerary value is changed, - // set the desired ui query param, or remove it if same as default. + // set the desired ui query param, or remove it if same as default, + // and store the current view as previousItineraryView. if (value !== urlParams.ui_itineraryView) { if (value !== ItineraryView.DEFAULT) { urlParams.ui_itineraryView = value @@ -271,6 +275,40 @@ export function setItineraryView (value) { } dispatch(setUrlSearch(urlParams)) + dispatch(setPreviousItineraryView(prevItineraryView)) + } + } +} + +/** + * Switch the mobile batch results view between full map view and the split state + * (itinerary list or itinerary leg view) that was in place prior. + */ +export function toggleBatchResultsMap () { + return function (dispatch, getState) { + const urlParams = coreUtils.query.getUrlParams() + const itineraryView = urlParams.ui_itineraryView || ItineraryView.DEFAULT + + if (itineraryView === ItineraryView.LEG) { + dispatch(setItineraryView(ItineraryView.LEG_HIDDEN)) + } else if (itineraryView === ItineraryView.LIST) { + dispatch(setItineraryView(ItineraryView.LIST_HIDDEN)) + } else { + const { previousItineraryView } = getState().otp.ui + dispatch(setItineraryView(previousItineraryView)) } } } + +/** + * Takes the user back to the mobile search screen in mobile views. + */ +export function showMobileSearchScreen () { + return function (dispatch, getState) { + // Reset itinerary view state to show the list of results *before* clearing the search. + // (Otherwise, if the map is expanded, the search is not cleared.) + dispatch(setItineraryView(ItineraryView.LIST)) + dispatch(clearActiveSearch()) + dispatch(setMobileScreen(MobileScreens.SEARCH_FORM)) + } +} diff --git a/lib/components/mobile/batch-results-screen.js b/lib/components/mobile/batch-results-screen.js index da1750bb3..51370a897 100644 --- a/lib/components/mobile/batch-results-screen.js +++ b/lib/components/mobile/batch-results-screen.js @@ -1,5 +1,5 @@ import coreUtils from '@opentripplanner/core-utils' -import React, { Component } from 'react' +import React from 'react' import { Button } from 'react-bootstrap' import { connect } from 'react-redux' import styled, { css } from 'styled-components' @@ -72,79 +72,58 @@ const { ItineraryView } = uiActions * This component renders the mobile view of itinerary results from batch routing, * and features a split view between the map and itinerary results or narratives. */ -class BatchMobileResultsScreen extends Component { - state = { - // Holds the previous split view state - previousSplitView: null - } - - /** - * Switch between full map view and the split state (itinerary list or itinerary leg view) - * that was in place prior. - */ - _toggleMapExpanded = () => { - const { itineraryView, setItineraryView } = this.props - - if (itineraryView === ItineraryView.LEG) { - this.setState({ previousSplitView: itineraryView }) - setItineraryView(ItineraryView.LEG_HIDDEN) - } else if (itineraryView === ItineraryView.LIST) { - this.setState({ previousSplitView: itineraryView }) - setItineraryView(ItineraryView.LIST_HIDDEN) - } else { - setItineraryView(this.state.previousSplitView) - } - } - - render () { - const { errors, itineraries, itineraryView } = this.props - const hasErrorsAndNoResult = itineraries.length === 0 && errors.length > 0 - const mapExpanded = itineraryView === ItineraryView.LEG_HIDDEN || itineraryView === ItineraryView.LIST_HIDDEN - const itineraryExpanded = itineraryView === ItineraryView.FULL - - return ( - - - { + const hasErrorsAndNoResult = itineraries.length === 0 && errors.length > 0 + const mapExpanded = itineraryView === ItineraryView.LEG_HIDDEN || itineraryView === ItineraryView.LIST_HIDDEN + const itineraryExpanded = itineraryView === ItineraryView.FULL + + return ( + + + + + - - - {' '} - {mapExpanded ? 'Show results' : 'Expand map'} - - - {hasErrorsAndNoResult - ? {' '} + {mapExpanded ? 'Show results' : 'Expand map'} + + + {hasErrorsAndNoResult + ? + : ( + - : ( - - - - ) - } - - ) - } + > + + + ) + } + + ) } // connect to the redux store @@ -152,7 +131,6 @@ class BatchMobileResultsScreen extends Component { const mapStateToProps = (state, ownProps) => { const activeSearch = getActiveSearch(state.otp) const urlParams = coreUtils.query.getUrlParams() - return { activeLeg: activeSearch ? activeSearch.activeLeg : null, errors: getResponsesWithErrors(state.otp), @@ -162,7 +140,7 @@ const mapStateToProps = (state, ownProps) => { } const mapDispatchToProps = { - setItineraryView: uiActions.setItineraryView + toggleBatchResultsMap: uiActions.toggleBatchResultsMap } export default connect(mapStateToProps, mapDispatchToProps)(BatchMobileResultsScreen) diff --git a/lib/components/mobile/edit-search-button.js b/lib/components/mobile/edit-search-button.js index d1ad67ea3..bdf514857 100644 --- a/lib/components/mobile/edit-search-button.js +++ b/lib/components/mobile/edit-search-button.js @@ -1,52 +1,20 @@ -import PropTypes from 'prop-types' -import React, { Component } from 'react' +import React from 'react' import { Button } from 'react-bootstrap' import { connect } from 'react-redux' -import * as formActions from '../../actions/form' import * as uiActions from '../../actions/ui' /** * Renders the "Edit" or "Back to search" button in mobile result views * that takes the user back to the mobile search screen. */ -class EditSearchButton extends Component { - static propTypes = { - clearActiveSearch: PropTypes.func, - setItineraryView: PropTypes.func, - setMobileScreen: PropTypes.func - } - - _handleClick = () => { - const { clearActiveSearch, setItineraryView, setMobileScreen } = this.props - - // Reset itinerary view state to show the list of results *before* clearing the search. - // (Otherwise, if the map is expanded, the search is not cleared.) - setItineraryView(uiActions.ItineraryView.LIST) - clearActiveSearch() - setMobileScreen(uiActions.MobileScreens.SEARCH_FORM) - } - - render () { - const { children, className, style } = this.props - return ( - - ) - } -} +const EditSearchButton = ({ showMobileSearchScreen, ...props }) => ( +