Feature/localization #480

neelmistry94 · 2021-06-17T18:43:47Z

PRs must be submitted under the terms of our Contributor License Agreement CLA.

Fixes: Localization Feature

Pull request checklist:

Briefly describe the changes in this PR.
Include before/after visuals or gifs if this PR includes visual changes.
Write tests for all new functionality. If tests were not written, please explain why.
Add example if relevant.
Document any changes to public APIs.
Apply changelog label ('breaking change', 'bug 🪲', 'build', 'docs', 'feature 🍏', 'performance ⚡', 'testing 💯') or use the label 'skip changelog'
Add an entry inside this element for inclusion in the mapbox-maps-ios changelog: <changelog>Adds localization support for v10 Maps SDK. This can be used by setting the mapView.locale. There is a SupportedLanguagesenum that will allow you to create a supportedLocale</changelog>.
Update the migration guide, API Docs, Markdown files - Readme, Developing, etc

Summary of changes

This is the introduction for Localization Support in Maps SDK v10. We will take the existing expression found on a SymbolLayer.textField and convert it to a JSON String. Replace all instances of the expression ["get","name_xx"] with the new locale that can be set on the mapView. It will then be converted back to an expression type and the layer will be updated.

User impact (optional)

Sources/MapboxMaps/Foundation/MapView.swift

nishant-karajgikar · 2021-06-22T14:36:35Z

Sources/MapboxMaps/Foundation/MapView.swift

+    public var locale: Locale = Locale.current {
+        didSet {
+            mapboxMap.style.localizeLabels(into: locale)
+        }
+    }


Could this be an unsupported Locale?

It can, but in that case, we will default to the local case if there is no data that exists

We may want to make that behavior more explicit.

I could add a check when passed in to see if it is under one of our supported languages. If so, then continue, otherwise don't do anything

Sources/MapboxMaps/Style/Style+Localization.swift

nishant-karajgikar · 2021-06-22T14:59:12Z

Sources/MapboxMaps/Style/Style+Localization.swift

