-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
More SymbolLayer properties + sourceLayerIdentifier support + zoom levels + local image support #42
Conversation
…tensions can be built, improving compatibility with source based features
@@ -161,3 +161,13 @@ public extension StyleLayer { | |||
modified(self) { $0.insertionPosition = .belowOthers } | |||
} | |||
} | |||
|
|||
public extension StyleLayerDefinition { |
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.
Please review if this how you picture its usage, as I see other zoom level code in this spm already, but didn't find a way to access it from the MapView view builder.
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.
@Archdoog on our review call: "I think he is on to something fantastic" 😂 We definitely missed exposing this idiomatically in the high-level StyleLayerDefinition
!
Capturing another random thought: it might make sense to split this file up since it's now got a bunch of random crap in it :) Especially for things like modifiers which might not be as obvious/discoverable. (Can do this later.)
fileprivate var iconImageName: NSExpression? | ||
public var iconImageName: NSExpression? | ||
|
||
private var iconImages = [UIImage]() | ||
public var iconImages = [UIImage]() |
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.
I made these two public, because this allows devs to make their own extensions for the SymbolStyleLayer. For example, we created a helper extension for ourselves like this, but needed these properties exposed to do so:
public extension SymbolStyleLayer {
func iconImage(mappings: [AnyHashable: UIImage], default defaultImage: UIImage) -> Self {
return modified(self) { it in
let expression1 = NSExpression(forKeyPath: "ios_category_icon_name")
let expression2 = NSExpression(forKeyPath: "ios_category_icon_color")
// Create an NSExpression that concatenates the two key paths
let attributeExpression = expression1.mgl_appending(expression2)
let mappingExpressions = mappings.mapValues { image in
NSExpression(forConstantValue: image.sha256())
}
let mappingDictionary = NSDictionary(dictionary: mappingExpressions)
let defaultExpression = NSExpression(forConstantValue: defaultImage.sha256())
// swiftlint:disable force_cast
it.iconImageName = NSExpression(
forMLNMatchingKey: attributeExpression,
in: mappingDictionary as! [NSExpression: NSExpression],
default: defaultExpression
)
// swiftlint:enable force_cast
it.iconImages = mappings.values + [defaultImage]
}
}
}
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.
That makes sense. Thanks for the detailed example clarifying the use case!
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.
This is awesome; thanks as always!
Final bit of feedback: could you implement the example that shows dynamic icon selection? There's a first pass which is currently commented out at the bottom of Examples/Layers.swift
.
@@ -161,3 +161,13 @@ public extension StyleLayer { | |||
modified(self) { $0.insertionPosition = .belowOthers } | |||
} | |||
} | |||
|
|||
public extension StyleLayerDefinition { |
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.
@Archdoog on our review call: "I think he is on to something fantastic" 😂 We definitely missed exposing this idiomatically in the high-level StyleLayerDefinition
!
Capturing another random thought: it might make sense to split this file up since it's now got a bunch of random crap in it :) Especially for things like modifiers which might not be as obvious/discoverable. (Can do this later.)
fileprivate var iconImageName: NSExpression? | ||
public var iconImageName: NSExpression? | ||
|
||
private var iconImages = [UIImage]() | ||
public var iconImages = [UIImage]() |
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.
That makes sense. Thanks for the detailed example clarifying the use case!
🤯 whoa! What on earth?? Previews are weird, yeah, but.... okay. That's interesting. That also probably explains why, after trying several different things a few weeks ago, I never got anything that worked for image matching! I was only ever testing in the previews. I'll take a look, but I believe that means this is probably mergeable as-is and we'll have to raise an issue upstream after collecting some more details on the crash. |
Sorry for conglomerate of different changes - these were missing features we needed for a testflight release.
Here is a screenshot from our app, using all these features (notice the text, halos, dynamic images created locally from sf symbols, etc - all this is loaded from a source in the style json):
Tests do not pass, I will update the PR once #41 is merged.