Events for selecting

[tsparksh]

Fixes #460

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

• PR is descriptively titled 📑 and links the original issue above 🔗
• tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with grunt
• code is in uniquely-named feature branch and has no merge conflicts 📁
• screenshots/GIFs are attached 📎 in case of UI updates
• @mention the original creator of the issue in a comment below for help or for a review

Example:

$(img).on("deselectImage", function() {
  console.log("deselected");
});

$(img).on("selectImage", function() {
  console.log("selected");
});

[tsparksh]

I used selectImage because select event is already used in jQuery

(Tests works fine with #493)

[Uzay-G]

Hey @tsparksh are you working on this? The task is open on Github

[tsparksh]

@Uzay-G, on the gci dashboard for some reason there are two identical tasks with a link to this issue, my task is marked as accepted

[Uzay-G]

Ok alright thanks

[VladimirMikulic]

@tsparksh 👍

[VladimirMikulic]

This is optional, but instead of using jQuery and adding it to the test dependecies you could have done this in JavaScript:

var selectImageEvent = new Event('selectImage');
this.dispatchEvent(selectImageEvent);

[sashadev-sky]

@VladimirMikulic thank you for reviewing! @tsparksh haven't gotten the chance to review this yet but for now can you please implement the suggestion regarding jQuery.

jQuery is already a dependency so using it was fair game but I actually have been wanting to update the few lines in the repo that use it to vanilla JS and remove it

[sashadev-sky]

also I think running this and pushing will fix your travis build:

$ rm -rf package-lock.json node_modules
$ npm install

[tsparksh]

I can’t fix it in the next week, but I’ll definitely do it as I return, thanks!

…

On 10 Jan 2020, at 13:49, Sasha Boginsky ***@***.***> wrote:  also I think running this and pushing will fix your travis build: $ rm -rf package-lock.json node_modules $ npm install — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[sashadev-sky]

@tsparksh okay no problem! Please note that the mentor approved the task for you so that you could move on to others in my absence, but you still need to complete them (merged PR) to actually get the points!

[tsparksh]

Usage:

image = img._image
image.addEventListener("selectImage", function(event) {
    console.log(event.type);
  });
image.addEventListener("deselectImage", function(event) {
    console.log(event.type);
  });

[tsparksh]

@sashadev-sky, can you check, please

[sashadev-sky]

I think the best thing to do here woudl be to use leaflet's fire (it's pretty much like trigger) so sorry didnt look into this more thoroughly before.

This would be an example of the sort of syntax looking for:

Leaflet.DistortableImage/src/edit/handles/EditHandle.js

Lines 33 to 35 in e0b5130

	_onHandleDragStart: function() {
	this._handled.fire('editstart');
	},

so maybe something like this.fire("select") and putting it up higher in the method like after the line:

if (!edit.enabled()) { return false; }

@sagarpreet-chadha @VladimirMikulic @jywarren @sidntrivedi012 what do ya'll think
** as background ** i think these methods will be utilized by MK to allow for synchronous editing.

[VladimirMikulic]

I agree @sashadev-sky. Consistency is important.
I didn't know that fire method existed, but know that I know we should definitely use it instead of 'manually' creating an event and dispatching it.