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.

Sign up for GitHub

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

Jump to bottom

Don't use ceremonial name when way is interstate #291

Closed
wants to merge 6 commits into from
Closed

Don't use ceremonial name when way is interstate #291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d8ebc10
Don't use ceremonial name when way is interstate
Jun 20, 2017
7a5da03
More ref
Jun 22, 2017
03f8a3c
check for keywords in name
Jul 18, 2017
ca65fe0
Use isFreeway
Jul 18, 2017
9c104c6
remove
Jul 18, 2017
c175151
Use plist
Jul 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions MapboxNavigation.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@
C55CEB281EB7E4240065D7D9 /* Locale.swift in Sources */ = {isa = PBXBuildFile; fileRef = C55CEB271EB7E4240065D7D9 /* Locale.swift */; };
C58159011EA6D02700FC6C3D /* MGLVectorSource.swift in Sources */ = {isa = PBXBuildFile; fileRef = C58159001EA6D02700FC6C3D /* MGLVectorSource.swift */; };
C58D6BAD1DDCF2AE00387F53 /* Constants.swift in Sources */ = {isa = PBXBuildFile; fileRef = C58D6BAC1DDCF2AE00387F53 /* Constants.swift */; };
C594260D1F1EBA2B00C8E59C /* Highways.plist in Resources */ = {isa = PBXBuildFile; fileRef = C594260C1F1EBA2B00C8E59C /* Highways.plist */; };
C5ADFBD81DDCC7840011824B /* MapboxCoreNavigationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C5ADFBD71DDCC7840011824B /* MapboxCoreNavigationTests.swift */; };
C5C94C1B1DDCD22B0097296A /* MapboxCoreNavigation.h in Headers */ = {isa = PBXBuildFile; fileRef = C5ADFBCC1DDCC7840011824B /* MapboxCoreNavigation.h */; settings = {ATTRIBUTES = (Public, ); }; };
C5C94C1C1DDCD2340097296A /* RouteController.swift in Sources */ = {isa = PBXBuildFile; fileRef = C5ADFBF91DDCC9580011824B /* RouteController.swift */; };
Expand Down Expand Up @@ -366,6 +367,7 @@
C55CEB271EB7E4240065D7D9 /* Locale.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Locale.swift; sourceTree = "<group>"; };
C58159001EA6D02700FC6C3D /* MGLVectorSource.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MGLVectorSource.swift; sourceTree = "<group>"; };
C58D6BAC1DDCF2AE00387F53 /* Constants.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Constants.swift; sourceTree = "<group>"; };
C594260C1F1EBA2B00C8E59C /* Highways.plist */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.plist.xml; path = Highways.plist; sourceTree = "<group>"; };
C5ADFBC91DDCC7840011824B /* MapboxCoreNavigation.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = MapboxCoreNavigation.framework; sourceTree = BUILT_PRODUCTS_DIR; };
C5ADFBCC1DDCC7840011824B /* MapboxCoreNavigation.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MapboxCoreNavigation.h; sourceTree = "<group>"; };
C5ADFBCD1DDCC7840011824B /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
Expand Down Expand Up @@ -561,6 +563,7 @@
children = (
351BEC281E5BD530006FE110 /* Assets.xcassets */,
351BEC2A1E5BD538006FE110 /* Shields.plist */,
C594260C1F1EBA2B00C8E59C /* Highways.plist */,
C520EE921EBB84F9008805BC /* Navigation.storyboard */,
DAAE5F321EAE4C4700832871 /* Localizable.strings */,
C5212B571EC4BE97009538EB /* Abbreviations.plist */,
Expand Down Expand Up @@ -1050,6 +1053,7 @@
files = (
351BEC2B1E5BD538006FE110 /* Shields.plist in Resources */,
DAAE5F301EAE4C4700832871 /* Localizable.strings in Resources */,
C594260D1F1EBA2B00C8E59C /* Highways.plist in Resources */,
351BEC291E5BD530006FE110 /* Assets.xcassets in Resources */,
C520EE901EBB84F9008805BC /* Navigation.storyboard in Resources */,
C5212B551EC4BE97009538EB /* Abbreviations.plist in Resources */,
Expand Down
7 changes: 7 additions & 0 deletions MapboxNavigation/Bundle.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,10 @@ var ShieldImageNamesByPrefix: [String: String] = {
}
return NSDictionary(contentsOfFile: plistPath) as! [String: String]
}()

