bcgov / ckanext-bcgov

BC Data Catalogue source code, main ckan extension
http://catalogue.data.gov.bc.ca
GNU Affero General Public License v3.0
24 stars 23 forks source link

update recline.js to use ESRI leaflet that consumes the BC tilecache #116

Closed Mbrownshoes closed 8 years ago

Mbrownshoes commented 8 years ago

MapQuest changed its terms of use for the default leaflet tilecache. fork the reclineview extension and replace the leaflet code in recline.js with the ESRI leaflet code that consumes our own tilecache.

Khalegh-H3 commented 8 years ago

Here is a link to the newly created issue about MapQuest on ckan-spatial git and some suggested solutions : https://github.com/ckan/ckanext-spatial/issues/157

kfishwick commented 8 years ago

The longer term approach would be to do something similar to what the geoview and spatial extensions do, and parameterize the tile selection. However, given the time constraints, the recommendation is to fork the CKAN version 2.3.4, make the change, and modify the deployment to use the forked version instead of direct from core.

@Darv72 and @ll911 please create the fork and we can proceed with the updates.

kfishwick commented 8 years ago

Note, it appears that this approach will probably have to continue in 2.5, since the same issue occurs there.

kfishwick commented 8 years ago

Note, this seems to also be logged in CKAN but they want the recline js provider to update it.

https://github.com/ckan/ckan/issues/3162

Darv72 commented 8 years ago

I've forked the ckan repo into bcgov, the 2.3.4 branch can be found here:

https://github.com/bcgov/ckan/tree/ckan-2.3.4

gjlawran commented 8 years ago

We are sticking with MapQuest for our 1.4 release - our 1.5 release will allow the inclusion of an attribution statement and URL in our ini. For our own tile cache the following will be the attribution hyperlink: Copyright © 2016, Province of British Columbia

ll911 commented 8 years ago

You mean MapBox?

From: Greg Lawrance notifications@github.com Reply-To: bcgov/ckanext-bcgov reply@reply.github.com Date: Tuesday, July 19, 2016 at 3:05 PM To: bcgov/ckanext-bcgov ckanext-bcgov@noreply.github.com Cc: "Lou, Leo GCPE:EX" Leo.Lou@gov.bc.ca, Assign assign@noreply.github.com Subject: Re: [bcgov/ckanext-bcgov] update recline.js to use ESRI leaflet that consumes the BC tilecache (#116)

We are sticking with MapQuest for our 1.4 release - our 1.5 release will allow the inclusion of an attribution statement and URL in our ini. For our own tile cache the following will be the attribution hyperlink: Copyright © 2016, Province of British Columbiahttps://catalogue.data.gov.bc.ca/dataset/a5f40ea5-2b14-4f10-9904-94465e085c7e

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHubhttps://github.com/bcgov/ckanext-bcgov/issues/116#issuecomment-233780974, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ALZU2cwnOhK55PGRDqskpkaen1vhVaHrks5qXUorgaJpZM4JK48G.

gjlawran commented 8 years ago

No. - I believe the default is MapQuest hosted tiles built from OSM ....

https://github.com/ckan/ckan/blob/ae2eaa39c2921e53daa439833b3ed267be2a47ad/ckanext/reclineview /theme/public/vendor/recline/recline.js#L2279

kfishwick commented 8 years ago

The commit we just merged into your ckan fork includes the ability to customize the map parameters in your ini.

ckanext.reclineview.mapview.type = ckanext.reclineview.mapview.mapbox.map_id = ckanext.reclineview.mapview.mapbox.access_token = ckanext.reclineview.mapview.attribution =

gjlawran commented 8 years ago

Ah yes - @ll911 is correct - we are replacing MapQuest hosted tiles with MapBox hosted tiles. The attribution string specified by MapBox who also uses OSM is specified here https://www.mapbox.com/help/attribution/

ckanext.reclineview.mapview.attribution = "© <a href='https://www.mapbox.com/map-feedback/'>Mapbox</a> © <a href='http://www.openstreetmap.org/copyright'>OpenStreetMap</a>"

kfishwick commented 8 years ago

Ok @ll911 will have to add that in the test & prod ini files. Thanks.

gjlawran commented 8 years ago

@Khalegh-H3 are you working on including the attribution specified earlier - and ensuring that the target of the attribution hyperlink opens in a new tab ? It is currently attempting to open within the iframe.

kfishwick commented 8 years ago

This is just a configuration parameter Greg, Leo can set it to what you want in test. Including opening in a new tab (html goes in the ini file).

Khalegh-H3 commented 8 years ago

I haven’t done anything in the code for showing attribution target. The fix only includes replacing map tiles through ini configs(including the attribution html) if there are any. Otherwise it uses whatever tiles defined in recline.js.

As Karen said the attribution text and links can be totally customized using html code in ini file.

Mbrownshoes commented 8 years ago

When we deploy to test, the ini file is automatically copied over from delivery, thus we need the correct attribution (including fixing the hyperlinks) done in delivery first.

gjlawran commented 8 years ago

@kfishwick @Khalegh-H3 In the Delivery ini file - please replace:

ckanext.reclineview.mapview.attribution=© <a target=_blank href='https://www.mapbox.com/map-feedback/'>Mapbox</a> © <a href='http://www.openstreetmap.org/copyright'>OpenStreetMap</a> with

ckanext.reclineview.mapview.attribution=© <a target=_blank href='https://www.mapbox.com/map-feedback/'>Mapbox</a> © <a target=_blank href='http://www.openstreetmap.org/copyright'>OpenStreetMap</a>

then @Darv72 will roll to Test.

Mbrownshoes commented 8 years ago

Woking in test, though the Leaflet attribution still opens within the same page, not in a new tab, as it should.

kfishwick commented 8 years ago

The leaflet link is not specified in the attribute parameter, I'll have to talk to Khalegh about whether it can be customized.

Thanks, Karen

On Jul 21, 2016, at 4:43 PM, Mathew notifications@github.com wrote:

Woking in test, though the Leaflet attribution still opens within the same page, not in a new tab, as it should.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

gjlawran commented 8 years ago

Verified in TEST. Will resolve issue with in frame Leaflet attribution in 1.4 or 1.5.

gjlawran commented 8 years ago

Confirmed in production. Will raise a new issue to resolve problem with Leaflet attribution occurring within iframe.

ll911 commented 8 years ago

Can you close the milestone?

Sent from Mobile

On Fri, Jul 22, 2016 at 10:34 AM -0700, "Greg Lawrance" notifications@github.com<mailto:notifications@github.com> wrote:

Confirmed in production. Will raise a new issue to resolve problem with Leaflet attribution occurring within iframe.

You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/bcgov/ckanext-bcgov/issues/116#issuecomment-234606260, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ALZU2eOP-lJhxk9qf3jX8zBAIb7ItHQeks5qYP7ggaJpZM4JK48G.