protomaps / protomaps-leaflet

Lightweight vector map rendering + labeling and symbology for Leaflet
https://protomaps.com/docs/frontends/leaflet
BSD 3-Clause "New" or "Revised" License
767 stars 43 forks source link

No default theme for leafletLayer #153

Open wginsberg opened 7 months ago

wginsberg commented 7 months ago

The theme option for the leafletLayer function is marked as optional, however when I omit nothing gets shown on the map.

I.e.

// This works
protomapsL.leafletLayer({url:'FILE.pmtiles OR ENDPOINT/{z}/{x}/{y}.mvt',theme:"light"})

// This doesn't work
protomapsL.leafletLayer({url:'FILE.pmtiles OR ENDPOINT/{z}/{x}/{y}.mvt'})

I am keen to try to submit a PR to fix this one.

bdon commented 6 months ago

Should we throw an error if no theme is passed?

It could be more explicit if we had positional arguments. In the future, the lang argument will also be required to specify en, de, etc.

Proposal welcome.

wginsberg commented 6 months ago

Just opened a PR where it just sets the theme to "light" by default. I think it is a little nicer than throwing an error (and maybe how this worked before? I was using an older version of this package and encountered this problem when I upgraded)

bdon commented 6 months ago

Thanks for the PR. I recall the issue here now, which is that if we pass this:

leafletLayer({paintRules:myPaintRules})

If I'm visualizing my own tileset, as of right now on main this sets labelRules to [] which makes sense. If we set a default theme it will make the labelRules default to the light theme label rules, which is also confusing and implicit behavior.

So we probably need some breaking change in the API to encapsulate these use cases:

1) initialize the map with a default theme 2) initialize the map with one of the pre-baked 5 themes 3) initialize the map with a custom set of label rules / paint rules

wginsberg commented 6 months ago

Fair enough! will close my PR for now

bdon commented 6 months ago

What if we created a 2nd entry point called like leafletBasemapLayer("light", {"url":"...."}) to encapsulate this use case?

Also, would a Typedoc page like this: https://protomaps.github.io/PMTiles/typedoc/ be helpful?