Removing Mapbox Road Hardcoding #43
Conversation
|
I agree that it doesn't make sense to link to a mapbox url that the app doesn't even know how to handle. Instead of removing this logic, is there a possibility of a maplibre hosted style? In theory it seems kind of nice that things could "just work" for library users, but maybe the project isn't interested in the amount of hosting that would imply. |
I thought about this more while working on #45, and personally concluded that injecting default styles in the middle of the code base is probably not the right approach, so I take back my comment. |
| }.filter(\.isMapboxStreets) | ||
|
|
||
| // Add Mapbox Streets if the map does not already have it | ||
| if streetsSources.isEmpty { |
There was a problem hiding this comment.
Do you expect that there are valid reasons for the person to be in this state where they would want to silently proceed without error?
If not, though we don't have a reasonable default layer to fall back to like mapbox did, could we instead help the user out with an assertionFailure or a log message here?
There was a problem hiding this comment.
I don't really see a way or reason for this. I would assume Mapbox added this because they were getting support emails from users saying "my custom style is not showing roads during navigation", and a common reason was that the users had removed or restyled the road layer to be incompatible with the styling of maplibre navigation.
If I'm a german developer and name my sources and layers for roads "strassennetz" things will work perfectly fine - and yet maplibre would now throw a warning that it can't find a road layer. So unless we want to enforce a certain naming scheme for things in the style.json for maplibre navigation, I don't think emitting warnings would be helpful.
|
Good catch to remove this code @hactar 👍. While I fully agree that we should remove this Mapbox specific logic here. There is similar usage here as well: I am currently wondering if we should make this configurable or just completely remove? |
The one that I found should just be plain removed I think, but the one that you found is an interesting one - it tries to modify the street labels to the countries locale they are in so that they match the signs people might see on streets. That could be a useful feature, but we would need to change this so that we can provide maplibre navigation with the layers that contain the road networks, instead of it trying to find the "mapbox road network". And that layer also needs to support "mul" locale for this to work I guess... So that one should be investigated, but I suggest outside this ticket. |
Done. |
|
@boldtrn the conflict has been resolved, I have no write permissions so you or someone else will have to hit the button |
During setting up, maplibre-navigation-ios checks if the mapbox street source is part of the map style. If it is not, it adds the source and creates a new layer. This results in the console throwing network errors as "mapbox://mapbox.mapbox-streets-v7" is not a valid URL in a non mapbox app.
This PR removes this hardcoded check - I guess this made sense when this was only used with mapbox services, but now that people use this with their non mapbox styles, this "hotfixing of the mapstyle" makes no sense.