From 413217c675239e8d3904239077a36b20b01b14ec Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 6 Oct 2023 16:23:28 -0400 Subject: [PATCH 1/5] refactor(util/ui): Extract method for determining next itin view. --- __tests__/util/ui.ts | 34 +++++++++++++++++++++++++++++++++- lib/util/ui.ts | 22 +++++++++++++++++++++- 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/__tests__/util/ui.ts b/__tests__/util/ui.ts index 813f90d2e..0eb9c61ce 100644 --- a/__tests__/util/ui.ts +++ b/__tests__/util/ui.ts @@ -1,4 +1,8 @@ -import { getItineraryView, ItineraryView } from '../../lib/util/ui' +import { + getItineraryView, + getMapToggleNewItineraryView, + ItineraryView +} from '../../lib/util/ui' describe('util > ui', () => { describe('getItineraryView', () => { @@ -29,6 +33,18 @@ describe('util > ui', () => { ui_itineraryView: ItineraryView.LEG }) ).toBe(ItineraryView.LIST) + expect( + getItineraryView({ + ui_activeItinerary: -1, + ui_itineraryView: ItineraryView.LEG_HIDDEN + }) + ).toBe(ItineraryView.LIST) + expect( + getItineraryView({ + ui_activeItinerary: -1, + ui_itineraryView: ItineraryView.LIST_HIDDEN + }) + ).toBe(ItineraryView.LIST_HIDDEN) }) it('returns the specified view mode when set in URL', () => { expect( @@ -39,4 +55,20 @@ describe('util > ui', () => { ).toBe(ItineraryView.LEG) }) }) + describe('getMapToggleNewItineraryView', () => { + it('should obtain the new itinerary view value', () => { + expect(getMapToggleNewItineraryView(ItineraryView.LEG)).toBe( + ItineraryView.LEG_HIDDEN + ) + expect(getMapToggleNewItineraryView(ItineraryView.LIST)).toBe( + ItineraryView.LIST_HIDDEN + ) + expect(getMapToggleNewItineraryView(ItineraryView.LEG_HIDDEN)).toBe( + ItineraryView.LEG + ) + expect(getMapToggleNewItineraryView(ItineraryView.LIST_HIDDEN)).toBe( + ItineraryView.LIST + ) + }) + }) }) diff --git a/lib/util/ui.ts b/lib/util/ui.ts index 54dba6d99..e2dd8bd7e 100644 --- a/lib/util/ui.ts +++ b/lib/util/ui.ts @@ -113,9 +113,29 @@ export function getItineraryView({ ((ui_activeItinerary === null || ui_activeItinerary === undefined || `${ui_activeItinerary}` === '-1') && - ItineraryView.LIST) || + (ui_itineraryView === ItineraryView.LIST_HIDDEN + ? ItineraryView.LIST_HIDDEN + : ItineraryView.LIST)) || ui_itineraryView || (isDefinedAndNotEqual(ui_activeItinerary, -1) && ItineraryView.FULL) || ItineraryView.LIST ) } + +/** + * Gets the new itinerary view to display based on current state and URL params. + */ +export function getMapToggleNewItineraryView( + currentView: ItineraryView +): ItineraryView { + switch (currentView) { + case ItineraryView.LEG: + return ItineraryView.LEG_HIDDEN + case ItineraryView.LIST: + return ItineraryView.LIST_HIDDEN + case ItineraryView.LEG_HIDDEN: + return ItineraryView.LEG + default: + return ItineraryView.LIST + } +} From a200fcf29f05d41c316dafb5fa5d8941ee88a162 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 6 Oct 2023 16:29:53 -0400 Subject: [PATCH 2/5] fix(actions/ui): Correctly set the next itin view state. --- lib/actions/apiV2.js | 2 -- lib/actions/ui.js | 33 +++++++------------ lib/components/mobile/batch-results-screen.js | 2 +- lib/reducers/create-otp-reducer.js | 4 +-- 4 files changed, 15 insertions(+), 26 deletions(-) diff --git a/lib/actions/apiV2.js b/lib/actions/apiV2.js index ae64d539c..3fdd4c714 100644 --- a/lib/actions/apiV2.js +++ b/lib/actions/apiV2.js @@ -878,8 +878,6 @@ export function routingQuery(searchId = null, updateSearchInReducer) { }) ) - dispatch(setItineraryView(ItineraryView.LIST)) - combinations.forEach((combo, index) => { const query = generateOtp2Query(combo) dispatch( diff --git a/lib/actions/ui.js b/lib/actions/ui.js index 3b254fa61..e50013218 100644 --- a/lib/actions/ui.js +++ b/lib/actions/ui.js @@ -9,12 +9,14 @@ import { getMatchingLocaleString, loadLocaleData } from '../util/i18n' -import { getModesForActiveAgencyFilter, getUiUrlParams } from '../util/state' import { + getItineraryView, + getMapToggleNewItineraryView, getPathFromParts, isDefinedAndNotEqual, ItineraryView } from '../util/ui' +import { getModesForActiveAgencyFilter, getUiUrlParams } from '../util/state' import { clearActiveSearch, @@ -39,7 +41,7 @@ export const setHoveredStop = createAction('SET_HOVERED_STOP') const viewTrip = createAction('SET_VIEWED_TRIP') const viewRoute = createAction('SET_VIEWED_ROUTE') export const toggleAutoRefresh = createAction('TOGGLE_AUTO_REFRESH') -const setPreviousItineraryView = createAction('SET_PREVIOUS_ITINERARY_VIEW') +const settingItineraryView = createAction('SET_ITINERARY_VIEW') export const setPopupContent = createAction('SET_POPUP_CONTENT') // This code-less action calls the reducer code @@ -314,21 +316,15 @@ export function handleBackButtonPress(e) { export function setItineraryView(value) { return function (dispatch, getState) { const urlParams = coreUtils.query.getUrlParams() - const prevItineraryView = urlParams.ui_itineraryView || ItineraryView.LIST // If the itinerary value is changed, - // set the desired ui query param + // set the desired ui query param (even if LIST, so it replaces the current value) // and store the current view as previousItineraryView. - if (value !== urlParams.ui_itineraryView) { - if (value !== ItineraryView.LIST) { - urlParams.ui_itineraryView = value - } else if (urlParams.ui_itineraryView) { - // Remove the ui_itineraryView param if it is set to LIST (default). - delete urlParams.ui_itineraryView - } + if (value !== getItineraryView(urlParams)) { + urlParams.ui_itineraryView = value dispatch(setUrlSearch(urlParams)) - dispatch(setPreviousItineraryView(prevItineraryView)) + dispatch(settingItineraryView(value)) } } } @@ -340,16 +336,11 @@ export function setItineraryView(value) { export function toggleBatchResultsMap() { return function (dispatch, getState) { const urlParams = coreUtils.query.getUrlParams() - const itineraryView = urlParams.ui_itineraryView || ItineraryView.LIST + const itineraryView = getItineraryView(urlParams) - 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)) - } + const newView = getMapToggleNewItineraryView(itineraryView) + console.log(`itin view: ${itineraryView}=>${newView}`) + dispatch(setItineraryView(newView)) } } diff --git a/lib/components/mobile/batch-results-screen.js b/lib/components/mobile/batch-results-screen.js index 2e88d01e9..69119f1c3 100644 --- a/lib/components/mobile/batch-results-screen.js +++ b/lib/components/mobile/batch-results-screen.js @@ -143,7 +143,7 @@ const mapStateToProps = (state) => { activeLeg: activeSearch ? activeSearch.activeLeg : null, errors: getResponsesWithErrors(state), itineraries: getActiveItineraries(state), - itineraryView: getItineraryView(urlParams) + itineraryView: getItineraryView(urlParams) || state.otp.ui.itineraryView } } diff --git a/lib/reducers/create-otp-reducer.js b/lib/reducers/create-otp-reducer.js index 0888474e9..8e7afa35b 100644 --- a/lib/reducers/create-otp-reducer.js +++ b/lib/reducers/create-otp-reducer.js @@ -1053,9 +1053,9 @@ function createOtpReducer(config) { }) case 'UPDATE_ITINERARY_FILTER': return update(state, { filter: { $set: action.payload } }) - case 'SET_PREVIOUS_ITINERARY_VIEW': + case 'SET_ITINERARY_VIEW': return update(state, { - ui: { previousItineraryView: { $set: action.payload } } + ui: { itineraryView: { $set: action.payload } } }) case 'UPDATE_LOCALE': return update(state, { From 2419489228061a810887d8f7a722ad7c7e86aad9 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 6 Oct 2023 16:58:17 -0400 Subject: [PATCH 3/5] test(util/ui): Refactor test. --- __tests__/util/ui.ts | 60 ++++++++++++++++++-------------------------- 1 file changed, 24 insertions(+), 36 deletions(-) diff --git a/__tests__/util/ui.ts b/__tests__/util/ui.ts index 0eb9c61ce..314e604e4 100644 --- a/__tests__/util/ui.ts +++ b/__tests__/util/ui.ts @@ -21,30 +21,21 @@ describe('util > ui', () => { ) }) it('returns an itinerary list view if URL contains ui_activeItinerary=-1 regardless of ui_itineraryView', () => { - expect( - getItineraryView({ - ui_activeItinerary: -1, - ui_itineraryView: ItineraryView.FULL - }) - ).toBe(ItineraryView.LIST) - expect( - getItineraryView({ - ui_activeItinerary: -1, - ui_itineraryView: ItineraryView.LEG - }) - ).toBe(ItineraryView.LIST) - expect( - getItineraryView({ - ui_activeItinerary: -1, - ui_itineraryView: ItineraryView.LEG_HIDDEN - }) - ).toBe(ItineraryView.LIST) - expect( - getItineraryView({ - ui_activeItinerary: -1, - ui_itineraryView: ItineraryView.LIST_HIDDEN - }) - ).toBe(ItineraryView.LIST_HIDDEN) + const expectedValues = { + [ItineraryView.FULL]: ItineraryView.LIST, + [ItineraryView.LEG]: ItineraryView.LIST, + [ItineraryView.LEG_HIDDEN]: ItineraryView.LIST, + [ItineraryView.LIST]: ItineraryView.LIST, + [ItineraryView.LIST_HIDDEN]: ItineraryView.LIST_HIDDEN + } + Object.entries(expectedValues).forEach(([k, v]) => { + expect( + getItineraryView({ + ui_activeItinerary: -1, + ui_itineraryView: k + }) + ).toBe(v) + }) }) it('returns the specified view mode when set in URL', () => { expect( @@ -57,18 +48,15 @@ describe('util > ui', () => { }) describe('getMapToggleNewItineraryView', () => { it('should obtain the new itinerary view value', () => { - expect(getMapToggleNewItineraryView(ItineraryView.LEG)).toBe( - ItineraryView.LEG_HIDDEN - ) - expect(getMapToggleNewItineraryView(ItineraryView.LIST)).toBe( - ItineraryView.LIST_HIDDEN - ) - expect(getMapToggleNewItineraryView(ItineraryView.LEG_HIDDEN)).toBe( - ItineraryView.LEG - ) - expect(getMapToggleNewItineraryView(ItineraryView.LIST_HIDDEN)).toBe( - ItineraryView.LIST - ) + const expectedValues = { + [ItineraryView.LEG]: ItineraryView.LEG_HIDDEN, + [ItineraryView.LEG_HIDDEN]: ItineraryView.LEG, + [ItineraryView.LIST]: ItineraryView.LIST_HIDDEN, + [ItineraryView.LIST_HIDDEN]: ItineraryView.LIST + } + Object.entries(expectedValues).forEach(([k, v]) => { + expect(getMapToggleNewItineraryView(k)).toBe(v) + }) }) }) }) From 6687464d5375b08ca324fe45d791fa58b60a6de0 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 6 Oct 2023 17:17:47 -0400 Subject: [PATCH 4/5] refactor(util/ui): Tweak comment [skip ci] --- lib/util/ui.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/util/ui.ts b/lib/util/ui.ts index e2dd8bd7e..ed992e860 100644 --- a/lib/util/ui.ts +++ b/lib/util/ui.ts @@ -123,7 +123,7 @@ export function getItineraryView({ } /** - * Gets the new itinerary view to display based on current state and URL params. + * Gets the new itinerary view to display based on current view. */ export function getMapToggleNewItineraryView( currentView: ItineraryView From 14fe67998dbdcb275471224baa3bca20c858b4c5 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Mon, 9 Oct 2023 13:41:38 -0400 Subject: [PATCH 5/5] refactor(actions/ui): Remove console statement. --- lib/actions/ui.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/actions/ui.js b/lib/actions/ui.js index e50013218..21856fd9b 100644 --- a/lib/actions/ui.js +++ b/lib/actions/ui.js @@ -339,7 +339,6 @@ export function toggleBatchResultsMap() { const itineraryView = getItineraryView(urlParams) const newView = getMapToggleNewItineraryView(itineraryView) - console.log(`itin view: ${itineraryView}=>${newView}`) dispatch(setItineraryView(newView)) } }