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

Add hideOnMove option to allow map to move without hiding context menu #282

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ztolley
Copy link

@ztolley ztolley commented Mar 22, 2024

Allows a user to provide an option to the constructor to disable hiding the context menu when the map is moved, either through code or dragged by the user.

A default value of true is provided to preserve current behaviour.

@ztolley
Copy link
Author

ztolley commented Mar 22, 2024

Related to issue #263

@jonataswalker
Copy link
Owner

@GastonZalba are you willing/able to review this PR? I'm not sure about how this should be handled.

@GastonZalba
Copy link
Contributor

Hello @jonataswalker. First, this Pull Request has nothing to do with the problem/feature-request initially reported in issue #263, it is not going to fix what was described there so it should not be related (the issue should be remain open even if this is merged).

Either way, it adds new functionality with only a small code change. But i think that this addition could be used to accommodate some small things in the code that this PR interacts with (the this.mapMoveListener attribute is declared and not used, and the movestart event is not cleared in the removeListeners function).

On the other hand, if setting this new parameter is intended from the constructor only, probably checking how it is set should be before setting the mapMove listener at the setMap method, and not every time the event is fired (and doing nothing if disabled). If it is intended so that the configuration can be changed even after the instance is created, perhaps it would be nice to specify this in the README: the ability to modify the option with instance.options.hideOnMove = true;, for example, or even better, create a setHideOnMenu method.

Sorry for my english :(

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