PaulLeCam / react-leaflet

React components for Leaflet maps
https://react-leaflet.js.org
Other
5.18k stars 888 forks source link

LayerGroup does not guarantee the order of markers #205

Closed kachkaev closed 8 years ago

kachkaev commented 8 years ago

Not sure if this is something that needs to be fixed or just documented, but here is the thing:

I've got quite a few circles and polylines on top of the map, which represent different entities and can be toggled (i.e. shown or hidden). Their visibility is controlled by simply including or excluding corresponding components from the iterators. The markers that have been switched from hidden to visible end up being always on the top of other (previously rendered) markers regardless of the order they have in the render() method.

I was expecting that grouping the markers into <LayerGroup /> would allow me to gain at least some control over the ordering but that did not happen. It seems that the only way to preserve the sequence of markers while keeping the layers switchable is to apply extra CSS styles to the hidden markers rather than excluding them from render() completely.

Would be nice if this issue was addressed at least in the docs if I'm not mistaken with it completely.

PS. All boxes are checked :-)

kachkaev commented 8 years ago

The same seems to happen when I try wrapping my objects into <LayerControl /> and <Overlay />. More than that, className does not update at least for <Circle />, <Circlemarker /> and <Multipolyline /> when the new value for this prop is passed.

PaulLeCam commented 8 years ago

Hi, There is no specific logic in this library regarding the ordering of layers so please refer to Leaflet documentation to check its behaviour. If the behaviour in this lib is indeed no matching Leaflet's one please provide a jsfiddle or similar. Regarding className changes, it should be supported so again, please provide an example to reproduce the issue.

kachkaev commented 8 years ago

Hi Paul,

I understand that the library is a thin wrapper, but I think that there should be some logic in ordering of layers though. React is declarative and the result of rendering is stateless (i.e. always the same for the given set of React components returned by render()). This creates an expectation that the leaflet markers will always appear in the same order as they are listed inside <Map />. This is currently not the case.

Imagine I have three markers, two of which are currently shown (the very first one is hidden). Map's children are 2 and 3. I change some prop of the parent component and the map re-renders; now there are three children (1, 2, 3). What react-leaflet will do in this case is keeping two already visible components still without any changes and then adding the third one to the very end of the list. The resulting order will be 2, 3, 1 instead of 1, 2, 3 as it would be expected from React's virtual DOM. In case if the visibility prop for the first marker is true from the beginning, markers will appear as expected: 1, 2, 3.

I'll try to produce a MWE for className and style in a few days if there is some free time.

PaulLeCam commented 8 years ago

Indeed this library is a wrapper, and React does not do the rendering, it's just an abstraction over Leaflet. If you want to add specific behaviour, you can always create custom components to implement specific logic but they would need to do so calling Leaflet APIs anyways. In your case, you can probably also recreate the entire <LayerGroup /> when its contents change by changing its key, I suppose that would ensure on Leaflet's side the contained layers are in order.

kachkaev commented 8 years ago

I tried playing with <LayerGroup /> and even <LayersControl.Overlay /> as well as changing keys, but could not get the desired result. 'Randomising' all keys to always get new re-ordered markers made each render a bit slow. The final solution was in hiding the circles by in changing their radius to minimum and applying opacity=0. All markers do exist at all times regardless of which of them are visible and which are not. This is a pretty dirty hack, but that's the only thing that worked for me after a few hours. The solution introduces a rather pity side-effect though: when a user moves a mouse over a blank space on the map, they sometimes see unwanted tooltips on the side because onMouseenter triggers on 'hidden' elements.

The purpose of React is to make all other APIs declarative, stateless and predictive. I know that if I what to use some drawing component tomorrow to draw a circle and a rectangle, it will always be the case that the shapes appear in the same order as the components inside render(), no matter how they are actually managed under the hood (it can be canvas, webgl, svg or whatever else). Same for leaflet: if at any point in time the state of the virtual DOM does not match what a user sees with their eyes, this means that something is not working as expected.

PaulLeCam commented 8 years ago

All these limitations are described in the documentation, and this library's purpose is only to provide an abstraction on top of Leaflet using Leaflet APIs, not to change them in whatever way people think is best. If the components provided don't support your needs, you can always create custom ones and call Leaflet's APIs as needed.