Reimplement label localization atop Expression#2933
Conversation
Ported the NSExpression-based label localization in map SDK v6.x to Expression.
1ec5
added
op-ex
| let streetsSourceIdentifiers: [String] = mapView.streetsSources().map { $0.id } | ||
|
|
||
| for layerInfo in layers where layerInfo.type == "symbol" { | ||
| guard var layer = try? mapView.style.getLayer(with: layerInfo.id, type: SymbolLayer.self).get(), |
There was a problem hiding this comment.
This call fails on styles that set text-field to a formatted expression, such as Streets and Navigation Day: mapbox/mapbox-maps-ios#268. Unfortunately, the label localization feature has no effect in these styles.
There was a problem hiding this comment.
Per mapbox/mapbox-maps-ios#268 (comment), the bug is fixed in v2.0.0-beta.17.
1ec5
added
blocked
|
|
||
| navigationMapView.mapView.on(.styleLoaded, handler: { [weak self] _ in | ||
| guard let self = self else { return } | ||
| self.navigationMapView.localizeLabels() |
There was a problem hiding this comment.
We were also calling localizeLabels() in RouteMapViewController, CarPlayMapViewController and CarPlayNavigationViewController in 1.x.
There was a problem hiding this comment.
It is still being called in RouteMapViewController, CarPlayMapViewController, and CarPlayNavigationViewController, but it must’ve fallen out of the example application by accident.
| @@ -1,5 +1,11 @@ | |||
| import MapboxMaps | |||
|
|
|||
| extension VectorSource { | |||
There was a problem hiding this comment.
Shall we move this VectorSource extension into VectorSource.swift?
| Returns a boolean value indicating whether the tile source is a supported version of the Mapbox Streets source. | ||
| */ | ||
| func isMapboxStreets(_ identifiers: [String]) -> Bool { | ||
| return identifiers.contains("mapbox.mapbox-streets-v8") || identifiers.contains("mapbox.mapbox-streets-v7") |
There was a problem hiding this comment.
Nit: IMO storing identifiers in array would be more agile. In some places we have too many or comparisons.
There was a problem hiding this comment.
These methods were moved from RouteMapViewController. We’ll need to keep an eye on #2902, which also moves them elsewhere.
This might be a good opportunity to drop support for Mapbox Streets v7. We supported both versions of the Streets source initially as a migration path, but I don’t think we need to do so long-term.
|
|
||
| let localizedTextField = textField.localized(into: locale) | ||
| if localizedTextField != textField { | ||
| layer.layout?.textField = .expression(localizedTextField) |
There was a problem hiding this comment.
Does this update work as expected? I thought that preferred way to update layers would be to call Style.updateLayer(id:type:update:).
| let preferences: [String] | ||
| if let identifier = locale?.identifier { | ||
| preferences = [identifier] | ||
| } else { | ||
| preferences = Locale.preferredLanguages | ||
| } |
There was a problem hiding this comment.
I'd just set default value for preferences instead of introducing else statement.
There was a problem hiding this comment.
It’s a matter of style, but in general, I prefer to minimize usage of local variables as much as possible to avoid mutability surprises further down in the code, even though it’s slightly more verbose.
| } | ||
|
|
||
| func localized(into locale: Locale?) -> Expression { | ||
| switch elements.first { |
There was a problem hiding this comment.
This usage is now broken as of MapboxMaps v2.0.0-beta.18: mapbox/mapbox-maps-ios#299.
|
Is this one still a |
|
This proof of concept has run its course. The map SDK once again comes with a built-in label localization feature that the navigation SDK can hook into: #3330. The map SDK is missing some functionality that was included in this PR, but there are separate issues to track that work. |
3 participants
Restored the
NavigationMapView.localizeLabels()implementation for automatically localizing map labels into the user’s language. The previous implementation was removed as part of upgrading to map SDK v10 in #2808, because it relied on an NSExpression-based localization feature in map SDK v6.x, which has been replaced by a new Expression type system. This PR ports map SDK v6.x’s NSExpression-based localization feature to Expression as part of MapboxNavigation, in anticipation of a more cross-platform solution in future versions of the map SDK. Note that this implementation continues to differ from map SDK v6.x’s default behavior, in that road labels remain unlocalized by design.More to come:
/cc @mapbox/navigation-ios @mapbox/maps-ios-reviewers