-
Notifications
You must be signed in to change notification settings - Fork 313
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
Integrated new directions API fields with the tertiary instruction and lane indications #1514
Changes from 51 commits
352f70b
c73c2a3
f7e88a1
61287aa
18be61f
e3d839d
3a3fc0d
966ec8a
4f450cb
409e2fa
37018f8
d8a0234
7892bff
fa9f08b
fa1b32c
bff3b2f
560c8ba
0bf61ed
6c8806e
f074bdd
2ebd13b
fb56051
6dc222c
67f0ab5
a1d71e4
b00f946
bdebc16
abdb380
8b13242
8f36366
a241dbb
5e3c16d
d555b9a
5dcc682
d755726
f08eb8e
63af724
1dd33da
76cb369
a2c45ab
6b1be17
291ef60
6af14a8
0e25212
ba701e6
b6fdc38
436889c
4ada71c
890f28f
feaf4d9
13f2d71
27f91a2
901e229
11902ca
6461783
a117366
00e211a
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 |
---|---|---|
|
@@ -11,8 +11,9 @@ | |
* Fixed an issue when selecting a step from the steps list, you could be brought to the wrong step. [#1524](https://github.com/mapbox/mapbox-navigation-ios/pull/1524/) | ||
* `StyleManager.locationFor(styleManager:)` now allows for an optional CLLocation to be returned. [#1523](https://github.com/mapbox/mapbox-navigation-ios/pull/1523) | ||
* NavigationViewController now uses the recommended way `.preferredStatusBarStyle` to set the style of the status bar. [#1535](https://github.com/mapbox/mapbox-navigation-ios/pull/1535) | ||
* Renamed `InstructionsBannerView.updateInstruction(_:)` to `InstructionsBannerView.update(for:)`. | ||
* Deleted `InstructionsBannerView.update(_:)`, which previously updated the banner instruction and the current distance. | ||
* This is replaced with `InstructionsBannerView.updateDistance(for:)` and `InstructionsBannerView.updateInstruction(_:)`. `InstructionsBannerView.updateDistance(for:)` should be called on every location update, while `InstructionsBannerView.updateInstruction(_:)` should be called when `NSNotification.Name. routeControllerDidPassVisualInstructionPoint` is emitted. | ||
* This is replaced with `InstructionsBannerView.updateDistance(for:)` and `InstructionsBannerView.update(for:)`. `InstructionsBannerView.updateDistance(for:)` should be called on every location update, while `InstructionsBannerView.update(for:)` should be called when `NSNotification.Name. routeControllerDidPassVisualInstructionPoint` is emitted. | ||
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. These three points can potentially be combined into one with less repetition. |
||
|
||
## v0.18.1 (June 19, 2018) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -141,4 +141,10 @@ class ExitView: StylableView { | |
let labelTrailing = trailingAnchor.constraint(equalTo: exitNumberLabel.trailingAnchor, constant: 8) | ||
return [imageLeading, imageLabelSpacing, labelTrailing] | ||
} | ||
|
||
static func criticalHash(side: ExitSide, dataSource: DataSource) -> 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. Why is this called “critical”? Is there something important about this method and similarly named methods that needs to be documented somewhere? 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. @JThramer could you chime in to elucidate the essence of this refactor for 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. Still unclear on this. 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. Yes -- the reason why it's called Considering that we're caching views by their rendered images, any property that affects how an |
||
let proxy = ExitView.appearance() | ||
let criticalProperties: [AnyHashable?] = [side, dataSource.font.pointSize, proxy.backgroundColor, proxy.foregroundColor, proxy.borderWidth, proxy.cornerRadius] | ||
return String(describing: criticalProperties.reduce(0, { $0 ^ ($1?.hashValue ?? 0)})) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,8 +8,9 @@ protocol InstructionPresenterDataSource: class { | |
var shieldHeight: CGFloat { get } | ||
} | ||
|
||
typealias DataSource = InstructionPresenterDataSource | ||
|
||
class InstructionPresenter { | ||
typealias DataSource = InstructionPresenterDataSource | ||
|
||
private let instruction: VisualInstruction | ||
private weak var dataSource: DataSource? | ||
|
@@ -66,7 +67,7 @@ class InstructionPresenter { | |
typealias AttributedInstructionComponents = (components: [VisualInstructionComponent], attributedStrings: [NSAttributedString]) | ||
|
||
func attributedPairs(for instruction: VisualInstruction, dataSource: DataSource, imageRepository: ImageRepository, onImageDownload: @escaping ImageDownloadCompletion) -> AttributedInstructionComponents { | ||
let components = instruction.textComponents | ||
let components = instruction.components.compactMap { $0 as? VisualInstructionComponent } | ||
var strings: [NSAttributedString] = [] | ||
var processedComponents: [VisualInstructionComponent] = [] | ||
|
||
|
@@ -134,7 +135,7 @@ class InstructionPresenter { | |
|
||
func attributedString(forGenericShield component: VisualInstructionComponent, dataSource: DataSource) -> NSAttributedString? { | ||
guard component.type == .image, let text = component.text else { return nil } | ||
return genericShield(text: text, cacheKey: component.genericCacheKey, dataSource: dataSource) | ||
return genericShield(text: text, component: component, dataSource: dataSource) | ||
} | ||
|
||
func attributedString(forShieldComponent shield: VisualInstructionComponent, repository:ImageRepository, dataSource: DataSource, onImageDownload: @escaping ImageDownloadCompletion) -> NSAttributedString? { | ||
|
@@ -166,7 +167,10 @@ class InstructionPresenter { | |
} | ||
|
||
private func instructionHasDownloadedAllShields() -> Bool { | ||
for component in instruction.textComponents { | ||
let textComponents = instruction.components.compactMap { $0 as? VisualInstructionComponent } | ||
guard !textComponents.isEmpty else { return false } | ||
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 written more succinctly as: guard let _ = instruction.components.contains(where: { $0 is VisualInstructionComponent }) else { return false } 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. |
||
|
||
for component in textComponents { | ||
guard let key = component.cacheKey else { | ||
continue | ||
} | ||
|
@@ -189,11 +193,10 @@ class InstructionPresenter { | |
return NSAttributedString(attachment: attachment) | ||
} | ||
|
||
private func genericShield(text: String, cacheKey: String, dataSource: DataSource) -> NSAttributedString? { | ||
let proxy = GenericRouteShield.appearance() | ||
let criticalProperties: [AnyHashable?] = [dataSource.font.pointSize, proxy.backgroundColor, proxy.foregroundColor, proxy.borderWidth, proxy.cornerRadius] | ||
let additionalKey = String(describing: criticalProperties.reduce(0, { $0 ^ ($1?.hashValue ?? 0)})) | ||
private func genericShield(text: String, component: VisualInstructionComponent, dataSource: DataSource) -> NSAttributedString? { | ||
guard let cacheKey = component.cacheKey else { return nil } | ||
|
||
let additionalKey = GenericRouteShield.criticalHash(dataSource: dataSource) | ||
let attachment = GenericShieldAttachment() | ||
|
||
let key = [cacheKey, additionalKey].joined(separator: "-") | ||
|
@@ -212,19 +215,18 @@ class InstructionPresenter { | |
} | ||
|
||
private func exitShield(side: ExitSide = .right, text: String, component: VisualInstructionComponent, dataSource: DataSource) -> NSAttributedString? { | ||
guard let cacheKey = component.cacheKey else { return nil } | ||
|
||
let proxy = ExitView.appearance() | ||
let criticalProperties: [AnyHashable?] = [side, dataSource.font.pointSize, proxy.backgroundColor, proxy.foregroundColor, proxy.borderWidth, proxy.cornerRadius] | ||
let additionalKey = String(describing: criticalProperties.reduce(0, { $0 ^ ($1?.hashValue ?? 0)})) | ||
let additionalKey = ExitView.criticalHash(side: side, dataSource: dataSource) | ||
let attachment = ExitAttachment() | ||
guard let cacheKey = component.cacheKey else { return nil } | ||
|
||
if let image = imageRepository.cachedImageForKey(cacheKey) { | ||
let key = [cacheKey, additionalKey].joined(separator: "-") | ||
if let image = imageRepository.cachedImageForKey(key) { | ||
attachment.image = image | ||
} else { | ||
let view = ExitView(pointSize: dataSource.font.pointSize, side: side, text: text) | ||
guard let image = takeSnapshot(on: view) else { return nil } | ||
imageRepository.storeImage(image, forKey: [cacheKey, additionalKey].joined(separator: "-"), toDisk: false) | ||
imageRepository.storeImage(image, forKey: key, toDisk: false) | ||
attachment.image = image | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -93,7 +93,7 @@ open class BaseInstructionsBannerView: UIControl { | |
/** | ||
Updates the instructions banner info with a given `VisualInstructionBanner`. | ||
*/ | ||
public func updateInstruction(_ instruction: VisualInstructionBanner?) { | ||
@objc public func update(for instruction: VisualInstructionBanner?) { | ||
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. The Objective-C name of this method is |
||
let secondaryInstruction = instruction?.secondaryInstruction | ||
primaryLabel.numberOfLines = secondaryInstruction == nil ? 2 : 1 | ||
|
||
|
@@ -104,15 +104,16 @@ open class BaseInstructionsBannerView: UIControl { | |
} | ||
|
||
primaryLabel.instruction = instruction?.primaryInstruction | ||
maneuverView.visualInstruction = instruction?.primaryInstruction | ||
maneuverView.drivingSide = instruction?.drivingSide ?? .right | ||
secondaryLabel.instruction = secondaryInstruction | ||
maneuverView.visualInstruction = instruction | ||
} | ||
|
||
override open func prepareForInterfaceBuilder() { | ||
super.prepareForInterfaceBuilder() | ||
maneuverView.isStart = true | ||
let component = VisualInstructionComponent(type: .text, text: "Primary text label", imageURL: nil, abbreviation: nil, abbreviationPriority: NSNotFound) | ||
let instruction = VisualInstruction(text: nil, maneuverType: .none, maneuverDirection: .none, textComponents: [component]) | ||
let instruction = VisualInstruction(text: nil, maneuverType: .none, maneuverDirection: .none, components: [component]) | ||
primaryLabel.instruction = instruction | ||
|
||
distance = 100 | ||
|
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.
Link to this PR from any new changeset entries, so that developers can get the full story from the PR description (which should be expanded to explain the API changes too).