-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Converted jquery.dragster.js from jquery plugin to vanilla js #3646
Conversation
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.
Thanks @niklabh! It works great. See comments for other request.
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.
Small changes needed, but in general looks good to me (besides the @saturninoabril comments).
Removed target.off jquery function in last commit. Using removeEventListener from inside the lib using js closure. |
Doing querySelector instead of querySelectorAll inside the lib now. |
10d8be4
to
1fd2104
Compare
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.
@niklabh Awesome, looks good to me!
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.
LGTM
Merged! Thanks @niklabh! 🎉 |
* Converted jquery.dragster.js to vanilla js * fix test * removed jquery * unbinding events on unmount; not checking class before classList.add * added new test; fixed old test
…most#3646) * Converted jquery.dragster.js to vanilla js * fix test * removed jquery * unbinding events on unmount; not checking class before classList.add * added new test; fixed old test
…most#3646) * Converted jquery.dragster.js to vanilla js * fix test * removed jquery * unbinding events on unmount; not checking class before classList.add * added new test; fixed old test
Summary
Converted jquery.dragster.js from jquery plugin to vanilla js
Ticket Link
Fixes: mattermost/mattermost#11429