cutting-room-floor / tilemill-reference-layer

TileMill Plugin that allows adding a tiled layer from tiles.mapbox.com
14 stars 7 forks source link

Add per project basemap setting #4

Closed ansis closed 12 years ago

ansis commented 12 years ago

Currently it defaults to MapBox Streets when left empty (which means you can't have no basemap). Should it be changed to have no default? Or should it have an initial default which can be removed by setting it blank?

willwhite commented 12 years ago

@dhcole will you review this when you have a chance? We're having a dicussion about whether it's better to have the default be MapBox Streets or no reference layer. On one hand, it would be nice to have the option to disable the layer on a project-by-project basis. on the other hand, people who enable this module may not be able to find the options easily.

springmeyer commented 12 years ago

+1 for being able to disable/enable per project.

willwhite commented 12 years ago

@springmeyer I think that's implemented, did you test?

springmeyer commented 12 years ago

ha, no, just skimmed the ticket too fast.

ansis commented 12 years ago

I don't think its possible to disable at the moment (defaults to mapbox streets), but I can add that soon.

ansis commented 12 years ago

Updated to work with TileMill 0.10.0. It now sets MapBox streets as the default for new maps, but allows the base layer to be removed by erasing the field.

Once this is merged, this repository should be renamed to tilemill-reference-layer

springmeyer commented 12 years ago

once problem I see in testing is that it appears clearing the reference layer does not stick and after saving the project the map remains. Upon viewing the project settings against the original url is still present. Changing the input to a space (just for testing) does effectively remove the map, but also of course leads to errors in the console:

[tilemill] Client Error: Uncaught SyntaxError: Unexpected token <
[tilemill]     at http://localhost:20009/?callback=grid:1
[tilemill] Client Error: Uncaught TypeError: Cannot read property 'tiles' of undefined
[tilemill]     at http://localhost:20009/assets/tilemill/js/vendor.js:4409
ansis commented 12 years ago

Should be fixed now. I had renamed a file incorrectly.

springmeyer commented 12 years ago

Also, as an aside, I installed by checking out the repo with the name tilemill-mapbox-streets. Since the branch is properly named tilemill-reference-layer when I try uninstalling it does not uninstall (understandable) and I see a console message of: uninstall not installed in /Users/dane/.tilemill/node_modules: "tilemill-reference-layer".

Any thoughts on whether we should try to handle this any differently? Seems like this is not a usecase (checking out from git) that needs to be supported, but figured I would mention it nevertheless.

ansis commented 12 years ago

This should be working now. Also, tracking in mapbox/tilemill#1523

springmeyer commented 12 years ago

great, working now locally, will merge. Excellent job @aibram.

springmeyer commented 12 years ago

now moving to rename the repo to tilemill-reference-layer.