From 4b0ff358958001e7fb8ea70cc86770a10e06e667 Mon Sep 17 00:00:00 2001 From: amy-corson-ibigroup <115499534+amy-corson-ibigroup@users.noreply.github.com> Date: Wed, 16 Oct 2024 13:26:16 -0500 Subject: [PATCH 1/6] feat(advanced-trip-form): Add autoplan --- lib/components/app/batch-routing-panel.tsx | 24 +++++++++++++++++-- .../form/advanced-settings-panel.tsx | 18 +++++++++++--- lib/components/form/batch-settings.tsx | 6 +---- 3 files changed, 38 insertions(+), 10 deletions(-) diff --git a/lib/components/app/batch-routing-panel.tsx b/lib/components/app/batch-routing-panel.tsx index 5c6a69c73..6ca1f6da2 100644 --- a/lib/components/app/batch-routing-panel.tsx +++ b/lib/components/app/batch-routing-panel.tsx @@ -3,12 +3,14 @@ import { CSSTransition, TransitionGroup } from 'react-transition-group' import { FormattedMessage, injectIntl, IntlShape } from 'react-intl' import React, { Component, FormEvent } from 'react' +import * as apiActions from '../../actions/api' import { advancedPanelClassName, mainPanelClassName, transitionDuration, TransitionStyles } from '../form/styled' +import { alertUserTripPlan } from '../form/util' import { getActiveSearch, getShowUserSettings } from '../../util/state' import { getPersistenceMode } from '../../util/user' import AdvancedSettingsPanel from '../form/advanced-settings-panel' @@ -22,9 +24,11 @@ import ViewerContainer from '../viewers/viewer-container' interface Props { activeSearch: any + currentQuery: any intl: IntlShape mainPanelContent: number mobile?: boolean + routingQuery: () => void showUserSettings: boolean } @@ -75,7 +79,13 @@ class BatchRoutingPanel extends Component { handleSubmit = (e: FormEvent) => e.preventDefault() handlePlanTripClick = () => { - this.setState({ planTripClicked: true }) + const { currentQuery, intl, routingQuery } = this.props + alertUserTripPlan( + intl, + currentQuery, + () => this.setState({ planTripClicked: true }), + routingQuery + ) } render() { @@ -128,6 +138,7 @@ class BatchRoutingPanel extends Component { > { (state.user.loggedInUser?.hasConsentedToTerms || getPersistenceMode(state.otp.config.persistence).isLocalStorage) const { mainPanelContent } = state.otp.ui + const currentQuery = state.otp.currentQuery return { activeSearch: getActiveSearch(state), + currentQuery, mainPanelContent, showUserSettings } } -export default connect(mapStateToProps)(injectIntl(BatchRoutingPanel)) +const mapDispatchToProps = { + routingQuery: apiActions.routingQuery +} + +export default connect( + mapStateToProps, + mapDispatchToProps +)(injectIntl(BatchRoutingPanel)) diff --git a/lib/components/form/advanced-settings-panel.tsx b/lib/components/form/advanced-settings-panel.tsx index ea085d017..97662470f 100644 --- a/lib/components/form/advanced-settings-panel.tsx +++ b/lib/components/form/advanced-settings-panel.tsx @@ -111,8 +111,10 @@ const DtSelectorContainer = styled.div` ` const AdvancedSettingsPanel = ({ + autoPlan, closeAdvancedSettings, enabledModeButtons, + handlePlanTrip, innerRef, modeButtonOptions, modeSettingDefinitions, @@ -121,8 +123,10 @@ const AdvancedSettingsPanel = ({ setCloseAdvancedSettingsWithDelay, setQueryParam }: { + autoPlan: boolean closeAdvancedSettings: () => void enabledModeButtons: string[] + handlePlanTrip: () => void innerRef: RefObject modeButtonOptions: ModeButtonDefinition[] modeSettingDefinitions: ModeSetting[] @@ -177,11 +181,16 @@ const AdvancedSettingsPanel = ({ ) ) + const closePanel = useCallback(() => { + autoPlan && handlePlanTrip() + closeAdvancedSettings() + }, [autoPlan, closeAdvancedSettings, handlePlanTrip]) + const onSaveAndReturnClick = useCallback(async () => { await setCloseAdvancedSettingsWithDelay() setClosingBySave(true) - closeAdvancedSettings() - }, [closeAdvancedSettings, setCloseAdvancedSettingsWithDelay]) + closePanel() + }, [closePanel, setCloseAdvancedSettingsWithDelay]) return ( @@ -190,7 +199,7 @@ const AdvancedSettingsPanel = ({ aria-label={closeButtonText} id="close-advanced-settings-button" onClick={() => { - closeAdvancedSettings() + closePanel() }} title={closeButtonText} > @@ -256,9 +265,12 @@ const mapStateToProps = (state: AppReduxState) => { state.otp.modeSettingDefinitions || [], modes?.initialState?.modeSettingValues || {} ) + + const { autoPlan } = state.otp.config const saveAndReturnButton = state.otp.config?.advancedSettingsPanel?.saveAndReturnButton return { + autoPlan: autoPlan !== false, currentQuery: state.otp.currentQuery, // TODO: Duplicated in apiv2.js enabledModeButtons: diff --git a/lib/components/form/batch-settings.tsx b/lib/components/form/batch-settings.tsx index 23bd13a6c..12f036491 100644 --- a/lib/components/form/batch-settings.tsx +++ b/lib/components/form/batch-settings.tsx @@ -80,10 +80,6 @@ function BatchSettings({ pipe(addModeButtonIcon(ModeIcon), setModeButtonEnabled(enabledModeButtons)) ) - const _planTrip = useCallback(() => { - alertUserTripPlan(intl, currentQuery, onPlanTripClick, routingQuery) - }, [currentQuery, intl, onPlanTripClick, routingQuery]) - const baseColor = getBaseColor() const accentColor = getDarkenedBaseColor() @@ -109,7 +105,7 @@ function BatchSettings({ /> Date: Wed, 16 Oct 2024 17:49:15 -0500 Subject: [PATCH 2/6] Prevent autoplan if there are validation errors --- .../form/advanced-settings-panel.tsx | 12 ++++++++--- lib/components/form/util.tsx | 20 +++++++++++++------ 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/lib/components/form/advanced-settings-panel.tsx b/lib/components/form/advanced-settings-panel.tsx index 97662470f..0d951f57f 100644 --- a/lib/components/form/advanced-settings-panel.tsx +++ b/lib/components/form/advanced-settings-panel.tsx @@ -30,7 +30,8 @@ import { onSettingsUpdate, pipe, populateSettingWithIcon, - setModeButton + setModeButton, + tripPlannerValidationErrors } from './util' import { setModeButtonEnabled } from './batch-settings' import { styledCheckboxCss } from './styled' @@ -113,6 +114,7 @@ const DtSelectorContainer = styled.div` const AdvancedSettingsPanel = ({ autoPlan, closeAdvancedSettings, + currentQuery, enabledModeButtons, handlePlanTrip, innerRef, @@ -125,6 +127,7 @@ const AdvancedSettingsPanel = ({ }: { autoPlan: boolean closeAdvancedSettings: () => void + currentQuery: any enabledModeButtons: string[] handlePlanTrip: () => void innerRef: RefObject @@ -181,10 +184,13 @@ const AdvancedSettingsPanel = ({ ) ) + const tripFormErrors = tripPlannerValidationErrors(currentQuery, intl) + const closePanel = useCallback(() => { - autoPlan && handlePlanTrip() + // Only autoplan if there are no validation errors + tripFormErrors.length === 0 && autoPlan && handlePlanTrip() closeAdvancedSettings() - }, [autoPlan, closeAdvancedSettings, handlePlanTrip]) + }, [autoPlan, closeAdvancedSettings, handlePlanTrip, tripFormErrors.length]) const onSaveAndReturnClick = useCallback(async () => { await setCloseAdvancedSettingsWithDelay() diff --git a/lib/components/form/util.tsx b/lib/components/form/util.tsx index 85dd3107d..c973af17c 100644 --- a/lib/components/form/util.tsx +++ b/lib/components/form/util.tsx @@ -72,13 +72,10 @@ export const setModeButton = ) } -export const alertUserTripPlan = ( - intl: IntlShape, +export const tripPlannerValidationErrors = ( currentQuery: any, - onPlanTripClick: () => void, - routingQuery: () => any -): void => { - // Check for any validation issues in query. + intl: IntlShape +): string[] => { const issues: string[] = [] if (!hasValidLocation(currentQuery, 'from')) { issues.push(intl.formatMessage({ id: 'components.BatchSettings.origin' })) @@ -88,6 +85,17 @@ export const alertUserTripPlan = ( intl.formatMessage({ id: 'components.BatchSettings.destination' }) ) } + return issues +} + +export const alertUserTripPlan = ( + intl: IntlShape, + currentQuery: any, + onPlanTripClick: () => void, + routingQuery: () => any +): void => { + // Check for any validation issues in query and alert user. + const issues = tripPlannerValidationErrors(currentQuery, intl) onPlanTripClick() if (issues.length > 0) { // TODO: replace with less obtrusive validation. From d2a884f3a502ab723a8054f798631170b2b13017 Mon Sep 17 00:00:00 2001 From: amy-corson-ibigroup <115499534+amy-corson-ibigroup@users.noreply.github.com> Date: Wed, 16 Oct 2024 18:10:05 -0500 Subject: [PATCH 3/6] Pass plan trip through mobile --- lib/components/mobile/batch-search-screen.tsx | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/lib/components/mobile/batch-search-screen.tsx b/lib/components/mobile/batch-search-screen.tsx index c24b4dacb..89b14a433 100644 --- a/lib/components/mobile/batch-search-screen.tsx +++ b/lib/components/mobile/batch-search-screen.tsx @@ -4,6 +4,7 @@ import { injectIntl, IntlShape } from 'react-intl' import React, { Component } from 'react' import styled from 'styled-components' +import * as apiActions from '../../actions/api' import * as uiActions from '../../actions/ui' import { advancedPanelClassName, @@ -11,6 +12,7 @@ import { transitionDuration, TransitionStyles } from '../form/styled' +import { alertUserTripPlan } from '../form/util' import { MobileScreens } from '../../actions/ui-constants' import AdvancedSettingsPanel from '../form/advanced-settings-panel' import BatchSettings from '../form/batch-settings' @@ -43,8 +45,10 @@ const MobileSearchSettings = styled.div<{ ` interface Props { + currentQuery: any intl: IntlShape map: React.ReactElement + routingQuery: any setMobileScreen: (screen: number) => void } @@ -63,7 +67,10 @@ class BatchSearchScreen extends Component { _advancedSettingRef = React.createRef() handlePlanTripClick = () => { - this.setState({ planTripClicked: true }) + const { currentQuery, intl, routingQuery } = this.props + alertUserTripPlan(intl, currentQuery, routingQuery, () => + this.setState({ planTripClicked: true }) + ) } openAdvancedSettings = () => { @@ -159,6 +166,7 @@ class BatchSearchScreen extends Component { > { // connect to the redux store +const mapStateToProps = (state: any) => { + const { currentQuery } = state.otp.currentQuery + return { + currentQuery + } +} + const mapDispatchToProps = { + routingQuery: apiActions.routingQuery, setMobileScreen: uiActions.setMobileScreen } -export default connect(null, mapDispatchToProps)(injectIntl(BatchSearchScreen)) +export default connect( + mapStateToProps, + mapDispatchToProps +)(injectIntl(BatchSearchScreen)) From 779615c055d8c6ad1a9f0724d8485bc8f534fdd6 Mon Sep 17 00:00:00 2001 From: amy-corson-ibigroup <115499534+amy-corson-ibigroup@users.noreply.github.com> Date: Wed, 16 Oct 2024 18:18:03 -0500 Subject: [PATCH 4/6] remove unnecessary props --- lib/components/form/batch-settings.tsx | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/lib/components/form/batch-settings.tsx b/lib/components/form/batch-settings.tsx index 12f036491..47700fdeb 100644 --- a/lib/components/form/batch-settings.tsx +++ b/lib/components/form/batch-settings.tsx @@ -5,9 +5,8 @@ import { ModeButtonDefinition } from '@opentripplanner/types' import { Search } from '@styled-icons/fa-solid/Search' import { SyncAlt } from '@styled-icons/fa-solid/SyncAlt' import { useIntl } from 'react-intl' -import React, { useCallback, useContext, useState } from 'react' +import React, { useContext, useState } from 'react' -import * as apiActions from '../../actions/api' import * as formActions from '../../actions/form' import { ComponentContext } from '../../util/contexts' import { getActiveSearch, hasValidLocation } from '../../util/state' @@ -16,7 +15,6 @@ import { StyledIconWrapper } from '../util/styledIcon' import { addModeButtonIcon, - alertUserTripPlan, modesQueryParamConfig, onSettingsUpdate, pipe, @@ -39,7 +37,6 @@ type Props = { modeButtonOptions: ModeButtonDefinition[] onPlanTripClick: () => void openAdvancedSettings: () => void - routingQuery: any setQueryParam: (evt: any) => void spacedOutModeSelector?: boolean } @@ -64,7 +61,6 @@ function BatchSettings({ modeButtonOptions, onPlanTripClick, openAdvancedSettings, - routingQuery, setQueryParam, spacedOutModeSelector }: Props) { @@ -147,7 +143,6 @@ const mapStateToProps = (state: any) => { } const mapDispatchToProps = { - routingQuery: apiActions.routingQuery, setQueryParam: formActions.setQueryParam } From 15252bad991f5215820e7217f87519356ed8d53e Mon Sep 17 00:00:00 2001 From: amy-corson-ibigroup <115499534+amy-corson-ibigroup@users.noreply.github.com> Date: Wed, 30 Oct 2024 12:41:19 -0500 Subject: [PATCH 5/6] Fix mapstatetoprops incorrect destructure --- lib/components/mobile/batch-search-screen.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/components/mobile/batch-search-screen.tsx b/lib/components/mobile/batch-search-screen.tsx index 89b14a433..5be2cc8c7 100644 --- a/lib/components/mobile/batch-search-screen.tsx +++ b/lib/components/mobile/batch-search-screen.tsx @@ -189,7 +189,7 @@ class BatchSearchScreen extends Component { // connect to the redux store const mapStateToProps = (state: any) => { - const { currentQuery } = state.otp.currentQuery + const currentQuery = state.otp.currentQuery return { currentQuery } From a00d5b5e38fcf24be27c2e6af6a7c6ccd6b73be6 Mon Sep 17 00:00:00 2001 From: amy-corson-ibigroup <115499534+amy-corson-ibigroup@users.noreply.github.com> Date: Wed, 30 Oct 2024 12:42:52 -0500 Subject: [PATCH 6/6] Fix percy tests and add openEditIfNeeded function --- percy/percy.test.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/percy/percy.test.js b/percy/percy.test.js index 37433032d..957f07e43 100644 --- a/percy/percy.test.js +++ b/percy/percy.test.js @@ -42,6 +42,12 @@ async function loadPath(otpPath) { return page } +const openEditIfNeeded = async (page, isMobile) => { + if (isMobile) { + await page.click('button.edit-search-button') + } +} + beforeAll(async () => { try { // Launch OTP-RR web server @@ -114,9 +120,7 @@ async function executeTest(page, isMobile, isCallTaker) { if (!isCallTaker) { // Edit trip params [mobile-specific] - if (isMobile) { - await page.click('button.edit-search-button') - } + await openEditIfNeeded(page, isMobile) // Change the modes: Activate Transit and remove Bike. await page.click('label[title="Transit"]') @@ -148,6 +152,8 @@ async function executeTest(page, isMobile, isCallTaker) { await page.waitForTimeout(500) // Delete both origin and destination + await openEditIfNeeded(page, isMobile) + await page.click('.from-form-control') await page.waitForTimeout(300) // Click the clear button next to it