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

Fix map blink when active Mapnik + some overlay #5570

Closed
wants to merge 1 commit into from

Conversation

deevroman
Copy link
Contributor

@deevroman deevroman commented Jan 29, 2025

#5474 added an unpleasant blink when an overlay is enabled and Mapnik is base layer, and you try to close the sidebar:

blick.mp4

This is because var layers = layerParam || "M"; does not add M when overlay is enabled (layerParam is not empty). I added a check that if Mapnik is the current layer, then we add its code, so that.removeLayer() was not called for it.

Checking this.getMapbaslaver() && is important because when opening the tab it can return undefined.

@deevroman deevroman marked this pull request as ready for review January 29, 2025 00:22
@AntonKhorev
Copy link
Collaborator

When the layers parameter layers= is empty (or doesn't exist), it could mean one of two things:

  • keep currently enabled layers
  • reset to default layers (standard base + no overlays)

osm-website's code is undecided about this:

  • map.setState behaves as if it wants to keep the currents layers because it won't call updateLayers when state.layers are empty. state.layers are populated with the layers param elsewhere
  • OSM.formatHash behaves as if empty value means standard layer because it removes M. Removing M doesn't always produce empty layers param because of overlay codes, and that's why this PR was opened.

@AntonKhorev
Copy link
Collaborator

But M is not removed from everywhere. The so-called _osm_location cookie which also stores currently selected layers keeps M.

@deevroman deevroman closed this Jan 29, 2025
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