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

do not zoom if marker was dragged #193

Merged
merged 2 commits into from
Jun 7, 2022
Merged

do not zoom if marker was dragged #193

merged 2 commits into from
Jun 7, 2022

Conversation

karussell
Copy link
Member

@karussell karussell commented Jun 5, 2022

The current behaviour is pretty annoying and so I disabled this, in this PR.

(not sure if this is the proper way to do this or if we should have the zoom parameter e.g. in SetPoint)

Related to #177

@karussell karussell added this to the 0.4 milestone Jun 5, 2022
@Janekdererste
Copy link
Collaborator

Janekdererste commented Jun 7, 2022

I agree that we need something like this. I could imaginge that one wants an enum of map behaviour at some point. But this can be refactored later.

From my side this can be merged.

@karussell karussell merged commit 2e58b18 into master Jun 7, 2022
@karussell karussell deleted the no_drag_focus branch June 7, 2022 17:50
@easbar
Copy link
Member

easbar commented Jun 9, 2022

Cool, thanks! This fixed #163, right? Makes drawing markers a lot more enjoyable for sure :)

Putting the zoom parameter into SetPoint makes sense to me. The only thing I was wondering is if the QueryStoreState and the RoutingArgs are the right place for the zoom parameter, because it is only related to our UI code and not the routing request. I mean I can see how it works this way and maybe it's not too important, but it seems a bit odd, so I was wondering if there was another way to do this?

@karussell
Copy link
Member Author

karussell commented Jun 9, 2022

The only thing I was wondering is if the QueryStoreState and the RoutingArgs are the right place for the zoom parameter

To be honest I do not know how to implement it properly. For RoutingArgs you are probably right that this does not belong there and I can have a look if I can move it out. But for QueryStoreState we could say that this zoom property (if "zooming to the query result" is enabled) should we stored there. Not sure.

@Janekdererste
Copy link
Collaborator

The only thing I was wondering is if the QueryStoreState and the RoutingArgs are the right place for the zoom parameter

I had the same thought, but then I didn't have another idea quickly. Since we all don't feel good about this we could think of a better way to do this, or at least remember it for the next UI related property and do it then.

@easbar
Copy link
Member

easbar commented Jun 10, 2022

Ok, good idea. I added a todo comment here: e95f88e

The zoom property is like a parameter that describes the UI behavior, but then it is also only 'valid' for a specific request: When dragging the marker it is set to false for the next request, but when we type two cities in the query boxes directly it should be true for the next request...

ZeroGxMax pushed a commit to minhhpkp/graphhopper-maps that referenced this pull request Nov 12, 2024
* do not zoom if marker was dragged

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

Successfully merging this pull request may close these issues.

3 participants