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

SetItemListener allowing duplicate click on item #150

Closed
angelocavallet opened this issue Sep 17, 2018 · 8 comments
Closed

SetItemListener allowing duplicate click on item #150

angelocavallet opened this issue Sep 17, 2018 · 8 comments

Comments

@angelocavallet
Copy link
Collaborator

setItemListener(li, index) {

Hello,
I don't know if this is what it's meant to be but when you do more than one click in an item the callback is getting called as many times as you clicked before closeMenu() is called.

You can simulate this on the example given on README
https://jsfiddle.net/jonataswalker/ooxs1w5d/
Just double click on "Add Marker" and you will see that two markers are added instead one.

I wrote a workaround for this (probably not the best and most elegant solution, but it works) as follows:

        setItemListener(li, index) {
            const this_ = this;
            let statusClick = true; //created a local flag to control when call the callback or not
            if (li && typeof this.items[index].callback === 'function') {
                (function (callback) {
                    li.addEventListener(
                        'click',
                        function (evt) {
                            evt.preventDefault();
                            if(statusClick){ //check if flag is true execute callback
                                statusClick = false; //set flag to false to prevent it get executed more than once
                                const obj = {
                                    coordinate: this_.getCoordinateClicked(),
                                    data: this_.items[index].data || null,
                                };
                                this_.closeMenu();
                                callback(obj, this_.map);
                            }
                        },
                        false,
                    );
                })(this.items[index].callback);
            }
        }

@jonataswalker
Copy link
Owner

Hi @angelocavallet would handle a PR with your fix?

@angelocavallet
Copy link
Collaborator Author

Yeah, it would be awesome!
I've never handle a PR, so I read about it and created a branch for it (v1.2.1) but I'm not able to push to origin.
I think you need to allow me to do that, right?

@jonataswalker
Copy link
Owner

Actually you didn't read the right article. Please read this one instead.

@angelocavallet
Copy link
Collaborator Author

I think it's done.

@jonataswalker
Copy link
Owner

Sorry @angelocavallet I didn't check the fiddle when you posted.

I've updated it fixing the icons URL and can't see what you've reported. Can you confirm it?

https://jsfiddle.net/jonataswalker/ooxs1w5d/

@angelocavallet
Copy link
Collaborator Author

No problem @jonataswalker.
On your fiddle, if you fast double click on "Add a marker" item from contextmenu before the menu get closed, then it's action is executed twice, creating two markers.

@angelocavallet
Copy link
Collaborator Author

You can notice that it executed twice when you try to remove the marker, because there is two markers, so you remove the first one and you'll see the other behind

@jonataswalker
Copy link
Owner

Fixed by #153

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

No branches or pull requests

2 participants