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

Allow configuration of zoom padding #95

Open
paul121 opened this issue Jan 8, 2021 · 3 comments
Open

Allow configuration of zoom padding #95

paul121 opened this issue Jan 8, 2021 · 3 comments

Comments

@paul121
Copy link
Member

paul121 commented Jan 8, 2021

Feature request! I'm working on some custom pages for farmOS that render the map much larger than the normal 400px height (likely full browser height X ~70% width). When zooming to vectors at this scale the standard zoom padding of 20px feels too small, especially when zooming to a single polygon which can occupy the majority of the browser window.

The zoom functionality & default padding is defined here: https://github.com/farmOS/farmOS-map/blob/master/src/instance/methods/zoom.js#L20 (Also curious if zoomToVectors could call zoomToLayer to consolidate this logic?)

The padding could be added as an argument to the zoomToVectors method, but then only the behaviors that are aware of the map's context (in this case being a "large" map) will provide the desired padding. So ideally this default padding could be saved & used for all zoomToVectors calls on the map. Since the instance zoom methods are not a map behavior, we can't use behavior settings either.

So.. I think this is a good use case for an "instance specific setting" as I described in #94. The padding could be retrieved via a method on the instance eg: this.getSetting('zoomPadding'), or use the default. Any behavior that triggers a zoom on the map would be using it's desired padding.

A step further.. it also seems useful to allow different padding values depending on the number of features. Basically my desired goal is to center a SINGLE feature in the map & zoom out a bit more. Adding more features gets tricky.. I suppose my desired padding really depends on the distance between the features (assuming these features are single polygons, too)... or maybe I'm really seeking a padding based on the "ratio of feature area to canvas area" (not sure if this is possible?)

@paul121
Copy link
Member Author

paul121 commented Jan 8, 2021

This is probably relevant: #80 (comment)

@mstenta
Copy link
Member

mstenta commented Jan 12, 2021

This makes some sense to me. I'm a bit hesitant to add a whole system for managing per-instance settings just for this, but if we do have plans to add more instance settings in the future perhaps it's justified.

Anecdotally, I have also found the default 20 px padding too small, when viewing a single feature. I wonder if it would make sense to just bump that up? If we did that, would it reduce the need for a setting overall? (And what effect would it have on multi-feature zooms, and other cases?)

(Also curious if zoomToVectors could call zoomToLayer to consolidate this logic?)

Hmm maybe? I need to refresh and see if there was a reason to keep them separate. But it seems to make sense!

@mstenta
Copy link
Member

mstenta commented Jan 12, 2021

I'm a bit hesitant to add a whole system for managing per-instance settings just for this, but if we do have plans to add more instance settings in the future perhaps it's justified.

Ah sorry - forgot to read #94 - seems like there is already justification for a settings system with that too. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants