Skip to content

GridLayer: fix _updateLevels and _removeTilesAtZoom #7123

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

Merged
merged 1 commit into from
May 4, 2020

Conversation

johnd0e
Copy link
Collaborator

@johnd0e johnd0e commented May 2, 2020

Because of particular quality of for ... in loop, type of z is 'string'.
Thus condition z === zoom never met in _updateLevels.
Considering wrong arg type, _removeTilesAtZoom also never had any action.

Cleaner solution would be to iterate Object.keys() instead, but it is available only since IE 9.

Note: the fix here is pretty same as #6064.
Additionally I've checked all other similar places in Leaflet code and made one more potential fix for _onRemoveLevel (is not used in Leaflet's core, but available for overriding).

Because of particular quality of `for ... in` loop, type of `z` is 'string'.
Thus condition `z === zoom` never met in _updateLevels.
Considering wrong arg type, _removeTilesAtZoom also never had any action.

Cleaner solution would be to iterate `Object.keys()` instead, but it is available only since IE 9.
@johnd0e johnd0e force-pushed the fix_removeTilesAtZoom branch from 3d2e90f to 233a718 Compare May 3, 2020 08:49
@johnd0e johnd0e changed the title GridLayer: fix _removeTilesAtZoom GridLayer: fix _updateLevels and _removeTilesAtZoom May 3, 2020
Copy link
Member

@mourner mourner left a comment

Choose a reason for hiding this comment

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

Nice catch! I prefer +z over Number(z) for brevity, but no big deal.

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.

None yet

2 participants