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.

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

[Example] Data-Driven, Anchored Styling #548

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 5 additions & 1 deletion Apps/Examples/Examples.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
CADCF73C258499200065C51B /* AppDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 077C4EED252F7E88007636F1 /* AppDelegate.swift */; };
CADCF743258499570065C51B /* BasicMapExample.swift in Sources */ = {isa = PBXBuildFile; fileRef = CADCF742258499570065C51B /* BasicMapExample.swift */; };
CAF9A9812583E49B007EF9EC /* TestableExampleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = CAF9A9802583E49B007EF9EC /* TestableExampleTests.swift */; };
F46FF17B26025A40007CC0E0 /* CustomAnchoredSymbolExample.swift in Sources */ = {isa = PBXBuildFile; fileRef = F46FF17A26025A3F007CC0E0 /* CustomAnchoredSymbolExample.swift */; };
/* End PBXBuildFile section */

/* Begin PBXContainerItemProxy section */
Expand Down Expand Up @@ -180,6 +181,7 @@
CAC195B625AC098A00F69FEA /* CameraAnimatorsExample.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CameraAnimatorsExample.swift; sourceTree = "<group>"; };
CADCF742258499570065C51B /* BasicMapExample.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BasicMapExample.swift; sourceTree = "<group>"; };
CAF9A9802583E49B007EF9EC /* TestableExampleTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestableExampleTests.swift; sourceTree = "<group>"; };
F46FF17A26025A3F007CC0E0 /* CustomAnchoredSymbolExample.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CustomAnchoredSymbolExample.swift; sourceTree = "<group>"; };
/* End PBXFileReference section */

/* Begin PBXFrameworksBuildPhase section */
Expand Down Expand Up @@ -243,11 +245,12 @@
CADCF742258499570065C51B /* BasicMapExample.swift */,
CA86E81725BE7C2200E5A1D9 /* BuildingExtrusionsExample.swift */,
C64ED3842540A2BE00ADADFB /* CameraAnimationExample.swift */,
CAC195B625AC098A00F69FEA /* CameraAnimatorsExample.swift */,
0CE3D3BE25818626000585A2 /* ColorExpressionExample.swift */,
0CC4ECE925B8AD3000F998B8 /* Custom2DPuckExample.swift */,
0C52BA9725AF8C880054ECA8 /* Custom3DPuckExample.swift */,
073475D625AFAE520049B0B8 /* CustomLayerExample.swift */,
CAC195B625AC098A00F69FEA /* CameraAnimatorsExample.swift */,
F46FF17A26025A3F007CC0E0 /* CustomAnchoredSymbolExample.swift */,
C64ED3C42540DD6E00ADADFB /* CustomPointAnnotationExample.swift */,
A4AC5DEB2542CB2200995E4C /* CustomStyleURLExample.swift */,
C69F01EC2543646A001AB49B /* DataDrivenSymbolsExample.swift */,
Expand Down Expand Up @@ -564,6 +567,7 @@
CADCF7222584990E0065C51B /* ColorExpressionExample.swift in Sources */,
17E28C5C2672A1160033DF0F /* SymbolClusteringExample.swift in Sources */,
CADCF733258499130065C51B /* Examples.swift in Sources */,
F46FF17B26025A40007CC0E0 /* CustomAnchoredSymbolExample.swift in Sources */,
CADCF743258499570065C51B /* BasicMapExample.swift in Sources */,
CADCF7232584990E0065C51B /* UpdatePointAnnotationPositionExample.swift in Sources */,
CADCF72C2584990E0065C51B /* PointAnnotationExample.swift in Sources */,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,321 @@
import UIKit
import MapboxMaps
import Turf

private enum SymbolTailPosition: Int {
case left
case right
case center
}

private struct DebugFeature {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we choose a different name here? What's Debug about this feature?

Copy link

Choose a reason for hiding this comment

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

Another name is fun. It was originally named debug since it was for debugging the visualization of the annotations and "Feature" was a reserved word. Maybe "AnchoredFeature" or similar is better now.

Copy link
Contributor

Choose a reason for hiding this comment

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

See #548 (comment). I think avoiding the term feature in this name would be advantageous.

var coordinate: CLLocationCoordinate2D
var highlighted: Bool
var sortOrder: Int
var tailPosition: SymbolTailPosition
var label: String
var imageName: String
}

