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

attribution from options overwrites Mapzen & OSM attribution #386

Open
hanbyul-here opened this issue May 7, 2017 · 4 comments
Open

attribution from options overwrites Mapzen & OSM attribution #386

hanbyul-here opened this issue May 7, 2017 · 4 comments

Comments

@hanbyul-here
Copy link
Contributor

hanbyul-here commented May 7, 2017

  • When user passes the attribution inside of options object, mapzen.js overwrites the attribution, getting rid of Mapzen attribution. I wonder it would be better to keep the Mapzen attribution and just add the attribution (as a prefix) that user passes. Any idea? @rfriberg ? @louh ?
@hanbyul-here hanbyul-here changed the title attribution from options overwrites the attribution attribution from options overwrites Mapzen & OSM attribution May 8, 2017
@rfriberg
Copy link
Contributor

rfriberg commented May 8, 2017

Good question. We intentionally left the attribution overwrite-able, but there is an argument to be made that we have a responsibility to OSM to make sure they are attributed properly, as well as a responsibility to our users to make it as easy as possible to follow our own attribution requirements.

So, I'm OK with making our attribution harder to remove.

We should also take this opportunity to review how we're handling attribution within individual components. @hanbyul-here - it looks like you have an open PR to remove the specific geocoder attribution -- thank you; I think that's the right move (at least until such time that we allow a geocoder w/o map).

We are also allowing specific attribution via tangramOptions, which gets appended to the end of the attribution. This is an artifact from allowing all Tangram and Leaflet options to pass through tangramOptions.

Example:

      var map = L.Mapzen.map('map', {
        tangramOptions: {
          attribution: 'attribution via tangramOptions'  // appended after Leaflet attribution
        },
        attribution: 'attribution via options'  // Overwrites our existing Mapzen attribution
      });

screen shot 2017-05-08 at 8 53 23 am

I'm inclined to leave that one for now, because it's not overwriting anything (and is unlikely to even be used due to lack of documentation/examples).

Moving forward, I propose we use one of the following formats for attribution that is passed in via the options object:

© Mapzen, OpenStreetMap, and others | Leaflet | User-provided attribution

or

User-provided attribution | © Mapzen, OpenStreetMap, and others | Leaflet

(Assume those have the appropriate links ☝️) Any preferences?

@rmglennon
Copy link
Contributor

Technically, Leaflet isn't required: https://groups.google.com/forum/#!topic/leaflet-js/fA6M7fbchOs

@rfriberg
Copy link
Contributor

rfriberg commented May 8, 2017

Oh, that's a good reminder. Because mapzen.js is built on Leaflet and passes Leaflet options, you can also turn off the attributionControl entirely:

      var map = L.Mapzen.map('map', {,
        attributionControl: false
      });

@hanbyul-here
Copy link
Contributor Author

hanbyul-here commented May 9, 2017

I think User-provided attribution | © Mapzen, OpenStreetMap, and others | Leaflet is the format what I expect to see when I pass the attribution. I left this comment on the closed pr too, but I think it is good not to encourage users to use each of component's attribution control? since the order of each attribution can be mixed and confusing?

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

No branches or pull requests

3 participants