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

Design proposal: Allow to add layers of type "slot" to facilitate toggling of layers #591

Open
sbachinin opened this issue Apr 3, 2024 · 8 comments
Labels
enhancement New feature or request

Comments

@sbachinin
Copy link
Contributor

Design Proposal: Allow to add layers of type 'slot'

Motivation

Motivated by a recent discussion in maplibre-gl-js and my own hard experience with toggling.

Proposed Change

User should be able to add layers of {type: slot} and no other properties except id.
My idea is that slots can be a convenient way to toggle layers, especially groups of layers.
Users will populate (or clear) the slots via a method called something like populateSlot.
This is what I'm already trying to implement for maplibre-gl-js, and it turns out fairly simple.
See rationale and comparison to mapbox slots in this PR

Migration Plan and Compatibility

This change is meant to introduce only new functionality, no breaking changes.

Rejected Alternatives

Slots can be designed in various ways. Mapbox implementation is quite different from the idea that I suggest here, read about it in the PR mentioned above.

In my implementation when you add "normal" layers to a slot, you do it in a batch, and these "normal" layers themselves have no knowledge of that.

In contrast, mapbox allows to specify a "slot" property on a "normal" layer object, pointing to a slot in which to put this layer.
So in mapbox approach each layer decides in which slot to go.

My idea is different. Based on my experience, it seems more likely that you have a coherent group of layers, such as "all about contours". And you populate a slot at once by calling populateSlot, providing an array of contour layers.
Then for example you can call populateSlot with an empty array, thus emptying the slot.
This approach in my opinion is simple and clean. It will give a user less opportunity of creating a mess in layers' order. (And order is the main problem that is ultimately addressed here).

@HarelM
Copy link
Member

HarelM commented Apr 3, 2024

Assuming I'm serializing the style to an object and then applying this object using seStyle to the map, how would this work? Would I still be able to toggle the same layers?

@sbachinin
Copy link
Contributor Author

sbachinin commented Apr 3, 2024

Well, it's one of the pending questions in my maplibre-gl-js implementation.
I think this style-spec change in itself doesn't force any particular solution here.
Though in theory some extra spec properties might be necessary to simplify things.
Currently in my code this won't work. (Serializing and applying this object will erase the slots I believe. Slotted layers will become free).

Let's decide on the expected behiviour, I would try it and tell if it's easily achievable with these style-spec rules.

1st of all, what should be in the serialized style?
A. Free layers + slots + slotted layers?
B. Free layers + slots?
C. Free layers + slotted layers?

Example solution:
Let's choose B.
This will be consistent with how layers are provided on initialization (only free layers and empty slots).
Then, when you setStyle(serializedStyle), slots are re-populated with the same slotted layers as before setStyle.

Does it look good?

@HarelM
Copy link
Member

HarelM commented Apr 3, 2024

One of the powerful things of the style spec and the style.json is that in most cases you can represent the current state in the style.json.
Creating slots as a layer with id only won't break that behavior, which is good, but the expected usage in the library will.
If we design an API that will keep the above idea in mind, I would say that we are going in the right direction.
I would consider the following:
Allow adding slots to facilitate for a better ergonomics when adding a layer, much like today where you can use beforeId.
So the above API doesn't change at all and its meaning doesn't change. We can consider adding an API called addLayers which received an array, but you can easily do this for outside the library.
The next API we can think of is toggleLayers(startId, endId) which will inverse the visibility of all the layers between these ids, it can use slots, but it also can use regular layers.

If we are keen on linking between slots and layers we should save it somehow in the exported json and therefore change the spec a bit more, otherwise it's only in memory and considered problematic from my perspective.

Also note, that adding these slots layres, without linking it to layers, allows external code to do the manipulation of bulk editing etc so the APIs above are nice-to-have I guess.

I also tend to think that adding an invisible circle layer can do the same as this slot concept and maybe its redundant.
In my app, I use these invisible layers to add and remove layers in between these virtual slots to toggle base map, overlay, routes etc.

That's my 2 cents I guess...

@sbachinin
Copy link
Contributor Author

Thanks for detailed response.
Do I understand it correctly that, from you pov, toggling the layers' visibility (instead of introducing the slots) is a technique that is good enough?
In such case the whole issue is meaningless.
It was based first of all on the assumption that such technique is evil. It was described as inconvenient in the linked discussion, and I personally agree with that. But I might be wrong.

@HarelM
Copy link
Member

HarelM commented Apr 7, 2024

I honestly don't know. It makes sense to add a "placeholder" layer that is not rendered, but the value of it is very limited.
Addind something more complex requires more work both from the spec point of view and obviously implementation.

@wipfli
Copy link
Member

wipfli commented Apr 8, 2024

Thanks for opening this issue @sbachinin. For clarification, can you maybe post an example style.json with the new slot property?

@sbachinin
Copy link
Contributor Author

@wipfli

It will be the same style.json but with layers of type "slot":

...,
layers: [
  {
      id: 'background',
      type: 'background',
      paint: {'background-color': 'rgb(216, 210, 206)'}
  },
  {
      id: 'buildings',
      type: 'slot'
  },
  ...
],
...

Here is an example page where such style.json is used.

@wipfli
Copy link
Member

wipfli commented Apr 16, 2024

Thanks for the example. If the main purpose of slots is to improve convenience when manipulating layers in MapLibre GL JS' API, then I would suggest that we create some new methods in MapLibre GL JS instead that work with layer prefixes.

For example for visibility, we could do this:

map.setLayoutPropertyByIdPrefix('buildings_', 'visibility', 'none')

which would act on all layers that have an id starting with buildings_ such as for example buildings_outline or buildings_fill.

@HarelM HarelM added the enhancement New feature or request label Apr 25, 2024
@HarelM HarelM changed the title Allow to add layers of type "slot" to facilitate toggling of layers Design proposal: Allow to add layers of type "slot" to facilitate toggling of layers Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants