From a2f2dc42ff084146b973429cc79ebe771053f25d Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 18 Aug 2023 16:05:36 -0400 Subject: [PATCH 1/4] refactor(lib/util): Strengthen getItineraryView tests. --- __tests__/util/ui.ts | 3 +++ lib/util/ui.ts | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/__tests__/util/ui.ts b/__tests__/util/ui.ts index 172c1b36e..0dfadfbf1 100644 --- a/__tests__/util/ui.ts +++ b/__tests__/util/ui.ts @@ -4,6 +4,9 @@ describe('util > ui', () => { describe('getItineraryView', () => { it('returns a list mode by default or ui_activeItinerary is -1', () => { expect(getItineraryView({})).toBe(ItineraryView.LIST) + expect(getItineraryView({ ui_activeItinerary: null })).toBe( + ItineraryView.LIST + ) expect(getItineraryView({ ui_activeItinerary: -1 })).toBe( ItineraryView.LIST ) diff --git a/lib/util/ui.ts b/lib/util/ui.ts index ba7dff093..b438374bd 100644 --- a/lib/util/ui.ts +++ b/lib/util/ui.ts @@ -102,7 +102,8 @@ export function getItineraryView({ }: UrlParams): ItineraryView { return ( ui_itineraryView || - (ui_activeItinerary !== undefined && + (ui_activeItinerary !== null && + ui_activeItinerary !== undefined && `${ui_activeItinerary}` !== '-1' && ItineraryView.FULL) || ItineraryView.LIST From aa3ddaf86d5fdcd19fa92a7a2df8f47ee474c2a8 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 18 Aug 2023 16:18:26 -0400 Subject: [PATCH 2/4] fix(actions/util): Treat ui_activeItinerary null same as -1 in back navigation. --- lib/actions/ui.js | 10 ++++++++-- lib/util/ui.ts | 14 ++++++++++---- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/lib/actions/ui.js b/lib/actions/ui.js index 28c03e244..4df30e074 100644 --- a/lib/actions/ui.js +++ b/lib/actions/ui.js @@ -10,7 +10,11 @@ import { loadLocaleData } from '../util/i18n' import { getModesForActiveAgencyFilter, getUiUrlParams } from '../util/state' -import { getPathFromParts, ItineraryView } from '../util/ui' +import { + getPathFromParts, + isDefinedAndNotEqual, + ItineraryView +} from '../util/ui' import { clearActiveSearch, @@ -269,7 +273,9 @@ export function handleBackButtonPress(e) { // previous search ID. if (activeSearchId !== previousSearchId) { dispatch(setActiveSearch(previousSearchId)) - } else if (uiUrlParams.ui_activeItinerary !== previousItinIndex) { + } else if ( + isDefinedAndNotEqual(uiUrlParams.ui_activeItinerary, previousItinIndex) + ) { // Active itinerary index has changed. dispatch(setActiveItinerary({ index: previousItinIndex })) } diff --git a/lib/util/ui.ts b/lib/util/ui.ts index b438374bd..60e1d4b0a 100644 --- a/lib/util/ui.ts +++ b/lib/util/ui.ts @@ -93,6 +93,15 @@ interface UrlParams { ui_itineraryView: ItineraryView } +export function isDefinedAndNotEqual( + subject: number | string, + value: number | string +): boolean { + return ( + subject !== null && subject !== undefined && `${subject}` !== `${value}` + ) +} + /** * Gets the itinerary view to display based on URL params. */ @@ -102,10 +111,7 @@ export function getItineraryView({ }: UrlParams): ItineraryView { return ( ui_itineraryView || - (ui_activeItinerary !== null && - ui_activeItinerary !== undefined && - `${ui_activeItinerary}` !== '-1' && - ItineraryView.FULL) || + (isDefinedAndNotEqual(ui_activeItinerary, -1) && ItineraryView.FULL) || ItineraryView.LIST ) } From 3deb8844d8b0e584b15d247bab294d214a2af205 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 18 Aug 2023 16:27:18 -0400 Subject: [PATCH 3/4] refactor(view-switcher): Remove highlight in accounts view. Remove history hook. --- lib/components/app/view-switcher.tsx | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/lib/components/app/view-switcher.tsx b/lib/components/app/view-switcher.tsx index d24615e13..39902f920 100644 --- a/lib/components/app/view-switcher.tsx +++ b/lib/components/app/view-switcher.tsx @@ -1,11 +1,10 @@ import { Button } from 'react-bootstrap' import { connect } from 'react-redux' import { FormattedMessage, useIntl } from 'react-intl' -import { useHistory } from 'react-router' import React from 'react' +import * as uiActions from '../../actions/ui' import { MainPanelContent } from '../../actions/ui-constants' -import { setMainPanelContent } from '../../actions/ui' type Props = { accountsActive: boolean @@ -23,7 +22,6 @@ const ViewSwitcher = ({ setMainPanelContent, sticky }: Props) => { - const history = useHistory() const intl = useIntl() const _showRouteViewer = () => { @@ -31,15 +29,11 @@ const ViewSwitcher = ({ } const _showTripPlanner = () => { - if (accountsActive) { - // Go up to root while preserving query parameters - history.push('..' + history.location.search) - } else { - setMainPanelContent(null) - } + // setMainPanelContent(null) already includes navigation to '/'. + setMainPanelContent(null) } - const tripPlannerActive = activePanel === null + const tripPlannerActive = activePanel === null && !accountsActive const routeViewerActive = activePanel === MainPanelContent.ROUTE_VIEWER return ( @@ -102,7 +96,7 @@ const mapStateToProps = (state: any) => { } const mapDispatchToProps = { - setMainPanelContent + setMainPanelContent: uiActions.setMainPanelContent } export default connect(mapStateToProps, mapDispatchToProps)(ViewSwitcher) From d92148be25f3730758cbf5e6dda3b7f9665cd462 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 18 Aug 2023 16:56:09 -0400 Subject: [PATCH 4/4] fix(actions/ui): Allow setting null main panel when in accounts page. --- lib/actions/ui.js | 10 +++++++--- lib/components/app/view-switcher.tsx | 12 +----------- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/lib/actions/ui.js b/lib/actions/ui.js index 4df30e074..43cf75b58 100644 --- a/lib/actions/ui.js +++ b/lib/actions/ui.js @@ -118,12 +118,16 @@ export function setViewedTrip(payload) { export function setMainPanelContent(payload) { return function (dispatch, getState) { const { otp, router } = getState() - if (otp.ui.mainPanelContent === payload) { + if ( + otp.ui.mainPanelContent === payload && + (payload || router.location.pathname === '/') + ) { + // Do nothing if the panel is already set (but do something if changing to main panel + // and the router path is not at the root (/#/?ui_activeSearch=...). + // This will guard against over-enthusiastic routing and overwriting current/nested states. console.warn( `Attempt to route from ${otp.ui.mainPanelContent} to ${payload}. Doing nothing` ) - // Do nothing if the panel is already set. This will guard against over - // enthusiastic routing and overwriting current/nested states. return } dispatch(setPanel(payload)) diff --git a/lib/components/app/view-switcher.tsx b/lib/components/app/view-switcher.tsx index 39902f920..801e5954e 100644 --- a/lib/components/app/view-switcher.tsx +++ b/lib/components/app/view-switcher.tsx @@ -78,20 +78,10 @@ const ViewSwitcher = ({ // connect to the redux store const mapStateToProps = (state: any) => { - const { mainPanelContent } = state.otp.ui - - // Reverse the ID to string mapping - const activePanelPair = Object.entries(MainPanelContent).find( - (keyValuePair) => keyValuePair[1] === mainPanelContent - ) - // activePanel is array of form [string, ID] - // The trip planner has id null - const activePanel = (activePanelPair && activePanelPair[1]) || null - return { // TODO: more reliable way of detecting these things, such as terms of storage page accountsActive: state.router.location.pathname.includes('/account'), - activePanel + activePanel: state.otp.ui.mainPanelContent } }