-
Notifications
You must be signed in to change notification settings - Fork 34
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
Add MapLibreGL #420
Add MapLibreGL #420
Conversation
BREAKING CHANGE: LEAFLET IS REMOVED, some stories are removed
BREAKING CHANGE: removes Leaflet and Leaflet support
BREAKING CHANGE: Removes Leaflet support
BREAKING CHANGE: Leaflet is removed
BREAKING CHANGE: Leaflet is removed BREAKING CHANGE: removes Leaflet; props change
BREAKING CHANGE: Removes Leaflet
BREAKING CHANGE: Removes Leaflet, removes some props
BREAKING CHANGE: removes Leaflet
BREAKING CHANGE: Removes many leaflet style props
BREAKING CHANGE: Removes Leaflet, adjusts path props
# Conflicts: # package.json # packages/transitive-overlay/package.json # yarn.lock
BREAKING CHANGE: Leaflet is removed, some props are removed, some features are removed
This is ready for your review @demory |
Merge conflicts are resolved now, types and core-utils are updated across all packages. |
@miles-grant-ibigroup something I've noticed a couple times in my testing -- in any overlay with a collection of items (stops, vehicles e.g.) and popups for the items, clicking on one item and then on another hides the first popup, but doesn't show the 2nd popup. You have to click the 2nd thing again to show its popup. This is different from the current behavior in leaflet / otp-rr, where one click both hides the 1st popup and shows the 2nd. Is that something that can be recreated here via configuration, or would it need code changes? |
Another popup comment: in the StopsOverlay stories, with the "Default" story, opening a popup and then zooming keeps the popup displayed. But with "Flex Stops", the popup disappears after a zoom change. What accounts for that? |
# Conflicts: # __snapshots__/storybook.test.ts.snap # yarn.lock
This seems to be related to the layer event handling. It would be possible to replicate the old behavior, but we'd have to overwrite MapLibre's map event handler. It's something that's possible, but if we wanted to go forward with it I'd do so in a new PR.
This has to do with the fact that there are multiple layers being rendered at the same time. Each layer has its own event handler and in the flex stops there are a lot that are competing to figure out what's going on. I would see what the behavior is like inside otp-rr before debugging anything too strongly. Again this can be solved with a custom event handler, but I would save that for a separate PR once we see how this behaves inside otp-rr.
There are a lot of text options inside MapLibre. So far I've enabled one that ensures that the text is never upside down. There seem to be others that might do what we want, I'll fiddle around with it some more. That being said, I think it's probably wiser to not spend too much time trying to replicate transitive behavior here and instead work towards getting transitive to render to MapLibre. Once we add webGL as a target renderer for transitive, it should be quite trivial to display it on the map: https://maplibre.org/maplibre-gl-js-docs/example/custom-style-layer/ |
# Conflicts: # __snapshots__/storybook.test.ts.snap
…rendering support
20845ce
to
c39ea21
Compare
c39ea21
to
1ec8b10
Compare
# Conflicts: # packages/endpoints-overlay/src/endpoint.tsx
This PR migrates all overlays to MapLibreGL, and therefore vector tiles.
As the migration is quite significant, the main focus here is to achieve a minimum viable product, while also focusing on making as few changes as possible.
The consequences of this are:
transit-vehicle-overlay
has lost all TriMet-specific features. These features were also quite Leaflet-specific, and will therefore also have to be re-implemented from the ground up. The "new" version of the layer is missing some functionality and especially good stories. This may be addressable in this PR.otp-ui
. Whether this is a good idea, or if perhaps an architecture shift might allow for the de-duplication of this code is another project for a separate PR.