Adding generic shield support to instruction banner. #1417
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| import Foundation | ||
| import UIKit | ||
|
|
||
| class GenericRouteShield: StylableView { | ||
|
There was a problem hiding this comment. Should this class be public, so that developers can customize the colors or roundedness? |
||
| static let labelFontSizeScaleFactor: CGFloat = 2.0/3.0 | ||
|
|
||
| @objc dynamic var foregroundColor: UIColor? { | ||
| didSet { | ||
| layer.borderColor = foregroundColor?.cgColor | ||
| routeLabel.textColor = foregroundColor | ||
| setNeedsDisplay() | ||
| } | ||
| } | ||
|
|
||
| lazy var routeLabel: UILabel = { | ||
| let label: UILabel = .forAutoLayout() | ||
| label.text = routeText | ||
| label.textColor = .black | ||
| label.font = UIFont.boldSystemFont(ofSize: pointSize * ExitView.labelFontSizeScaleFactor) | ||
|
|
||
| return label | ||
| }() | ||
|
|
||
| var routeText: String? { | ||
| didSet { | ||
| routeLabel.text = routeText | ||
| invalidateIntrinsicContentSize() | ||
| } | ||
| } | ||
| var pointSize: CGFloat { | ||
| didSet { | ||
| routeLabel.font = routeLabel.font.withSize(pointSize * ExitView.labelFontSizeScaleFactor) | ||
| rebuildConstraints() | ||
| } | ||
| } | ||
|
|
||
| convenience init(pointSize: CGFloat, text: String) { | ||
| self.init(frame: .zero) | ||
| self.pointSize = pointSize | ||
| self.routeText = text | ||
| commonInit() | ||
| } | ||
|
|
||
| override init(frame: CGRect) { | ||
| pointSize = 0.0 | ||
| super.init(frame: frame) | ||
| } | ||
|
|
||
| required init?(coder aDecoder: NSCoder) { | ||
| pointSize = 0.0 | ||
| super.init(coder: aDecoder) | ||
| commonInit() | ||
| } | ||
|
|
||
| func rebuildConstraints() { | ||
| NSLayoutConstraint.deactivate(self.constraints) | ||
| buildConstraints() | ||
| } | ||
|
|
||
| func commonInit() { | ||
| translatesAutoresizingMaskIntoConstraints = false | ||
| layer.masksToBounds = true | ||
|
|
||
| //build view hierarchy | ||
| addSubview(routeLabel) | ||
| buildConstraints() | ||
|
|
||
| setNeedsLayout() | ||
| invalidateIntrinsicContentSize() | ||
| layoutIfNeeded() | ||
| } | ||
|
|
||
| func buildConstraints() { | ||
| let height = heightAnchor.constraint(equalToConstant: pointSize * 1.2) | ||
|
|
||
| let labelCenterY = routeLabel.centerYAnchor.constraint(equalTo: centerYAnchor) | ||
|
|
||
| let labelLeading = routeLabel.leadingAnchor.constraint(equalTo: leadingAnchor, constant: 8) | ||
| let labelTrailingSpacing = routeLabel.trailingAnchor.constraint(equalTo: trailingAnchor, constant: -8) | ||
|
|
||
| let constraints = [height, labelCenterY, labelLeading, labelTrailingSpacing] | ||
|
|
||
| addConstraints(constraints) | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -83,37 +83,44 @@ class InstructionPresenter { | |
| strings.append(attributedStrings.reduce(initial, +)) | ||
| } | ||
|
|
||
| switch component.type { | ||
| //Throw away exit components. We know this is safe because we know that if there is an exit component, | ||
| // there is an exit code component, and the latter contains the information we care about. | ||
| case .exit: | ||
| continue | ||
|
|
||
| //If we have a exit, in the first two components, lets handle that. | ||
| case .exitCode where 0...1 ~= index: | ||
| guard let exitString = self.attributedString(forExitComponent: component, maneuverDirection: instruction.maneuverDirection, dataSource: dataSource) else { fallthrough } | ||
| build(component, [exitString]) | ||
|
|
||
| //If we have an icon component, lets turn it into a shield. | ||
| case .icon: | ||
| if let shieldString = attributedString(forShieldComponent: component, repository: imageRepository, dataSource: dataSource, onImageDownload: onImageDownload) { | ||
| build(component, [joinString, shieldString]) | ||
| } else if let genericShieldString = attributedString(forGenericShield: component, dataSource: dataSource) { | ||
| build(component, [joinString, genericShieldString]) | ||
| } else { | ||
| fallthrough | ||
|
There was a problem hiding this comment. This is a clever use of a switch statement, but do you think the fallthroughs inside conditionals make the code flow a little more difficult to keep track of? There was a problem hiding this comment. @1ec5 It's certainly a complex switch, but I'd make the case that it's a heck of a lot easier to follow than the mess of I like to think that the There was a problem hiding this comment. Would it make sense to factor this switch out into it's own function? This is function is getting boarder-line large. |
||
| } | ||
|
|
||
| //if it's a delimiter, skip it if it's between two shields. | ||
| case .delimiter: | ||
| let componentBefore = components.component(before: component) | ||
| let componentAfter = components.component(after: component) | ||
|
|
||
| if let shieldKey = componentBefore?.cacheKey(), | ||
| imageRepository.cachedImageForKey(shieldKey) != nil { | ||
| continue | ||
| } | ||
| if let shieldKey = componentAfter?.cacheKey(), | ||
| imageRepository.cachedImageForKey(shieldKey) != nil { | ||
| continue | ||
| } | ||
| fallthrough | ||
|
|
||
| //Otherwise, process as text component. | ||
| default: | ||
| guard let componentString = attributedString(forTextComponent: component, dataSource: dataSource) else { continue } | ||
| build(component, [joinString, componentString]) | ||
| } | ||
|
|
@@ -130,8 +137,14 @@ class InstructionPresenter { | |
| return exitString | ||
| } | ||
|
|
||
| func attributedString(forGenericShield component: VisualInstructionComponent, dataSource: DataSource) -> NSAttributedString? { | ||
| guard component.type == .icon, let text = component.text, let key = component.cacheKey() else { return nil } | ||
|
|
||
| return genericShield(text: text, cacheKey: key, dataSource: dataSource) | ||
| } | ||
|
|
||
| func attributedString(forShieldComponent shield: VisualInstructionComponent, repository:ImageRepository, dataSource: DataSource, onImageDownload: @escaping ImageDownloadCompletion) -> NSAttributedString? { | ||
| guard shield.imageURL != nil, let shieldKey = shield.cacheKey() else { return nil } | ||
|
|
||
| //If we have the shield already cached, use that. | ||
| if let cachedImage = repository.cachedImageForKey(shieldKey) { | ||
|
|
@@ -141,8 +154,8 @@ class InstructionPresenter { | |
| // Let's download the shield | ||
| shieldImageForComponent(shield, in: repository, height: dataSource.shieldHeight, completion: onImageDownload) | ||
|
|
||
| //Return nothing in the meantime, triggering downstream behavior (generic shield or text) | ||
| return nil | ||
| } | ||
|
|
||
| func attributedString(forTextComponent component: VisualInstructionComponent, dataSource: DataSource) -> NSAttributedString? { | ||
|
|
@@ -182,6 +195,24 @@ class InstructionPresenter { | |
| return NSAttributedString(attachment: attachment) | ||
| } | ||
|
|
||
| private func genericShield(text: String, cacheKey: String, dataSource: DataSource) -> NSAttributedString? { | ||
| let view = GenericRouteShield(pointSize: dataSource.font.pointSize, text: text) | ||
|
|
||
| let attachment = ExitAttachment() | ||
|
|
||
| if let image = imageRepository.cachedImageForKey(cacheKey) { | ||
| attachment.image = image | ||
| } else { | ||
| guard let image = takeSnapshot(on: view) else { return nil } | ||
| imageRepository.storeImage(image, forKey: cacheKey, toDisk: false) | ||
| attachment.image = image | ||
| } | ||
|
|
||
| attachment.font = dataSource.font | ||
|
|
||
| return NSAttributedString(attachment: attachment) | ||
| } | ||
|
|
||
| private func exitShield(side: ExitSide = .right, text: String, component: VisualInstructionComponent, dataSource: DataSource) -> NSAttributedString? { | ||
|
|
||
| let view = ExitView(pointSize: dataSource.font.pointSize, side: side, text: text) | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
How does this look for the night style?
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.
Pretty good, all we end up having to do is needing to change the
foregroundColor: LineThere 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.
Oh i missed that, 👍