From a1e63b3cd18544d40eeae993ecaaa4933f88072c Mon Sep 17 00:00:00 2001 From: Evan Siroky Date: Tue, 9 Mar 2021 16:27:33 -0800 Subject: [PATCH] refactor: refactor ResponsiveWebapp and some other things BREAKING CHANGE: refactored example.js, ResponsiveWebapp and associated component signatures to use components provided to component context --- __tests__/reducers/create-otp-reducer.js | 2 +- __tests__/test-utils/mock-data/store.js | 7 +- example.js | 156 ++++++------------ lib/components/app/batch-routing-panel.js | 1 - lib/components/app/responsive-webapp.js | 48 +++++- lib/components/mobile/main.js | 23 +-- lib/components/mobile/navigation-bar.js | 12 +- lib/components/mobile/results-screen.js | 131 ++++++++------- lib/components/mobile/welcome-screen.js | 5 +- .../narrative/narrative-itineraries.js | 1 - lib/index.js | 11 +- lib/reducers/create-otp-reducer.js | 8 +- 12 files changed, 200 insertions(+), 205 deletions(-) diff --git a/__tests__/reducers/create-otp-reducer.js b/__tests__/reducers/create-otp-reducer.js index 05fcdf475..979268d0b 100644 --- a/__tests__/reducers/create-otp-reducer.js +++ b/__tests__/reducers/create-otp-reducer.js @@ -6,6 +6,6 @@ describe('lib > reducers > create-otp-reducer', () => { it('should be able to create the initial state', () => { setDefaultTestTime() - expect(getInitialState({}, {})).toMatchSnapshot() + expect(getInitialState({})).toMatchSnapshot() }) }) diff --git a/__tests__/test-utils/mock-data/store.js b/__tests__/test-utils/mock-data/store.js index 90e7a696e..724435f0d 100644 --- a/__tests__/test-utils/mock-data/store.js +++ b/__tests__/test-utils/mock-data/store.js @@ -22,10 +22,11 @@ const storeMiddleWare = [ * Get the initial stop of the redux reducer for otp-rr */ export function getMockInitialState () { - const mockConfig = {} - const mockInitialQuery = {} + const mockConfig = { + initialQuery: {} + } return clone({ - otp: getInitialState(mockConfig, mockInitialQuery), + otp: getInitialState(mockConfig), router: connectRouter(history) }) } diff --git a/example.js b/example.js index 1845f3ee5..0f4484e8c 100644 --- a/example.js +++ b/example.js @@ -4,35 +4,37 @@ import 'es6-math' import {ClassicLegIcon, ClassicModeIcon} from '@opentripplanner/icons' import { createHashHistory } from 'history' import { connectRouter, routerMiddleware } from 'connected-react-router' -import React, { Component } from 'react' +import React from 'react' import { render } from 'react-dom' import { createStore, combineReducers, applyMiddleware, compose } from 'redux' import { Provider } from 'react-redux' import thunk from 'redux-thunk' import createLogger from 'redux-logger' -// import Bootstrap Grid components for layout -import { Grid, Row, Col } from 'react-bootstrap' - // import OTP-RR components import { + BatchRoutingPanel, + BatchSearchScreen, CallTakerControls, CallTakerPanel, CallTakerWindows, DefaultItinerary, DefaultMainPanel, - DesktopNav, - BatchRoutingPanel, - Map, - MobileMain, + MobileSearchScreen, ResponsiveWebapp, createCallTakerReducer, createOtpReducer, - createUserReducer + createUserReducer, + otpUtils } from './lib' // load the OTP configuration import otpConfig from './config.yml' +const isBatchRoutingEnabled = otpUtils.itinerary.isBatchRoutingEnabled( + otpConfig +) +const isCallTakerModuleEnabled = !!otpConfig.datastoreUrl + // Set useCustomIcons to true to override classic icons with the exports from // custom-icons.js const useCustomIcons = false @@ -49,18 +51,32 @@ if (useCustomIcons) { // define some application-wide components that should be used in // various places. The following components can be provided here: +// - defaultMobileTitle (required) // - ItineraryBody (required) // - ItineraryFooter (optional) // - LegIcon (required) +// - MainControls (optional) +// - MainPanel (required) +// - MapWindows (optional) +// - MobileSearchScreen (required) // - ModeIcon (required) const components = { + defaultMobileTitle: () =>
OpenTripPlanner
, ItineraryBody: DefaultItinerary, LegIcon: MyLegIcon, + MainControls: isCallTakerModuleEnabled ? CallTakerControls : null, + MainPanel: isCallTakerModuleEnabled + ? CallTakerPanel + : isBatchRoutingEnabled + ? BatchRoutingPanel + : DefaultMainPanel, + MapWindows: isCallTakerModuleEnabled ? CallTakerWindows : null, + MobileSearchScreen: isBatchRoutingEnabled + ? BatchSearchScreen + : MobileSearchScreen, ModeIcon: MyModeIcon } -// Get the initial query from config (for demo/testing purposes). -const {initialQuery} = otpConfig const history = createHashHistory() const middleware = [ thunk, @@ -76,109 +92,41 @@ if (process.env.NODE_ENV === 'development') { const store = createStore( combineReducers({ callTaker: createCallTakerReducer(), - otp: createOtpReducer(otpConfig, initialQuery), + otp: createOtpReducer(otpConfig), user: createUserReducer(), router: connectRouter(history) }), compose(applyMiddleware(...middleware)) ) -// define a simple responsive UI using Bootstrap and OTP-RR -class OtpRRExample extends Component { - render () { - /** desktop view **/ - const desktopView = ( -
- - - - - {/* - Note: the main tag provides a way for users of screen readers - to skip to the primary page content. - TODO: Find a better place. - */} -
- {/* TODO: extract the BATCH elements out of CallTakerPanel. */} - {otpConfig.datastoreUrl - ? - : otpConfig.routingTypes.find(t => t.key === 'BATCH') - ? - : - } -
- - {otpConfig.datastoreUrl ? : null} - - {otpConfig.datastoreUrl ? : null} - - - - -
- ) - - /** mobile view **/ - const mobileView = ( - //
Needed for accessibility checks. TODO: Find a better place. -
- } - title={
OpenTripPlanner
} - /> -
- ) - - /** - * The main webapp. - * - * Note: the ResponsiveWebapp creates a React context provider - * (./util/contexts#ComponentContext to be specific) to supply custom - * components to various other subcomponents throughout otp-react-redux. If - * the ResponsiveWebapp is not used and instead some subcomponents that use - * the components in the `components` variable are imported and rendered - * outside of the ResponsiveWebapp component, then the ComponentContext will - * need to wrap that component in order for the subcomponents to be able to - * access the component context. For example: - * - * ```js - * import RouteViewer from 'otp-react-redux/build/components/viewers/route-viewer' - * import { ComponentContext } from 'otp-react-redux/build/util/contexts' - * - * const components = { - * ModeIcon: MyCustomModeIconComponent - * } - * const ContextAwareRouteViewer = () => ( - * - * - * - * ) - * ``` - */ - return ( - - ) - } -} - // render the app render( ( - { /** - * If not using router history, simply include OtpRRExample here: - * e.g. - * - */ - } - + {/** + * Note: the ResponsiveWebapp creates a React context provider + * (./util/contexts#ComponentContext to be specific) to supply custom + * components to various other subcomponents throughout otp-react-redux. If + * the ResponsiveWebapp is not used and instead some subcomponents that use + * the components in the `components` variable are imported and rendered + * outside of the ResponsiveWebapp component, then the ComponentContext will + * need to wrap that component in order for the subcomponents to be able to + * access the component context. For example: + * + * ```js + * import RouteViewer from 'otp-react-redux/build/components/viewers/route-viewer' + * import { ComponentContext } from 'otp-react-redux/build/util/contexts' + * + * const components = { ModeIcon: MyCustomModeIconComponent } + * const ContextAwareRouteViewer = () => ( + * + * + * + * ) + * ``` + */} + - ) - , - + ), document.getElementById('root') ) diff --git a/lib/components/app/batch-routing-panel.js b/lib/components/app/batch-routing-panel.js index 14fee5bc5..0cc9a36a8 100644 --- a/lib/components/app/batch-routing-panel.js +++ b/lib/components/app/batch-routing-panel.js @@ -1,4 +1,3 @@ -import coreUtils from '@opentripplanner/core-utils' import React, { Component } from 'react' import { connect } from 'react-redux' import styled from 'styled-components' diff --git a/lib/components/app/responsive-webapp.js b/lib/components/app/responsive-webapp.js index 7b2b9b27d..2d1ac0c4d 100644 --- a/lib/components/app/responsive-webapp.js +++ b/lib/components/app/responsive-webapp.js @@ -5,6 +5,7 @@ import isEqual from 'lodash.isequal' import coreUtils from '@opentripplanner/core-utils' import PropTypes from 'prop-types' import React, { Component } from 'react' +import { Col, Grid, Row } from 'react-bootstrap' import { connect } from 'react-redux' import { Redirect, Route, Switch, withRouter } from 'react-router' @@ -16,6 +17,9 @@ import * as formActions from '../../actions/form' import * as locationActions from '../../actions/location' import * as mapActions from '../../actions/map' import * as uiActions from '../../actions/ui' +import Map from '../map/map' +import MobileMain from '../mobile/main' +import DesktopNav from './desktop-nav' import { getAuth0Config } from '../../util/auth' import { ACCOUNT_PATH, @@ -39,12 +43,12 @@ const { isMobile } = coreUtils.ui class ResponsiveWebapp extends Component { static propTypes = { - desktopView: PropTypes.element, initZoomOnLocate: PropTypes.number, - mobileView: PropTypes.element, query: PropTypes.object } + static contextType = ComponentContext + /** Lifecycle methods **/ componentDidUpdate (prevProps) { @@ -144,9 +148,45 @@ class ResponsiveWebapp extends Component { window.removeEventListener('popstate', this.props.handleBackButtonPress) } + renderDesktopView = () => { + const { MainControls, MainPanel, MapWindows } = this.context + return ( +
+ + + + + {/* + Note: the main tag provides a way for users of screen readers + to skip to the primary page content. + TODO: Find a better place. + */} +
+ {} +
+ + {MainControls && } + + {MapWindows && } + + + + +
+ ) + } + + renderMobileView = () => { + return ( + //
Needed for accessibility checks. TODO: Find a better place. +
+ +
+ ) + } + render () { - const { desktopView, mobileView } = this.props - return isMobile() ? mobileView : desktopView + return isMobile() ? this.renderMobileView() : this.renderDesktopView() } } diff --git a/lib/components/mobile/main.js b/lib/components/mobile/main.js index 03463e90b..dc9eb2344 100644 --- a/lib/components/mobile/main.js +++ b/lib/components/mobile/main.js @@ -2,19 +2,17 @@ import React, { Component } from 'react' import PropTypes from 'prop-types' import { connect } from 'react-redux' -import BatchSearchScreen from './batch-search-screen' 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 MobileSearchScreen from './search-screen' import MobileStopViewer from './stop-viewer' import MobileTripViewer from './trip-viewer' import MobileRouteViewer from './route-viewer' import { MobileScreens, MainPanelContent, setMobileScreen } from '../../actions/ui' -import { isBatchRoutingEnabled } from '../../util/itinerary' +import { ComponentContext } from '../../util/contexts' import { getActiveItinerary } from '../../util/state' class MobileMain extends Component { @@ -26,6 +24,8 @@ class MobileMain extends Component { uiState: PropTypes.object } + static contextType = ComponentContext + componentDidUpdate (prevProps) { // Check if we are in the welcome screen and both locations have been set OR // auto-detect is denied and one location is set @@ -45,7 +45,8 @@ class MobileMain extends Component { } render () { - const { config, map, title, uiState } = this.props + const { MobileSearchScreen } = this.context + const { uiState } = this.props // check for route viewer if (uiState.mainPanelContent === MainPanelContent.ROUTE_VIEWER) { @@ -60,7 +61,7 @@ class MobileMain extends Component { switch (uiState.mobileScreen) { case MobileScreens.WELCOME_SCREEN: - return + return case MobileScreens.SET_INITIAL_LOCATION: return ( @@ -73,14 +74,8 @@ class MobileMain extends Component { case MobileScreens.SEARCH_FORM: // Render batch search screen if batch routing enabled. Otherwise, // default to standard search screen. - const SearchScreen = isBatchRoutingEnabled(config) - ? BatchSearchScreen - : MobileSearchScreen return ( - + ) case MobileScreens.SET_FROM_LOCATION: @@ -106,9 +101,7 @@ class MobileMain extends Component { return case MobileScreens.RESULTS_SUMMARY: - return ( - - ) + return default: return

Invalid mobile screen

} diff --git a/lib/components/mobile/navigation-bar.js b/lib/components/mobile/navigation-bar.js index cac315e1c..41d4e274e 100644 --- a/lib/components/mobile/navigation-bar.js +++ b/lib/components/mobile/navigation-bar.js @@ -8,6 +8,7 @@ import { setMobileScreen } from '../../actions/ui' import AppMenu from '../app/app-menu' import NavLoginButtonAuth0 from '../../components/user/nav-login-button-auth0' import { accountLinks, getAuth0Config } from '../../util/auth' +import { ComponentContext } from '../../util/contexts' class MobileNavigationBar extends Component { static propTypes = { @@ -15,10 +16,11 @@ class MobileNavigationBar extends Component { headerAction: PropTypes.element, headerText: PropTypes.string, showBackButton: PropTypes.bool, - setMobileScreen: PropTypes.func, - title: PropTypes.element + setMobileScreen: PropTypes.func } + static contextType = ComponentContext + _backButtonPressed = () => { const { backScreen, onBackClicked } = this.props if (backScreen) this.props.setMobileScreen(this.props.backScreen) @@ -26,12 +28,12 @@ class MobileNavigationBar extends Component { } render () { + const { defaultMobileTitle } = this.context const { auth0Config, headerAction, headerText, - showBackButton, - title + showBackButton } = this.props return ( @@ -49,7 +51,7 @@ class MobileNavigationBar extends Component {
{headerText ?
{headerText}
- :
{title}
+ :
{defaultMobileTitle}
}
diff --git a/lib/components/mobile/results-screen.js b/lib/components/mobile/results-screen.js index 4fe96e7d8..93bb9bb61 100644 --- a/lib/components/mobile/results-screen.js +++ b/lib/components/mobile/results-screen.js @@ -6,7 +6,7 @@ import { Button, Col, Row } from 'react-bootstrap' import { connect } from 'react-redux' import styled from 'styled-components' -import DefaultMap from '../map/default-map' +import Map from '../map/map' import ErrorMessage from '../form/error-message' import ItineraryCarousel from '../narrative/itinerary-carousel' @@ -55,10 +55,8 @@ const StyledLocationIcon = styled(LocationIcon)` class MobileResultsScreen extends Component { static propTypes = { activeItineraryIndex: PropTypes.number, - map: PropTypes.element, query: PropTypes.object, resultCount: PropTypes.number, - setMobileScreen: PropTypes.func } @@ -97,46 +95,53 @@ class MobileResultsScreen extends Component { this._setExpanded(!this.state.expanded) } - _toggleRealtime = () => this.props.setUseRealtimeResponse({useRealtime: !this.props.useRealtime}) + _toggleRealtime = () => this.props.setUseRealtimeResponse( + {useRealtime: !this.props.useRealtime} + ) - render () { - const { - activeItineraryIndex, - error, - query, - realtimeEffects, - resultCount, - useRealtime - } = this.props - const { expanded } = this.state + renderDots = () => { + const { activeItineraryIndex, resultCount } = this.props - const narrativeContainerStyle = expanded - ? { top: 140, overflowY: 'auto' } - : { height: 80, overflowY: 'hidden' } + // Construct the 'dots' + const dots = [] + for (let i = 0; i < resultCount; i++) { + dots.push( +
+ ) + } - // Ensure that narrative covers map. - narrativeContainerStyle.backgroundColor = 'white' + return ( +
{dots}
+ ) + } - let headerAction = null - const showRealtimeAnnotation = realtimeEffects.isAffectedByRealtimeData && ( - realtimeEffects.exceedsThreshold || - realtimeEffects.routesDiffer || - !useRealtime + renderError = () => { + const { error } = this.props + + return ( + + + {this.renderLocationsSummary()} +
+
+ +
+ +
+
+ ) + } - /* Old navbar alert - if (showRealtimeAnnotation) { - headerAction = ( - - ) - */ + renderLocationsSummary = () => { + const { query } = this.props - const locationsSummary = ( + return ( @@ -156,29 +161,34 @@ class MobileResultsScreen extends Component { ) + } - if (error) { - return ( - - - {locationsSummary} -
-
- -
- -
-
- - ) - } + render () { + const { + activeItineraryIndex, + error, + realtimeEffects, + resultCount, + useRealtime + } = this.props + const { expanded } = this.state - // Construct the 'dots' - const dots = [] - for (let i = 0; i < resultCount; i++) { - dots.push(
) + const narrativeContainerStyle = expanded + ? { top: 140, overflowY: 'auto' } + : { height: 80, overflowY: 'hidden' } + + // 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 ( @@ -190,10 +200,10 @@ class MobileResultsScreen extends Component { } headerAction={headerAction} /> - {locationsSummary} + {this.renderLocationsSummary()}
- {this.props.map} +
- -
{dots}
+ {this.renderDots()} ) } diff --git a/lib/components/mobile/welcome-screen.js b/lib/components/mobile/welcome-screen.js index d31712c05..31ced700b 100644 --- a/lib/components/mobile/welcome-screen.js +++ b/lib/components/mobile/welcome-screen.js @@ -12,8 +12,6 @@ import { setLocationToCurrent } from '../../actions/map' class MobileWelcomeScreen extends Component { static propTypes = { - map: PropTypes.element, - setLocationToCurrent: PropTypes.func, setMobileScreen: PropTypes.func } @@ -36,10 +34,9 @@ class MobileWelcomeScreen extends Component { } render () { - const { title } = this.props return ( - +
({ ...l, type: 'suggested' }))) @@ -243,8 +243,8 @@ export function getInitialState (userDefinedConfig, initialQuery) { } } -function createOtpReducer (config, initialQuery) { - const initialState = getInitialState(config, initialQuery) +function createOtpReducer (config) { + const initialState = getInitialState(config) // validate the initial state validateInitialState(initialState)