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

Issue 3162 recline view #3200

Merged
merged 8 commits into from Aug 17, 2016
Merged

Conversation

jrods
Copy link
Contributor

@jrods jrods commented Aug 10, 2016

Fixes #3162

Proposed fixes:

This provides a way to swap the map tiling via values in your config file.
It's the same configuration as ckanext-spatial, except that the config properties are prefixed with ckanext.reclineview.mapview .

Example:

ckanext.reclineview.mapview.type = 
ckanext.reclineview.mapview.mapbox.map_id = <your id>
ckanext.reclineview.mapview.mapbox.access_token = <your token>
ckanext.reclineview.mapview.attribution = <copyright, link, etc. >

Features:

  • includes tests covering changes
  • includes updated documentation
  • includes user-visible changes
  • includes API changes
  • includes bugfix for possible backport

Please [X] all the boxes above that apply

@amercader amercader self-assigned this Aug 12, 2016
@amercader
Copy link
Member

amercader commented Aug 12, 2016

Hi @jrods, many thanks for this. This is essentially the last thing that was missing from the list I made on #3174 (comment).

I think we can improve a couple of things:

  1. I've merged changes in Recline (Change default tiles for map view, allow customization datopian/datahub#502) to allow customization of the tiles without having to extend recline.View.Map. We just need to pass the relevant options when initialising the view on recline_view.js:

    view = new recline.View.Map({
        model: dataset,
        mapTilesURL: '//{s}.tiles.mapbox.com/v4/mapbox.mapbox-streets-v7/{z}/{x}/{y}.png?access_token=pk.XXXX',
        mapTilesAttribution: '&copy; MapBox etc..',
        mapTilesSubdomains: 'ab'
    })

    Just move the logic here to recline_view.js and pass the relevant url, attribution and domains functions when creating the view (so no need for mapview_ext.js).

    This should be available in master as soon as Update Recline version #3184 is merged (it is now ready to go) Merged now

  2. It would be great if users could control the behaviour of all map widgets with a single set of config options. Right now the widgets on ckanext-spatial and ckanext-geoview use ckanext.spatial.common_map.* so I think it makes sense to use those as well for the recline map

  3. If you can mention these options somewhere on the docs so people is aware of them that would be awesome.

Khalegh-H3 and others added 7 commits August 12, 2016 11:06
… map configuration for both recline_view and recline_map_view
removed mapview_ext.js

changed the namespace for getting the config options to
'ckanext.spatial.common_map.'
Just a quick blurb about what options
A quick blurb about what can be included to the config file
@jrods
Copy link
Contributor Author

jrods commented Aug 15, 2016

@amercader I've committed the suggestions you made in your comment to the PR

Just added a private method that returns the options in a obj that's needed for the map view
@kfishwick
Copy link

Would it be possible to have this change backported to version 2.5.x? That is the release we are using for bcgov.

@amercader amercader merged commit d19835f into ckan:master Aug 17, 2016
@amercader
Copy link
Member

@jrods Great stuff, many thanks for the patch!

@kfishwick as this requires a whole upgrade of recline I'm afraid it will not make it to the 2.5.x line. We will replace the failing tiles directly with Stamen ones as in #3174.
If you don't want to patch your local fork of CKAN I suggest you follow your original approach and override the relevant recline view methods with ones in a script loaded from your own extension.

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.

DataStore Map and Explorer not displaying map tiles since 11 July 2016
5 participants