protomaps-leaflet icon indicating copy to clipboard operation
protomaps-leaflet copied to clipboard

No default theme for leafletLayer

Open wginsberg opened this issue 1 year ago • 5 comments

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.

wginsberg avatar May 04 '24 02:05 wginsberg

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.

bdon avatar May 06 '24 05:05 bdon

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)

wginsberg avatar May 07 '24 16:05 wginsberg

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

bdon avatar May 08 '24 01:05 bdon

Fair enough! will close my PR for now

wginsberg avatar May 08 '24 15:05 wginsberg

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?

bdon avatar May 09 '24 03:05 bdon