Only evaluate CrossFaded properties at integer zooms #6430
Conversation
| @@ -18,5 +18,12 @@ export default function validateExpression(options: any) { | |||
| return [new ValidationError(options.key, options.value, 'Invalid data expression for "text-font". Output values must be contained as literals within the expression.')]; | |||
| } | |||
|
|
|||
| if (options.expressionContext === 'property' && expression.value.kind === 'camera' && options.propertyKey.match(/(-pattern)|(-dasharray)/)) { | |||
There was a problem hiding this comment.
Would it be slightly clearer to explicitly list all the properties this applies to instead of using a regular expression?
There was a problem hiding this comment.
It's another case where #6389 (specifically #6389 (comment)) would help.
|
I'm slightly worried about the consequences of changing this. We had at least one case where we made validation stricter in a similar area and later had to roll it back because existing styles stopped working (mapbox/mapbox-gl-style-spec#413). |
@mollymerp is the above correct? If so, and we want to avoid risking breaking existing styles, we could just do that instead. |
|
@jfirebaugh good point – definitely don't want to break existing styles. @anandthakker yes, we can take this approach and it will be less strict – |
src/style-spec/reference/v8.json
Outdated
| @@ -2965,7 +2965,7 @@ | |||
| "function": "piecewise-constant", | |||
| "zoom-function": true, | |||
| "transition": true, | |||
| "doc": "Name of image in sprite to use for drawing images on extruded fills. For seamless patterns, image width and height must be a factor of two (2, 4, 8, ..., 512).", | |||
| "doc": "Name of image in sprite to use for drawing images on extruded fills. For seamless patterns, image width and height must be a factor of two (2, 4, 8, ..., 512). Note that this property does not support zoom-dependent expressions with non-integer zoom inputs.", | |||
There was a problem hiding this comment.
Maybe reword to: "Note that zoom-dependent expressions will be evaluated only at integer zoom levels."?
There was a problem hiding this comment.
👍 yep that's more concise.
mollymerp
changed the title
mollymerp
force-pushed
the
validate-nonint-crossfade
branch
2 times, most recently
from
6e162e9 to
617bb68
Compare
mollymerp
force-pushed
the
validate-nonint-crossfade
branch
from
617bb68 to
09e7894
Compare
fixes #6390
question: should we have a style spec field that indicates if the property is cross-faded or not to avoid having to hard code the property name string check?
Launch Checklist