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

Calling setStyle doesn't clear previously set root properties. #11939

Closed
tristen opened this issue May 26, 2022 · 9 comments · Fixed by #11942
Closed

Calling setStyle doesn't clear previously set root properties. #11939

tristen opened this issue May 26, 2022 · 9 comments · Fixed by #11942

Comments

@tristen
Copy link
Member

tristen commented May 26, 2022

In this example, I have two stylesheets with terrain properties that look like this:

// Version A
{
  source: 'mapbox://mapbox.mapbox-terrain-dem-v1',
  exaggeration: 500
}

// Version B
{
  source: 'mapbox://mapbox.mapbox-terrain-dem-v1'
}

Calling setStyle to replace version A with version B results in a situation where exaggeration is still set to 500 and not the default value of 1

set-style-bug

Demo to reproduce: https://jsfiddle.net/tristen/ucpynakb/
Version: Latest stable v2.8.2

@avpeery
Copy link
Contributor

avpeery commented May 26, 2022

Likely related to #11916

@avpeery
Copy link
Contributor

avpeery commented May 27, 2022

In addition, when setting "exaggeration" in style b, it overwrites the "exaggeration" value in style a. See codepen (click map to toggle styles): https://codepen.io/avpeery/pen/OJQzmrG

@tristen
Copy link
Member Author

tristen commented May 27, 2022

@avpeery yeah, I think this is the case for all root properties. If the next style doesn't contain a previously set property, it's not discarded. My expectation is that the default value would be used if the property is no longer present.

If exaggeration is set in both styles the behavior seems right? That properties value overwrites the other.

@avpeery
Copy link
Contributor

avpeery commented May 27, 2022

@tristen The codepen example has exaggeration set in both style a and style b. If you click the map in the code pen example from style a to style b to style a again, style a’s exaggeration is overwritten to be style b’s exaggeration. I feel like this should not be expected behavior?

I opened a PR that prevents overwriting stylesheet properties (and includes setting the default value for the style terrain exaggeration if not present).

@tristen
Copy link
Member Author

tristen commented May 27, 2022

If you click the map in the code pen example from style a to style b to style a again, style a’s exaggeration is overwritten to be style b’s exaggeration

Ah! I missed that in the example you shared. Not expected no 😛

@tristen
Copy link
Member Author

tristen commented Jun 2, 2022

@avpeery can we mark this as a release blocker to v2.9.0?

@avpeery
Copy link
Contributor

avpeery commented Jun 2, 2022

@tristen Discussed this with @karimnaaji and since this has been in a couple of the past releases we were planning on not releasing it with 2.9.0 and waiting for the next beta. Is this very high priority for you?

cc: @karimnaaji

@tristen
Copy link
Member Author

tristen commented Jun 3, 2022

I would consider this a high priority. With globe support in Studio adding broader editing capabilities to root properties than before (like authoring expressions and clearing values) it amplifies this older issue.

@karimnaaji
Copy link
Contributor

Thanks for the feedback @tristen , we'll wrap this one up and include it! The fix is almost ready, we just need to make sure to carefully profile it as it can have a performance impact.

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

Successfully merging a pull request may close this issue.

3 participants