-
Notifications
You must be signed in to change notification settings - Fork 310
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
Map labels are no longer blank and using China Map Style URLs if the application is configured to use Mapbox China APIs #1558
Changes from 4 commits
34f90ac
cc9286a
87bfdea
d681921
d3664e5
2ee884c
f701580
3688c3b
d59a428
a064cc8
cfe9473
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
import Foundation | ||
|
||
class NetworkConfiguration : NSObject{ | ||
@objc static let sharedConfiguration = NetworkConfiguration() | ||
|
||
// The current API base URL of map | ||
private var apiBaseURL : String? | ||
|
||
// The PRC base URL for Mapbox APIs other than the telemetry API. | ||
private let mapboxChinaBaseAPIURL = "https://api.mapbox.cn" | ||
|
||
// The base URL host for Mapbox China | ||
public let mapboxChinaBaseURLHost = "api.mapbox.cn" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. MGLNetworkConfiguration is a private class in the map SDK. Is there a need to make these symbols public in the navigation SDK? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it can make public, that is really helpful for only change the map endpoint. But for a further consideration, I wanna to make this module easily change the map endpoint, api-directions endpoint and integrate Chinese TTS service. |
||
|
||
// The URL String of China map style. | ||
public let mapboxChinaStyleURL = "mapbox://styles/mapbox/streets-zh-v1" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This property is currently unused. Let’s leave it out until we need it. Normally it’s the responsibility of the map SDK to handle style URLs anyways. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This property is used for my next PR. I'll change the codes in `MGLStyle.swift' sth like:
It will be the default style of .cn map for China navigation. |
||
|
||
// Value of whether the map is China map or not | ||
public var hasChinaBaseURL : Bool{ | ||
apiBaseURL = Bundle.main.object(forInfoDictionaryKey:"MGLMapboxAPIBaseURL") as? String | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it necessary to store this base URL upfront? I think it would be more straightforward to execute this code on demand, as part of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, the value doesn’t change, but my suggestion is to call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be a local variable. |
||
return apiBaseURL == mapboxChinaBaseAPIURL | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per mapbox/mapbox-gl-native#12417, it seems like the
hasChinaAPIBaseURL
property should go on an extension of MGLAccountManager instead of this singleton.