Skip to content

Fixed a problem when using the open function #341

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

Merged
merged 2 commits into from
Jan 14, 2016

Conversation

RareDevil
Copy link
Contributor

I had a problem where e.offsetX and e.offsetY is undefined. This was mainly when i used the open function like the example below.
$(selector).contextMenu({ x: ShowX, y: ShowY });
So i have made a check if they are undefined and if they are pageX and pageY are used instead.

@RareDevil
Copy link
Contributor Author

@bbrala Have the time to look at this?

@bbrala
Copy link
Member

bbrala commented Jan 14, 2016

The first thing that i was thinking is that ShowX and ShowY aren't documented so shouldn't you use other properties to do this?

http://swisnl.github.io/jQuery-contextMenu/docs/events.html

@RareDevil
Copy link
Contributor Author

Well ShowX and ShowY is just a name i used instead of typing in a random number like this
$(selector).contextMenu({ x: 50, y: 25 });
But the main thing was that when i used this command i experienced that e.offsetX and e.offsetY was undefined in the code. If you try to look at code i changed then before my fix it used e.offsetX and e.offsetY and those was undefined so all i did was made a check to make sure that they was not undefined before they was used and if they is undefined e.pageX, e.pageY is used instead.

@bbrala bbrala merged commit 9a12d9e into swisnl:master Jan 14, 2016
@bbrala
Copy link
Member

bbrala commented Jan 14, 2016

Interestingly the docs even say: // or $('.context-menu-one').contextMenu({x: 100, y: 100});

I've merged your change, thanks again for the contribution :)

bbrala added a commit that referenced this pull request May 24, 2016
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.

2 participants