-
Notifications
You must be signed in to change notification settings - Fork 27
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
Remove unnecessary code that breaks jQuery 3 #318
Conversation
event.props was removed in jquery 3 (jquery/api.jquery.com#405). It looks like this wasn't doing anything useful, and broke the schedule editor.
It is not present in modern Firefox. Use the event argument we got.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a couple of questions, but looks good to me.
if (e.stopPropagation) { | ||
e.stopPropagation(); // stops the browser from redirecting. | ||
} | ||
e.preventDefault(); // stops the browser from redirecting | ||
|
||
var slot = e.target.getAttribute('data-slot'); | ||
var venue = e.target.getAttribute('data-venue'); | ||
|
||
//noinspection JSUnresolvedVariable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove this comment now?
Any idea how the event.
references worked before? Some sort of crazy magic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently it's a legacy thing: http://stackoverflow.com/a/20522980
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool. Let's remove the noinspection
then.
Not sure what they were used for, the Internet says IntilliJ. Add a JSHint/JSLint declaration of used globals.
👍 |
event.props was removed in jquery 3 (jquery/api.jquery.com#405).
It looks like this wasn't doing anything useful, and broke the schedule
editor.