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

Revise special case handling for fill-outline-color #6242

Merged
merged 1 commit into from
Feb 27, 2018
Merged

Conversation

jfirebaugh
Copy link
Contributor

The prior code in FillStyleLayer#recalculate handled the case where fill-outline-color was currently undefined, but not the case where it was transitioning from an undefined value to a defined value. In order to handle that case, we must modify the special casing in DataDrivenProperty<T>#interpolate so that it returns a PossiblyEvaluatedPropertyValue containing undefined, rather than a bare undefined value. This makes the return value consistent with the "default value" for fill-outline-color (the one contained in PossiblyEvaluated#_values's prototype), and then both cases can be handled with one conditional in FillStyleLayer#recalculate.

Fixes #6238.

Launch Checklist

  • briefly describe the changes in this PR
  • write tests for all new functionality
  • manually test the debug page

The prior code in FillStyleLayer#recalculate handled the case where fill-outline-color was _currently_ undefined, but not the case where it was transitioning from an undefined value to a defined value. In order to handle that case, we must modify the special casing in DataDrivenProperty<T>#interpolate so that it returns a PossiblyEvaluatedPropertyValue containing undefined, rather than a bare undefined value. This makes the return value consistent with the "default value" for fill-outline-color (the one contained in PossiblyEvaluated#_values's prototype), and then both cases can be handled with one conditional in FillStyleLayer#recalculate.
@jfirebaugh jfirebaugh requested a review from lbud February 27, 2018 20:20
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.

2 participants