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

expose text font names as a modifier #47

Merged
merged 1 commit into from
Aug 19, 2024

Conversation

NaifAlrashed
Copy link
Contributor

Consumers of SwiftUI dsl are now able to customize the font family for the text attribute for SymbolLayer. It provides a solution to this problem. According to the docs, the font family can be set to nil to reset to default value, so the exposed type is [String]?

Copy link
Collaborator

@ianthetechie ianthetechie left a comment

Choose a reason for hiding this comment

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

Looks good; thanks!

@ianthetechie ianthetechie merged commit b84df18 into maplibre:main Aug 19, 2024
2 checks passed
@hactar
Copy link
Collaborator

hactar commented Aug 19, 2024

Looking at the other properties we have so far and their documentation, all expressions are optional, but so far we have been adding the underlying variable as non optional, this appears to be the first one we are adding as optional. Not saying this is wrong, but trying to understand in which case this needs to be optional and in which cases it doesn't. Whats the criteria here?

@ianthetechie
Copy link
Collaborator

That's a good question... I didn't notice that; thanks for catching it!

Can anyone think of a (non-contrived) use case for building a symbol style layer (reminder: this is currently always from scratch; not loading from JSON or something) and needing to set a property to null? I can't (which is why they are non-optional elsewhere).

@NaifAlrashed
Copy link
Contributor Author

I cannot think of a use case to pass nil. In fact, our internal solution was [String]. I just did it to expose the whole functionality because I thought this is library code. Would you like me to make it [String]?

@ianthetechie
Copy link
Collaborator

I'll wait to hear if @hactar or @Archdoog have any arguments otherwise, but yeah I'm leaning toward making it [String].

@hactar
Copy link
Collaborator

hactar commented Aug 22, 2024

I can't think of a useful use case either...

@ianthetechie
Copy link
Collaborator

Ok; I'll fix this up ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants