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

Allow tokens in combination with data-driven values? #4484

Closed
nickidlugash opened this issue Mar 23, 2017 · 5 comments
Closed

Allow tokens in combination with data-driven values? #4484

nickidlugash opened this issue Mar 23, 2017 · 5 comments
Labels
cross-platform 📺 Requires coordination with Mapbox GL Native (style specification, rendering tests, etc.) needs discussion 💬

Comments

@nickidlugash
Copy link

From #4074 (review):

I am not sure we want to allow the combination of data-driven properties and interpolation syntax. It could further complicate the future of features like mapbox/mapbox-gl-style-spec#47 and mapbox/mapbox-gl-style-spec#104.

Just as a point of discussion I wanted to surface a few examples (based on our Mapbox styles) where tokens + DDS would be useful:

  • State labels text-field values

    • Different size labels switch from an abbreviated name ({abbr}) to the full name (name_en) at different zoom levels
  • Rail labels text-field values

    • Switches from "" to name_en at a certain zoom level. Would be useful if heavy rail stations could switch at an earlier zoom level than light rail and metro stations.

Basically, these are situations where one might want to switch from either empty values or different data fields in a hierarchical manner across zoom levels.

These are not critical examples, since they can be separated out into different layers as necessary. I think token fallbacks and arbitrary functions are both more critical, and I agree with @jfirebaugh that we don't want to further block the development of these features. But since tokens + DDS can be useful, perhaps it is worthwhile to get more clarification beyond that one PR comment as to the implications of enabling tokens + DDS for text-field and icon-image?

/cc @anandthakker

@nickidlugash nickidlugash added the cross-platform 📺 Requires coordination with Mapbox GL Native (style specification, rendering tests, etc.) label Mar 23, 2017
@1ec5
Copy link
Contributor

1ec5 commented Mar 26, 2017

/cc @mapbox/gl

@nickidlugash
Copy link
Author

@jfirebaugh @anandthakker if we were to implement tokens in combination with DDS, would it be possible to assign a token as the function's default value?

@jfirebaugh
Copy link
Contributor

Yes, the implementation applies token replacement after function evaluation, and function evaluation includes returning the default value when a value isn't otherwise available.

@jfirebaugh
Copy link
Contributor

Thinking about this further, we actually definitely want to avoid naively composing replaceTokens(evaluateFunction(fn, feature)). Doing so would subject the results of identity functions to token replacement, with unexpected results for a feature whose property value happens to contain { and } characters.

So in order to support tokens in combination with data-driven values, token replacement would need to be intertwined with function evaluation, so that replacement happens for strings literally contained in the function definition (including default values), but not feature property strings. And the function evaluator would need to know which property it's evaluating, so that the token replacement continues to happen only for icon-image and text-font.

These complications push my moral compass further toward the feeling that supporting this would be a mistake.

@jfirebaugh
Copy link
Contributor

Closing per above. (And expressions are the future anyway! 😎)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cross-platform 📺 Requires coordination with Mapbox GL Native (style specification, rendering tests, etc.) needs discussion 💬
Projects
None yet
Development

No branches or pull requests

4 participants