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

Optionally show or hide Bing 'placeholder' tiles #14832

Merged
merged 4 commits into from Jun 18, 2023

Conversation

Shane98c
Copy link
Contributor

@Shane98c Shane98c commented Jun 15, 2023

Fixes #14541

Placeholder tiles are shown by default for different styles as described in the issue. The behavior is controlled by the presence of the n=z url param. This PR adds an optional parameter allowing users to decide whether to show these tiles or, alternatively, overzoom the best available resolution. If the parameter is not set, the default behavior for the style is used.

This is a fairly big improvement from the method of using maxzoom to hide placeholder tiles mentioned in the example because Bing's zoom level coverage differs across the globe, so while a maxzoom of x works to hide the tiles in the US, a different one is required in, eg, Spain.

@github-actions
Copy link

📦 Preview the website for this branch here: https://deploy-preview-14832--ol-site.netlify.app/.

Copy link
Contributor

@jahow jahow left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution! That feature is definitely useful. I've made a couple of remarks regarding the implementation, because I think it can be simplified.

Do you think you'd be able to add a test covering this feature here?

describe('#tileUrlFunction()', function () {

Thanks again

src/ol/source/BingMaps.js Outdated Show resolved Hide resolved
src/ol/source/BingMaps.js Outdated Show resolved Hide resolved
@Shane98c
Copy link
Contributor Author

Thanks for the thoughtful review @jahow! All makes sense.

I've made the suggested changes and added a few basic tests. I'm new to using karma, so any pointers on that front would be very welcome.

@Shane98c Shane98c requested a review from jahow June 17, 2023 22:50
Copy link
Contributor

@jahow jahow left a comment

Choose a reason for hiding this comment

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

I think it's better now, thanks!

@jahow jahow merged commit 6b6a37a into openlayers:main Jun 18, 2023
8 checks passed
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.

Make Bing "placeholder" tiles optional
2 participants