var HighwayNamesByPrefix: [String: String] = {
guard let plistPath = Bundle.mapboxNavigation.path(forResource: "Highways", ofType: "plist") else {
return [:]
}
return NSDictionary(contentsOfFile: plistPath) as! [String: String]
}()
26 changes: 26 additions & 0 deletions MapboxNavigation/Highways.plist
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>us-interstate</key>
<string>I</string>
<key>us-interstate-business</key>
<string>I</string>
<key>us-interstate-duplex</key>
<string>I</string>
<key>us-interstate-truck</key>
<string>I</string>
<key>us-highway</key>
<string>US</string>
<key>us-highway-duplex</key>
<string>US</string>
<key>us-highway-alternate</key>
<string>US</string>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given:

shield: us-highway-alternate
ref: 50

the label will incorrectly say “US 50”, whereas it would be more accurate to say “US 50 Alt” or “Alt US 50”. Same for the other banners below.

<key>us-highway-business</key>
<string>US</string>
<key>us-highway-bypass</key>
<string>US</string>
<key>us-highway-truck</key>
<string>US</string>
</dict>
</plist>
10 changes: 7 additions & 3 deletions MapboxNavigation/RouteManeuverViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,10 +170,14 @@ class RouteManeuverViewController: UIViewController {
}

func updateStreetNameForStep() {
if let name = step?.names?.first {
destinationLabel.unabridgedText = name
} else if let destinations = step?.destinations {
if let destinations = step?.destinations {
destinationLabel.unabridgedText = destinations.prefix(min(numberOfDestinationLines, destinations.count)).joined(separator: "\n")
} else if let name = step?.names?.first {
if let ref = step.codes?.first, name.isFreeway {
destinationLabel.unabridgedText = ref
} else {
destinationLabel.unabridgedText = name
}
} else if let step = step {
destinationLabel.unabridgedText = routeStepFormatter.string(for: step)
}
Expand Down
26 changes: 21 additions & 5 deletions MapboxNavigation/RouteMapViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,18 +336,34 @@ extension RouteMapViewController: NavigationMapViewDelegate {
if minDistanceBetweenPoints < smallestLabelDistance {
smallestLabelDistance = minDistanceBetweenPoints

if let line = feature as? MGLPolylineFeature, let name = line.attribute(forKey: "name") as? String {
currentName = name
} else if let line = feature as? MGLMultiPolylineFeature, let name = line.attribute(forKey: "name") as? String {
currentName = name
if let line = feature as? MGLPolylineFeature {
if let name = line.attribute(forKey: "name") as? String, !name.isFreeway {
currentName = name
} else if let ref = line.attribute(forKey: "ref") as? String, let shield = line.attribute(forKey: "shield") as? String {
if let highwayName = HighwayNamesByPrefix[shield] {
currentName = "\(highwayName) \(ref)"
} else {
currentName = ref
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For any country outside the U.S., this logic would display only the number. Even inside the U.S., something like a county road would get just the number, which is opposite what I would expect. I think it would be preferable in all these cases to either show the name and number or to show nothing at all.

}
}
} else if let line = feature as? MGLMultiPolylineFeature {
if let name = line.attribute(forKey: "name") as? String, !name.isFreeway {
currentName = name
} else if let ref = line.attribute(forKey: "ref") as? String, let shield = line.attribute(forKey: "shield") as? String {
if let highwayName = HighwayNamesByPrefix[shield] {
currentName = "\(highwayName) \(ref)"
} else {
currentName = ref
}
}
} else {
currentName = nil
}
}
}
}

if smallestLabelDistance < 5 && currentName != nil {
if smallestLabelDistance < 10 && currentName != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh, nope.

wayNameLabel.text = currentName
wayNameView.isHidden = false
} else {
Expand Down
9 changes: 6 additions & 3 deletions MapboxNavigation/String.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,7 @@ extension String {
return NSRange(location: 0, length: characters.count)
}
}
}


extension String {
typealias Replacement = (of: String, with: String)

func byReplacing(_ replacements: [Replacement]) -> String {
Expand All @@ -28,4 +25,10 @@ extension String {
("'", "&apos;")
])
}

var isFreeway: Bool {
return self.components(separatedBy: " ").filter {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: drop self..

["freeway", "expressway", "highway", "fwy", "hwy"].contains($0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Mapbox Streets source abbreviates “expressway” as “Expy”.

}.isEmpty
}
}