@objc(CustomAnchoredSymbolExample)
public class CustomAnchoredSymbolExample: UIViewController, ExampleProtocol {
Copy link
Contributor

Choose a reason for hiding this comment

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

App code never needs to be public since it won't be consumed by other modules. Keeping it internal unlocks additional compile-time optimizations that are not possible for public constructs.

static let symbols = "custom_symbols"
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we improve the naming of this constant? Is it an identifier?

Copy link
Contributor

Choose a reason for hiding this comment

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

It also seems unnecessary for this to be static. Letting it be an instance property would clean things up at the point of use.


internal var mapView: MapView!

// Configure a label
public lazy var label: UILabel = {
let label = UILabel(frame: CGRect.zero)
label.translatesAutoresizingMaskIntoConstraints = false
label.backgroundColor = UIColor.systemBlue
label.layer.cornerRadius = 12.0
label.textColor = UIColor.white
label.textAlignment = .center
label.font = UIFont.boldSystemFont(ofSize: 24.0)
return label
}()

override public func viewDidLoad() {
super.viewDidLoad()

// Center the map camera over New York City
let centerCoordinate = CLLocationCoordinate2D(latitude: 40.7128, longitude: -74.0060)
let options = MapInitOptions(cameraOptions: CameraOptions(center: centerCoordinate,
zoom: 15.0))

mapView = MapView(frame: view.bounds, mapInitOptions: options)
mapView.autoresizingMask = [.flexibleWidth, .flexibleHeight]
view.addSubview(mapView)

let tapGestureRecognizer = UITapGestureRecognizer(target: self, action: #selector((mapSymbolTap(sender:))))
mapView.addGestureRecognizer(tapGestureRecognizer)

// Allows the delegate to receive information about map events.
Copy link
Contributor

Choose a reason for hiding this comment

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

Use of the word delegate in this comment is confusing given that this doesn't really fit the typical delegation pattern on iOS

mapView.mapboxMap.onNext(.mapLoaded) { [weak self] _ in
guard let self = self else { return }

self.updateSymbolImages()
let features = self.addFeatures()
self.addSymbolLayer(features: features)

// The below line is used for internal testing purposes only.
self.finish()
}

// Add the label on top of the map view controller.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Add the label on top of the map view controller.
// Add the label on top of the map view.

addLabel()
}

public func addLabel() {
label.text = "Select A Marker"
view.addSubview(label)

NSLayoutConstraint.activate([
label.leadingAnchor.constraint(equalTo: view.leadingAnchor),
label.trailingAnchor.constraint(equalTo: view.trailingAnchor),
label.topAnchor.constraint(equalTo: view.safeAreaLayoutGuide.topAnchor),
label.heightAnchor.constraint(equalToConstant: 60.0)
])
}

@objc private func mapSymbolTap(sender: UITapGestureRecognizer) {
if sender.state == .recognized {
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider converting to a guard

let layers: [String] = [CustomAnchoredSymbolExample.symbols]

mapView.mapboxMap.queryRenderedFeatures(at: sender.location(in: mapView),
options: RenderedQueryOptions(layerIds: layers, filter: nil)) { result in
switch result {
case .success(let features):
if features.count > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if features.count > 0 {
if !features.isEmpty {

See docs on isEmpty for the recommended best practice

guard let featureText = features[0].feature.properties["text"] as? String else { return }
Comment on lines +89 to +90
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if features.count > 0 {
guard let featureText = features[0].feature.properties["text"] as? String else { return }
if let featureText = features.first?.feature.properties["text"] as? String {

self.label.text = featureText
}
case .failure(let error):
print("An error occurred: \(error.localizedDescription)")
}
}
}
}

private func updateSymbolImages() {
let style = mapView.mapboxMap.style

let regularTintColor = UIColor.white
let highlightTintColor = UIColor(hue: 0.831372549, saturation: 0.72, brightness: 0.59, alpha: 1.0)

// Centered pin
if let image = UIImage(named: "ImageCalloutCentered") {
let stretchX = [ImageStretches(first: Float(10), second: Float(15)), ImageStretches(first: Float(45), second: Float(50))]
let stretchY = [ImageStretches(first: Float(13), second: Float(16))]
let imageContent = ImageContent(left: 10, top: 13, right: 50, bottom: 16)

try? style.addImage(image.tint(regularTintColor),
id: "imageCentered",
sdf: false,
stretchX: stretchX,
stretchY: stretchY,
content: imageContent)

try? style.addImage(image.tint(highlightTintColor),
id: "imageCentered-Highlighted",
sdf: false,
stretchX: stretchX,
stretchY: stretchY,
content: imageContent)
}

let stretchX = [ImageStretches(first: Float(16), second: Float(21))]
let stretchY = [ImageStretches(first: Float(13), second: Float(16))]
let imageContent = ImageContent(left: 16, top: 13, right: 23, bottom: 16)

// Right-hand pin
if let image = UIImage(named: "ImageCalloutRightHanded") {
try? style.addImage(image.tint(regularTintColor),
id: "imageRightHanded",
sdf: false,
stretchX: stretchX,
stretchY: stretchY,
content: imageContent)

try? style.addImage(image.tint(highlightTintColor),
id: "imageRightHanded-Highlighted",
sdf: false,
stretchX: stretchX,
stretchY: stretchY,
content: imageContent)
}

// Left-hand pin
if let image = UIImage(named: "ImageCalloutLeftHanded") {
try? style.addImage(image.tint(regularTintColor),
id: "imageLeftHanded",
sdf: false,
stretchX: stretchX,
stretchY: stretchY,
content: imageContent)

try? style.addImage(image.tint(highlightTintColor),
id: "imageLeftHanded-Highlighted",
sdf: false,
stretchX: stretchX,
stretchY: stretchY,
content: imageContent)
}
}

private func addFeatures() -> FeatureCollection {
Copy link
Contributor

Choose a reason for hiding this comment

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

The naming of this method implies side effects, but there aren't any. How about makeFeatures instead, following the Swift API naming conventions?

Screen Shot 2021-07-20 at 11 36 01 AM

https://swift.org/documentation/api-design-guidelines/


var features = [Turf.Feature]()

let featureList = [
DebugFeature(coordinate: CLLocationCoordinate2DMake(40.714203, -74.006314), highlighted: false, sortOrder: 0, tailPosition: .left, label: "Chambers & Broadway - Lefthand Stem", imageName: "imageLeftHanded"),
DebugFeature(coordinate: CLLocationCoordinate2DMake(40.707918, -74.006008), highlighted: false, sortOrder: 0, tailPosition: .right, label: "Cliff & John - Righthand Stem", imageName: "imageRightHanded"),
DebugFeature(coordinate: CLLocationCoordinate2DMake(40.716281, -74.004526), highlighted: true, sortOrder: 1, tailPosition: .right, label: "Broadway & Worth - Right Highlighted", imageName: "imageRightHanded-Highlighted"),
DebugFeature(coordinate: CLLocationCoordinate2DMake(40.710194, -74.004248), highlighted: true, sortOrder: 1, tailPosition: .left, label: "Spruce & Gold - Left Highlighted", imageName: "imageLeftHanded-Highlighted"),
DebugFeature(coordinate: CLLocationCoordinate2DMake(40.7128, -74.0060), highlighted: true, sortOrder: 2, tailPosition: .center, label: "City Hall - Centered Highlighted", imageName: "imageCentered-Highlighted"),
DebugFeature(coordinate: CLLocationCoordinate2DMake(40.711427, -74.008614), highlighted: false, sortOrder: 3, tailPosition: .center, label: "Broadway & Vesey - Centered Stem", imageName: "imageCentered")
]
Comment on lines +168 to +177
Copy link
Contributor

Choose a reason for hiding this comment

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

Calling the lower array featureList is confusing given that now we have Turf.Feature and DebugFeature. Maybe we could rename DebugFeature in terms of just some generic data entity and avoid using the term feature in its name


for (index, feature) in featureList.enumerated() {
let point = Turf.Point(feature.coordinate)
Comment on lines +179 to +180
Copy link
Contributor

Choose a reason for hiding this comment

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

With https://github.com/mapbox/mapbox-maps-ios/pull/548/files#r673294649, in mind, maybe this would become something like…

Suggested change
for (index, feature) in featureList.enumerated() {
let point = Turf.Point(feature.coordinate)
for (index, dataItem) in dataItems.enumerated() {
let point = Turf.Point(dataItem.coordinate)

var pointFeature = Feature(geometry: .point(point))

// Set the feature attributes which will be used in styling the symbol style layer.
pointFeature.properties = ["highlighted": feature.highlighted,
"tailPosition": feature.tailPosition.rawValue,
"text": feature.label,
"imageName": feature.imageName,
"sortOrder": feature.highlighted == true ? index : -index]

features.append(pointFeature)
Comment on lines +181 to +190
Copy link
Contributor

Choose a reason for hiding this comment

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

And then down here things get a lot more clear because we don't have to parse feature vs pointFeature

}

return FeatureCollection(features: features)
}

private func addSymbolLayer(features: FeatureCollection) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: features implies [Feature] which is only 1 aspect of a FeatureCollection.

Suggested change
private func addSymbolLayer(features: FeatureCollection) {
private func addSymbolLayer(with featureCollection: FeatureCollection) {

if mapView.mapboxMap.style.sourceExists(withId: CustomAnchoredSymbolExample.symbols) {
try? mapView.mapboxMap.style.updateGeoJSONSource(withId: CustomAnchoredSymbolExample.symbols, geoJSON: features)
} else {
var dataSource = GeoJSONSource()
dataSource.data = .featureCollection(features)
try? mapView.mapboxMap.style.addSource(dataSource, id: CustomAnchoredSymbolExample.symbols)
}

var shapeLayer: SymbolLayer

if mapView.mapboxMap.style.layerExists(withId: CustomAnchoredSymbolExample.symbols) {
shapeLayer = try! mapView.mapboxMap.style.layer(withId: CustomAnchoredSymbolExample.symbols) as SymbolLayer
} else {
shapeLayer = SymbolLayer(id: CustomAnchoredSymbolExample.symbols)
}

shapeLayer.source = CustomAnchoredSymbolExample.symbols

shapeLayer.textField = .expression(Exp(.get) {
"text"
})

shapeLayer.iconImage = .expression(Exp(.get) {
"imageName"
})

shapeLayer.textColor = .expression(Exp(.switchCase) {
Exp(.any) {
Exp(.get) {
"highlighted"
}
}
UIColor.white
UIColor.black
})

shapeLayer.textSize = .constant(16)
shapeLayer.iconTextFit = .constant(.both)
shapeLayer.iconAllowOverlap = .constant(true)
shapeLayer.textAllowOverlap = .constant(true)
shapeLayer.textJustify = .constant(.left)
shapeLayer.symbolZOrder = .constant(.auto)
shapeLayer.symbolSortKey = .expression(Exp(.get) { "sortOrder" })
shapeLayer.textFont = .constant(["DIN Pro Medium"])

let expression = Exp(.switchCase) {
Exp(.eq) {
Exp(.get) { "tailPosition" }
0
}
"bottom-left"
Exp(.eq) {
Exp(.get) { "tailPosition" }
1
}
"bottom-right"
Exp(.eq) {
Exp(.get) { "tailPosition" }
2
}
"bottom"
""
}
shapeLayer.textAnchor = .expression(expression)

let offsetExpression = Exp(.switchCase) {
Exp(.eq) {
Exp(.get) { "tailPosition" }
0
}
Exp(.literal) {
[0.7, -2.0]
}
Exp(.eq) {
Exp(.get) { "tailPosition" }
1
}
Exp(.literal) {
[-0.7, -2.0]
}
Exp(.eq) {
Exp(.get) { "tailPosition" }
2
}
Exp(.literal) {
[-0.2, -2.0]
}
Exp(.literal) {
[0.0, -2.0]
}
}
shapeLayer.textOffset = .expression(offsetExpression)

try? mapView.mapboxMap.style.addLayer(shapeLayer)
}
}

extension UIImage {
// Produce a copy of the image with tint color applied.
// Useful for deployment to iOS versions prior to 13 where tinting support was added to UIImage natively.
Comment on lines +295 to +296
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we do some availability checks inside this method and use the built-in method when available? Then when we drop support for iOS 12 and lower someday, Xcode should lead us here with a warning and we can remove this extension entirely

func tint(_ tintColor: UIColor) -> UIImage {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
func tint(_ tintColor: UIColor) -> UIImage {
func tinted(with tintColor: UIColor) -> UIImage {
  • tinted vs tint: The latter implies mutation. The former implies the creation of a new object.
  • _ vs with: without the argument label, at the point of use, it reads as if you're tinting the color rather than the image.

let imageSize = size
let imageScale = scale
let contextBounds = CGRect(origin: .zero, size: imageSize)

UIGraphicsBeginImageContextWithOptions(imageSize, false, imageScale)

defer { UIGraphicsEndImageContext() }

UIColor.black.setFill()
UIRectFill(contextBounds)
draw(at: .zero)

guard let imageOverBlack = UIGraphicsGetImageFromCurrentImageContext() else { return self }
tintColor.setFill()
UIRectFill(contextBounds)

imageOverBlack.draw(at: .zero, blendMode: .multiply, alpha: 1)
draw(at: .zero, blendMode: .destinationIn, alpha: 1)

guard let finalImage = UIGraphicsGetImageFromCurrentImageContext() else { return self }

return finalImage
}
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"images" : [
{
"filename" : "AnnotationCentered.png",
"idiom" : "universal"
}
],
"info" : {
"author" : "xcode",
"version" : 1
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"images" : [
{
"filename" : "RouteInfoAnnotationLeftHanded.png",
"idiom" : "universal"
}
],
"info" : {
"author" : "xcode",
"version" : 1
}
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"images" : [
{
"filename" : "RouteInfoAnnotationRightHanded.png",
"idiom" : "universal"
}
],
"info" : {
"author" : "xcode",
"version" : 1
}
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading