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

Localization plugin should transform expressions #374

Closed
1ec5 opened this issue Mar 19, 2018 · 4 comments
Closed

Localization plugin should transform expressions #374

1ec5 opened this issue Mar 19, 2018 · 4 comments
Assignees
Labels
bug A bug is found inside the code base and should be immediately brought to attention ASAP localization-plugin

Comments

@1ec5
Copy link
Contributor

1ec5 commented Mar 19, 2018

The map localization plugin added in #74 only works if a text-field property is set to a constant value that uses the legacy token syntax, e.g. {name_en}. In a style such as Mapbox Streets, some layers’ text-fields are set to camera functions, so they remain unlocalized. To make matters worse, future versions of these and other styles will be written for versions of the SDK that support expressions. {name_en} will become ["get", "name_en"], so this plugin will end up having no effect whatsoever.

/ref mapbox/mapbox-gl-native#10713 mapbox/mapbox-gl-js#6197
/cc @langsmith @tobrun

@1ec5
Copy link
Contributor Author

1ec5 commented Apr 6, 2018

mapbox/mapbox-navigation-ios#1076 implements a fairly thorough expression transformation:

https://github.com/mapbox/mapbox-navigation-ios/blob/cced8ad5d37032f059def9ea8a19e47e299536bc/MapboxNavigation/NSExpression.swift
https://github.com/mapbox/mapbox-navigation-ios/blob/cced8ad5d37032f059def9ea8a19e47e299536bc/MapboxNavigation/MGLVectorTileSource.swift
https://github.com/mapbox/mapbox-navigation-ios/blob/cced8ad5d37032f059def9ea8a19e47e299536bc/MapboxNavigation/NavigationMapView.swift#L952-L1010

The iOS map SDK’s built-in label localization functionality (see mapbox/mapbox-gl-native#10713) is more directly analogous to what the Android localization plugin does, but it’s probably a bit easier to understand the Swift implementation than it’ll be to understand my Objective-C port of this code to the map SDK.

@cammace cammace mentioned this issue Apr 20, 2018
2 tasks
@cammace
Copy link
Contributor

cammace commented Apr 20, 2018

This issues blocked by a few problems downstream, mainly mapbox/mapbox-gl-native#11659 and mapbox/mapbox-gl-native#11749.

@1ec5
Copy link
Contributor Author

1ec5 commented Aug 22, 2018

#455 is a very early start at localizing expressions that has stalled out. Both mapbox/mapbox-gl-native#11659 and mapbox/mapbox-gl-native#11749 have been fixed, so this work is technically unblocked.

However, in the process of implementing mapbox/mapbox-navigation-android#1226, we discovered another missing piece of the map SDK. There’s no way to determine a layer’s source, only a layer’s source layer: mapbox/mapbox-gl-native#12691. So although it is possible to localize a style, the localization will be very aggressive, affecting all layers, not only the ones backed by the Streets source. For example, if the style contains a symbol layer whose text field is based on a GeoJSON source’s name field, this plugin would potentially localize it to a field that isn’t provided by the GeoJSON source.

Until mapbox/mapbox-gl-native#12691 is fixed, this plugin will have to take the same approach as mapbox/mapbox-gl-native#12387 of coalescing the localized field with name. That ends up being necessary for fully supporting Chinese anyways.

@1ec5
Copy link
Contributor Author

1ec5 commented Oct 31, 2018

Fixed in #654.

@1ec5 1ec5 closed this as completed Oct 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug is found inside the code base and should be immediately brought to attention ASAP localization-plugin
Projects
None yet
Development

No branches or pull requests

3 participants