Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OTP2 route support in Transitive #792

Merged

Conversation

binh-dam-ibigroup
Copy link
Collaborator

@binh-dam-ibigroup binh-dam-ibigroup commented Oct 28, 2024

This PR adds support for OTP2 leg routes, so that route colors and names from OTP2-returned itineraries render correctly. A new mock is added to itinerary-body but that's only for Storybook for transitive-overlay (i.e. dev and test time), so no new package release for itinerary-body will be created.

I have removed map coordinates and zoom from transitive-overlay stories as these story parameters don't seem to affect rendering - the map is autofitted when a new itinerary is rendered.

Copy link
Contributor

@amy-corson-ibigroup amy-corson-ibigroup left a comment

Choose a reason for hiding this comment

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

Looks really good! Just one cleanup item but nothing blocking. Thanks

Comment on lines 407 to 410
agency_id: leg.agencyId,
route_id: routeId,
route_short_name: routeLabel,
route_long_name: leg.route.longName || "",
Copy link
Contributor

Choose a reason for hiding this comment

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

These four are the same in both cases, can we extract these into their own constant and conditionally add the type, color, and text color?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I separated the common attributes in a022c74.

Copy link
Contributor

@daniel-heppner-ibigroup daniel-heppner-ibigroup left a comment

Choose a reason for hiding this comment

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

Code looks good, I just wish this otp1/otp2 stuff wasn't so messy. :/

@binh-dam-ibigroup binh-dam-ibigroup merged commit f28b99f into opentripplanner:master Nov 13, 2024
2 checks passed
@binh-dam-ibigroup binh-dam-ibigroup deleted the transitive-routes-otp2 branch November 13, 2024 14:17
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.

3 participants