Nearby view support #655
Nearby view support #655
Conversation
daniel-heppner-ibigroup
force-pushed
the
nearby-view-support
branch
from
8e11fa3 to
a4483f6
Compare
|
@miles-grant-ibigroup @daniel-heppner-ibigroup Is this one of these PRs that need to be split, especially for the types package, so that a version number is available for other packages to use as reference? |
| import { FormattedMessage } from "react-intl"; | ||
|
|
||
| import * as S from "../styled"; | ||
| import { defaultMessages } from "../util"; | ||
|
|
||
| interface Props { | ||
| onStopClick: ({ stopId: string }) => void; | ||
| stopId: string; | ||
| onStopClick: (stop: Stop) => void; |
There was a problem hiding this comment.
This is a breaking change. Either:
- Add breaking change commits to all packages that use this new method signature, or,
- If maintaining compatibility, change the signature of
onStopClickto(stop: Stop | { stopId: string })(You could create an aliastype StopEventHanlder = (stop: Stop | { stopId: string }) => voidin the types package to avoid repetition.)
There was a problem hiding this comment.
That's a great idea! I'll add it. Thank you for the suggestion
|
Merge conflicts resolved, ready for another look @binh-dam-ibigroup |
|
That's embarrassing. I assigned it back to you before I finished my commits. |
daniel-heppner-ibigroup
assigned binh-dam-ibigroup and unassigned daniel-heppner-ibigroup
| import { FormattedMessage } from "react-intl"; | ||
|
|
||
| import * as S from "../styled"; | ||
| import { defaultMessages } from "../util"; | ||
|
|
||
| interface Props { | ||
| onStopClick: ({ stopId: string }) => void; | ||
| stopId: string; |
There was a problem hiding this comment.
Keep this prop (as a fallback) otherwise it is a breaking change.
There was a problem hiding this comment.
Made it an either or type, let me know if that's good
| @@ -16,10 +16,10 @@ | |||
| "react-map-gl": "^7.0.15" | |||
| }, | |||
| "dependencies": { | |||
| "@opentripplanner/map-popup": "^2.0.5" | |||
| "@opentripplanner/map-popup": "^2.0.7-alpha.1" | |||
There was a problem hiding this comment.
The tile overlay package should probably be released from a separate PR after the map popup package is released.
| "@opentripplanner/map-popup": "^2.0.5", | ||
| "@opentripplanner/base-map": "^3.0.15", | ||
| "@opentripplanner/from-to-location-picker": "^2.1.10", | ||
| "@opentripplanner/map-popup": "^2.0.7-alpha.1", |
There was a problem hiding this comment.
The stops overlay package should probably be released from a separate PR after the map popup package is released.
|
I can't update the Map Overlay package first because it breaks the types in other packages, since the callback can now return a Stop. I think that means a breaking change is unavoidable in Map Overlay and they have to be released with alpha packages. Sidenote, this is why upgrading to a more modern package manager such as yarn 3 or pnpm would be advantageous, since you can put |
daniel-heppner-ibigroup
assigned binh-dam-ibigroup and unassigned daniel-heppner-ibigroup
packages/itinerary-body/src/otp-react-redux/view-stop-button.tsx
Outdated
Show resolved
Hide resolved
That's fine, but then upgrade to the non-alpha versions soon after they are released. |
BREAKING CHANGE: Callback handler for setViewedStop needs to handle a Stop object
daniel-heppner-ibigroup
assigned miles-grant-ibigroup and unassigned daniel-heppner-ibigroup
miles-grant-ibigroup
assigned daniel-heppner-ibigroup and unassigned miles-grant-ibigroup
# Conflicts: # yarn.lock
daniel-heppner-ibigroup
merged commit d76d949
into
opentripplanner:master
This PR adjusts all the places where the stop viewer is called to pass in the whole stop object rather than just the stop ID string. This allows support for the nearby view in OTP-RR, which relies on the lat/lon of the stop rather than its ID.