Skip to content

Popup licence#21

Open
JimBacon wants to merge 6 commits intorob-murray:masterfrom
JimBacon:popup-licence
Open

Popup licence#21
JimBacon wants to merge 6 commits intorob-murray:masterfrom
JimBacon:popup-licence

Conversation

@JimBacon
Copy link
Contributor

This pull request offers the following:

  • Adds a layer option, termsURL, which allows a user to specify a URL for the terms and conditions link in the attribution. This means they can link to their own copy of the terms if preferred.
  • Allows null to be passed as the apiUrl parameter when the user wants to omit it but supply options.
  • Adds a hidden div containing a copy of the OS End User Terms.
  • If termsURL is not specified, adds a click handler attached to the terms and conditions link which shows the hidden div. Standard map interaction is disabled while this is visible.
  • Default styling for the div is provided in a css file. It sets a z-index of 1200 to ensure it appears on top of all other map artefacts.

It's a heck of a lot of code to add for a simple thing. The advantage is that, by default, users of the code will conform with OS Terms and Conditions. Whether it is worth burdening the module with that responsibility is a good question!

When an OS Layer is added, creates a hidden div containing terms and
conditions. Adds an event handler to the terms and conditions link to
show the div when the link is clicked.
If options are needed but no apiUrl then allows null to be passed to
indicate this.
options were being ignored and were unable to override default layer
settings such as maxZoom.

Note: it would also be possible to merge wmsParams in to options so that they too can be overridden by user options. However, in this case, Leaflet would change width and height settings for retina screens and I'm unable to test if this would work so I left it alone.
Allows a layer option of {termsURL: 'http://some/preferred/url') for the
link to terms and conditions if the popup version is unsuitable.
@rob-murray
Copy link
Owner

Nice approach!

Works well and does the job but... I'm not sure it is worth burdening the library with this content, like you say, and then having to maintain the content. I feel the logo is less likely to change and I'd prefer a few extra bytes to save a network request to a URL that may change - ideally OS would provide a link or at least an anchor for the End User T&Cs and we could use that.

I'm going to put it on hold for this version if thats OK? I don't think it would be a problem to add this in version 1.0.1 for example, I will think about it some more and get version 1.0 out first.

I see the End User T&Cs are way down the content the other end of the link we have but I think for now that satisfies the point 4.6.1 in the developer agreement.

@JimBacon
Copy link
Contributor Author

Agreed. I have raised the issue on the forum, https://www.ordnancesurvey.co.uk/forums/discussion/1010372/link-to-end-user-terms but am deeply pessimistic that any action will be taken.

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