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

AttributionControl misses updates #9824

Closed
blq opened this issue Jun 24, 2020 · 2 comments · Fixed by #9943
Closed

AttributionControl misses updates #9824

blq opened this issue Jun 24, 2020 · 2 comments · Fixed by #9943
Assignees
Labels

Comments

@blq
Copy link

blq commented Jun 24, 2020

Updating custom attributions is missed if the source layer is not initially visible.

This codepen https://codepen.io/blqute/pen/PoZjbpp illustrates it (based on https://docs.mapbox.com/mapbox-gl-js/example/wms/ )
Only changes:

  1. I added a custom attribution to the source ("nj.gov")
  2. added minzoom=10 to the layer to make it initially hidden (appear after couple of zoom ins)

=> the custom attribution is never shown.
Comment out "minzoom" and the attribution shows.

Checking the code I think the AttributionControl:onAdd/onRemove methods need to nudge _updateAttributions also on "moveend" event, otherwise an initially hidden attribution never get's a chance to appear.

Sizing: needs investigation

@mourner
Copy link
Member

mourner commented Aug 19, 2020

Thanks a lot for the report! I've attempted to fix this in #9943 — check it out.

@blq
Copy link
Author

blq commented Aug 26, 2020

Seems to work!
But.. now I want more ;) Ideally I think attributions should match what's actually visible on the screen, regardless of expressions. If you have several overlays or sources spread across the globe, you'd like the ones not visible to not show attribution.
Or at least layers that are bounded with the new "within" expression (that's my case now)
That's how Google maps does it for example, local map data providers' attributions only show for the local area.
Is that feasible to add you think?

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

Successfully merging a pull request may close this issue.

3 participants