+
+    /// This function creates an expression that will localize the `textField` property of a `SymbolLayer`
+    /// - Parameter locale: A `SupportedLanguage` based `Locale`
+    internal func localizeLabels(into locale: Locale) {


Nav SDK has a requirement that not all labels are localized I think.. They want to exempt road labels for example from any localization.

To help them do this, it's probably best to break this function up into some functions like this:

public func localizeAllLabels(into locale: Locale) { .. } public func localizeLabel(withId id: String, into locale: Locale) { .. }

@julianrex @tobrun This feels like something we need to align on. This functionality does not exist on Android

Sources/MapboxMaps/Style/Style+Localization.swift

Sources/MapboxMaps/Style/SupportedLanguage.swift

julianrex · 2021-06-22T15:34:11Z

Noting that this PR does not support Locale.preferredLanguages (which < 10.0.0 did). This is something that we should address in a later PR @tobrun @sbma44

1ec5

Thank you for implementing this important feature. Here’s some belated feedback based on lessons learned in the past.

1ec5 · 2021-06-22T18:57:06Z

Sources/MapboxMaps/Style/Style+Localization.swift

+            let tempLayer = try layer(withId: layerInfo.id) as SymbolLayer
+
+            if var stringExpression = String(data: try JSONEncoder().encode(tempLayer.textField), encoding: .utf8),
+               stringExpression != "null" {
+                stringExpression.updateExpression(replacement: replacement, regex: expressionCoalesce)
+                stringExpression.updateExpression(replacement: replacement, regex: expressionAbbr)
+
+                // Turn the new json string back into an Expression
+                let data = stringExpression.data(using: .utf8)
+                let convertedExpression = try JSONSerialization.jsonObject(with: data!, options: [])
+
+                try setLayerProperty(for: tempLayer.id, property: "text-field", value: convertedExpression)
+            }


This code should be extracted into a public method, so that a map optimized for turn-by-turn navigation can localize place and POI labels but not road labels.

1ec5 · 2021-06-22T19:02:45Z

Sources/MapboxMaps/Style/Style+Localization.swift

+        let symbolLayers = allLayerIdentifiers.filter { layer in
+            return layer.type == .symbol
+        }


Not every symbol layer should be forced to have a name-based text-field, which is specific to the Mapbox Streets source. A symbol layer may be backed by a vector tileset other than Mapbox Streets or a GeoJSON source; that source’s features may have name_en properties but not name properties. There’s code elsewhere that checks the source URL; it would be useful to conditionalize all localization on it, not just detection of Streets source v7.

1ec5 · 2021-06-22T19:09:03Z

Sources/MapboxMaps/Style/Style+Localization.swift

+                if tempSource.url?.contains("mapbox.mapbox-streets-v7") == true {
+                    if locale.identifier.contains("zh") {
+                        // v7 styles does not support value of "name_zh-Hant"
+                        if locale.identifier == SupportedLanguage.traditionalChinese.rawValue {


Replace SupportedLanguage with an array of Locales to avoid unchecked usage of raw values.

1ec5 · 2021-06-22T19:13:35Z

Sources/MapboxMaps/Style/Style+Localization.swift

+        let expressionCoalesce = try NSRegularExpression(pattern: "\\[\"get\",\\s*\"(name_.{2,7})\"\\]",
+                                                         options: .caseInsensitive)
+        let expressionAbbr = try NSRegularExpression(pattern: "\\[\"get\",\\s*\"abbr\"\\]",
+                                                     options: .caseInsensitive)


It’s unnecessary to convert the entire expression to JSON, then to a string, then monkeypatch the string. That defeats the purpose of having a structured, type-safe representation of an expression.

Consider adapting the implementation in mapbox/mapbox-navigation-ios#2933 that recurses into the expression, surgically replacing occurrences of name or name_* with the coalescing expression. That approach is much more reliable and maintainable than trying to account for the variety of possible text-field values with finding-and-replacing in source code.

1ec5 · 2021-06-22T19:17:16Z

Sources/MapboxMaps/Foundation/MapView.swift

@@ -34,6 +35,17 @@ open class MapView: UIView {
    /// Controls the addition/removal of annotations to the map.
    public internal(set) var annotations: AnnotationOrchestrator!

+    /// Property that describes the current language for `SymbolLayer.textField`
+    public var locale: Locale = Locale.current {


Locale.current only matches the language that the iOS UI is currently displayed in, which can differ from both the surrounding application and the user’s preference. Bundle.preferredLocalizations has the advantage of matching the language that the rest of the application is currently displayed in. Locale.preferredLanguages has the advantage of matching the user’s preferred content language settings, even if the application is unavailable in the preferred language.

1ec5 · 2021-06-22T19:18:22Z

Sources/MapboxMaps/Style/Style+Localization.swift

+            do {
+                let tempSource = try source(withId: sourceInfo.id) as VectorSource
+
+                if tempSource.url?.contains("mapbox.mapbox-streets-v7") == true {


The tileset ID always occurs in the hostname part of the URL, so compare against URL.host for additional robustness.

neelmistry94 added the feature 🍏 When working on a new feature or feature enhancement label Jun 17, 2021

Neel Mistry added 6 commits June 21, 2021 15:01

Localize symbol layer text fields function

3d16037

localization example

140fa7b

code and project file clean up

ddc0ec9

documentation

3fbb7ef

move localize code to style

318817d

support for v7

8be4ca5

neelmistry94 force-pushed the feature/localization branch from f3cd789 to 8be4ca5 Compare June 21, 2021 19:01

Neel Mistry added 2 commits June 21, 2021 16:29

updated expression replacement

73ab57b

updating regex

6e2a40e

nishant-karajgikar reviewed Jun 22, 2021

View reviewed changes

Sources/MapboxMaps/Foundation/MapView.swift Outdated Show resolved Hide resolved

nishant-karajgikar reviewed Jun 22, 2021

View reviewed changes

code cleanup

d8873f3

neelmistry94 marked this pull request as ready for review June 22, 2021 14:50

nishant-karajgikar reviewed Jun 22, 2021

View reviewed changes

Sources/MapboxMaps/Style/Style+Localization.swift Outdated Show resolved Hide resolved

nishant-karajgikar reviewed Jun 22, 2021

View reviewed changes

julianrex approved these changes Jun 22, 2021

View reviewed changes

Sources/MapboxMaps/Style/Style+Localization.swift Outdated Show resolved Hide resolved

Sources/MapboxMaps/Style/SupportedLanguage.swift Show resolved Hide resolved

pr comments

87a6ac2

Neel Mistry added 5 commits June 22, 2021 13:34

swift lint and changelog

3573782

swift lint

6a8a7c1

[run device tests]

a04a530

adding missing file [run device tests]

3510b14

[run device tests] swift lint errors that were missed

ca1dfc8

neelmistry94 merged commit 13f10e6 into main Jun 22, 2021

1ec5 reviewed Jun 22, 2021

View reviewed changes

This was referenced Sep 7, 2021

Unable to localize labels into Chinese #652

Open

Unable to unlocalize a layer’s labels #653

Closed

macdrevx deleted the feature/localization branch October 21, 2021 15:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/localization #480

Feature/localization #480

neelmistry94 commented Jun 17, 2021 •

edited

Loading

nishant-karajgikar Jun 22, 2021

neelmistry94 Jun 22, 2021

nishant-karajgikar Jun 22, 2021

neelmistry94 Jun 22, 2021

nishant-karajgikar Jun 22, 2021

neelmistry94 Jun 22, 2021

julianrex commented Jun 22, 2021

1ec5 left a comment

1ec5 Jun 22, 2021

1ec5 Jun 22, 2021 •

edited

Loading

1ec5 Jun 22, 2021

1ec5 Jun 22, 2021

1ec5 Jun 22, 2021

1ec5 Jun 22, 2021

Feature/localization #480

Feature/localization #480

Conversation

neelmistry94 commented Jun 17, 2021 • edited Loading

Pull request checklist:

Summary of changes

User impact (optional)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

julianrex commented Jun 22, 2021

1ec5 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1ec5 Jun 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neelmistry94 commented Jun 17, 2021 •

edited

Loading

1ec5 Jun 22, 2021 •

edited

Loading