Skip to content

Conversation

alec-georgoff
Copy link
Collaborator

@alec-georgoff alec-georgoff commented Aug 6, 2025

Description:

This PR adds functionality to automatically select the rental vehicle mode in the trip planner when starting a trip from a selected rental vehicle in Nearby View. The logic respects the existing trip modes that are selected.

Currently blocked until opentripplanner/otp-ui#871 is merged and version bumped.

PR Checklist:

  • Does the code follow accessibility standards (WCAG 2.1 AA Compliant)?
  • Are all languages supported (Internationalization/Localization)?
  • Are appropriate Typescript types implemented?

@alec-georgoff alec-georgoff marked this pull request as ready for review August 6, 2025 22:05
Copy link
Collaborator

@miles-grant-ibigroup miles-grant-ibigroup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few architectural questions

const overlayTypes = overlays
?.filter((overlay) => overlay?.type === 'otp2')?.[0]
?.layers?.map((layer) => layer?.type)
const handleSetLocation = (location: MapLocationActionArg) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is one approach, but could we not fix this by adjusting query params? What if we check for map state in the places where query params are set? Might that be a better separation of concerns than setting modes within the map component?

@@ -486,6 +509,7 @@ const mapStateToProps = (state) => {
bikeRentalStations: state.otp.overlay.bikeRental.stations,
carRentalStations: state.otp.overlay.carRental.stations,
config: state.otp.config,
currentQuery: state.otp.currentQuery,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a huge fan of importing query info into the map component...

@alec-georgoff alec-georgoff added the BLOCKED Blocked (waiting on another PR to be merged) label Aug 20, 2025
@alec-georgoff alec-georgoff removed the BLOCKED Blocked (waiting on another PR to be merged) label Aug 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants