From 56f43aa60534bba0c4769c2ddff3140480ec04e2 Mon Sep 17 00:00:00 2001 From: miles-grant-ibigroup Date: Fri, 17 Nov 2023 11:17:01 -0500 Subject: [PATCH 1/3] potential approach for restoring auto-highlight behavior --- lib/actions/api.js | 4 --- .../narrative/narrative-itineraries.js | 32 +++++++++++++++++-- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/lib/actions/api.js b/lib/actions/api.js index dafc7cc42..7dafff4d4 100644 --- a/lib/actions/api.js +++ b/lib/actions/api.js @@ -12,7 +12,6 @@ import { getSecureFetchOptions } from '../util/middleware' import { getStopViewerConfig } from '../util/state' import { MainPanelContent } from './ui-constants' -import { setVisibleItinerary } from './narrative' import v1Actions from './apiV1' import v2Actions from './apiV2' @@ -139,9 +138,6 @@ export function updateOtpUrlParams(state, searchId) { params.ui_activeItinerary = -1 // At the same time, reset/delete the ui_itineraryView param. params.ui_itineraryView = undefined - if (config.itinerary?.showFirstResultByDefault) { - dispatch(setVisibleItinerary({ index: 0 })) - } // Merge in the provided OTP params and update the URL. dispatch(setUrlSearch(Object.assign(params, otpParams))) } diff --git a/lib/components/narrative/narrative-itineraries.js b/lib/components/narrative/narrative-itineraries.js index 6d20a2437..80eec5c36 100644 --- a/lib/components/narrative/narrative-itineraries.js +++ b/lib/components/narrative/narrative-itineraries.js @@ -331,13 +331,16 @@ class NarrativeItineraries extends Component { ) } - componentDidUpdate() { + componentDidUpdate(prevProps) { // If set in URL, set the active itinerary in the state, once. const { activeItinerary, activeSearch, + itineraryConfig, + mergedItineraries, setActiveItinerary, - setVisibleItinerary + setVisibleItinerary, + visibleItinerary } = this.props const { ui_activeItinerary: uiActiveItinerary } = coreUtils.query.getUrlParams() || {} @@ -350,6 +353,28 @@ class NarrativeItineraries extends Component { setActiveItinerary({ index: +uiActiveItinerary }) setVisibleItinerary({ index: +uiActiveItinerary }) } + + /** + * Show first result by default is a lot more complicated now that we have + * fixed indeces. We must update the visible itinerary here instead of in the redux state. + * Also, we need to update whenever new items arrive, to make sure that the highlighted + * itinerary is indeed the first one. + * + * Finally, we need to make sure we only update if the data changes, not if the user actually + * highlighted something else on their own. + */ + if (itineraryConfig?.showFirstResultByDefault) { + const firstSortedItin = + mergedItineraries?.length > 0 && mergedItineraries?.[0].index + + if ( + activeItinerary === -1 && + (visibleItinerary === null || visibleItinerary === false) && + prevProps.mergedItineraries.length !== mergedItineraries.length + ) { + setVisibleItinerary({ index: firstSortedItin }) + } + } } // eslint-disable-next-line complexity @@ -531,7 +556,7 @@ const reduceErrorsFromResponse = (acc, cur) => { const mapStateToProps = (state) => { const { config, filter } = state.otp - const { co2, errorMessages, modes } = config + const { co2, errorMessages, itinerary, modes } = config const { sort } = filter const activeSearch = getActiveSearch(state) @@ -590,6 +615,7 @@ const mapStateToProps = (state) => { groupItineraries, groupTransitModes, itineraries: allItineraries, + itineraryConfig: itinerary, itineraryIsExpanded, // use a key so that the NarrativeItineraries component and its state is // reset each time a new search is shown From b12a1327b34e700885fa44e2e875c6ba3e053a33 Mon Sep 17 00:00:00 2001 From: miles-grant-ibigroup Date: Fri, 17 Nov 2023 11:26:21 -0500 Subject: [PATCH 2/3] appease codespell --- lib/components/narrative/narrative-itineraries.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/components/narrative/narrative-itineraries.js b/lib/components/narrative/narrative-itineraries.js index 80eec5c36..4fa12dbad 100644 --- a/lib/components/narrative/narrative-itineraries.js +++ b/lib/components/narrative/narrative-itineraries.js @@ -356,7 +356,7 @@ class NarrativeItineraries extends Component { /** * Show first result by default is a lot more complicated now that we have - * fixed indeces. We must update the visible itinerary here instead of in the redux state. + * fixed indices. We must update the visible itinerary here instead of in the redux state. * Also, we need to update whenever new items arrive, to make sure that the highlighted * itinerary is indeed the first one. * From f1161015fd5035f478f237ae6d1fd91b96b76d46 Mon Sep 17 00:00:00 2001 From: miles-grant-ibigroup <86619099+miles-grant-ibigroup@users.noreply.github.com> Date: Tue, 28 Nov 2023 09:58:35 -0500 Subject: [PATCH 3/3] address pr feedback Co-authored-by: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> --- lib/components/narrative/narrative-itineraries.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/components/narrative/narrative-itineraries.js b/lib/components/narrative/narrative-itineraries.js index 4fa12dbad..d9f12359a 100644 --- a/lib/components/narrative/narrative-itineraries.js +++ b/lib/components/narrative/narrative-itineraries.js @@ -355,7 +355,7 @@ class NarrativeItineraries extends Component { } /** - * Show first result by default is a lot more complicated now that we have + * Showing the first result by default is a lot more complicated now that we have * fixed indices. We must update the visible itinerary here instead of in the redux state. * Also, we need to update whenever new items arrive, to make sure that the highlighted * itinerary is indeed the first one. @@ -364,15 +364,14 @@ class NarrativeItineraries extends Component { * highlighted something else on their own. */ if (itineraryConfig?.showFirstResultByDefault) { - const firstSortedItin = - mergedItineraries?.length > 0 && mergedItineraries?.[0].index - if ( activeItinerary === -1 && (visibleItinerary === null || visibleItinerary === false) && prevProps.mergedItineraries.length !== mergedItineraries.length ) { - setVisibleItinerary({ index: firstSortedItin }) + setVisibleItinerary({ + index: mergedItineraries?.length > 0 && mergedItineraries?.[0].index + }) } } }