pigeon-maps icon indicating copy to clipboard operation
pigeon-maps copied to clipboard

Controlled map allows moving and zooming

Open tiberiuana opened this issue 6 years ago • 5 comments

Hi there,

I am trying to control the map center and zoom myself, so I can limit it to a given boundary.

<Map center={myCenter} zoom={myZoom} onBoundsChanged={()=>{}}></Map>

The map renders with the provided center and zoom level, but then allows free moving and zooming.

Is this a bug or am I misunderstanding the docs? What would be the proper way to (re)set the center and zoom after a bounds change attempt?

Thanks!

tiberiuana avatar Sep 19 '19 16:09 tiberiuana

Hi, set mouseEvents and touchEvents to false to disable moving. With your approach, it would still be possible to move and drag the map around, just it would usually snap back to myCenter and myZoom. Here it won't since probably there is nothing that triggers a re-render of your host component, so the map is left to do whatever it pleases.

mariusandra avatar Sep 20 '19 11:09 mariusandra

Thanks for the quick answer Marius!

What I want is to have normal mouse and touch events, but to limit the map to an area.

The way I would expect this to work (consistent with React inputs): if center or zoom are provided, then the component is controlled and does not persist any changes to those props. Instead, onBoundsChanged has the chance to change the external variables, and thus have the map redraw. If onBoundsChanged does nothing, then it shouldn't be possible to move or zoom the map (persistently).

Then something like:

<Map center={[12,34]} zoom={12} onBoundsChanged={()=>{}}></Map>

should behave just like you mentioned, with the map snapping back to those coordinates after each change.

Unless I'm missing something here, this is not what's happening. Because the props don't ever change, the map never rerenders and appears uncontrolled because it keeps its own state instead -- and that is a problem.

tiberiuana avatar Sep 20 '19 12:09 tiberiuana

Hi, in order for the map to remain fast, the onBoundsChanged event is fired only when the map finishes animating. Try it with the demo on https://pigeon-maps.js.org - when you drag the mouse and don't let go, no event is fired, as evidenced by the center coordinates below the map remaining fixed.

Having an onBoundsChanged that does nothing will not limit your map to an area, btw.

mariusandra avatar Sep 20 '19 14:09 mariusandra

Thanks, in practice this means that the component is in fact not controlled even when passing center and zoom. I think this is a good discussion for a separate issue (maybe with a PR too).

About my actual issue, can you think of another straightforward way to limit the map to an area?

tiberiuana avatar Sep 20 '19 15:09 tiberiuana

Hi, yes and no regarding the component being controlled or not. They way it works now is a design decision that favours practicality over strict adherence to any definition. Thus I can't fully confirm nor deny what you wrote.

In a controlled component, the contract you make with the component is that whatever is returned by an onChange handler should be immediately passed back to the component as a prop. This is also a requirement in "controlled" pigeon maps. In the same way whenever the prop is changed, the component should immediately update itself to reflect the change. This is also what pigeon maps does.

So according to this doctrine it's controlled.

However the definition becomes muddy when you take animation into account.

Open the demo on https://pigeon-maps.js.org and click the different cities in the bottom of the page. You'll see that the map smoothly animates between them.

In a pure controlled component this would be impossible unless you manually implement all what is necessary to use window.requestAnimationFrame to periodically change the location based on the elapsed time, making sure the animation is visually pleasing (speeds up and slows down) and not linear. That's way too much work for users of the library.

That's why it's implemented internally: you pass the destination location as a prop to pigeon maps and the map either animates to the new location (if it's less than 3 screens away) or just jumps to it (if further).

Returning back the new location in every animation frame puts way too much load on your state management library and can produce many unintended side-effects, especially different timing and precision related problems. I could send an onBoundsChanged at every animation step, but in practice all the different timing related issues quickly make the map spiral out of control (pun intended).

That's why it works like it does. Consider it a "controlled debounce" if you will.

For very similar reasons when you're in the middle of a drag, we don't sync the location to props.

--

As for limiting the map to an area, there is no way to do it right now. You could adjust the center after an onBoundsChanged to make the map move back into the area you want it to be in... but unless you want to limit the center point of the map, this requires some geometric calculations based on the zoom level, etc (to limit the top-left point of the map, for example). In short, it's not too easy and the result is probably not what you want anyway.

There was some work in progress to limit the map on the lowest zoom setting (when you see the entire world) such that there would never be any gray area and the tiles would always fill the screen... but I never had time to finish it. This same logic could be expanded to further limit the map to an even smaller area. However I have no idea when I would have time to work on that. Definitely not any time soon.

mariusandra avatar Sep 23 '19 12:09 mariusandra