2

I am working on an lit-element image component that has two views.

  • A single view where the user can see just one image at a time.
  • A grid view where the user can see all the images at once.

I can switch between the two views at the press of a button. When I am in 'grid view' I have setup a way to select an image when you click on it it will be marked as selected.

When I load images in and switch to grid view I can select and deselect the images with no issues. If I click back to single view then return to grid view I cannot select any images. If I do it a third time I am able to select Images again.

With debugging I found that the event listener that is added to the image is not being removed. I have tried several different ways through my research on this issue with no luck with resolving this issue.

here is the code that I have and would like some second eyes on to help out

case 'grid':
                    this.changeViewState(buttonName);
                    imageItems.forEach((image) => {
                        image.addEventListener('click', this.selectImage.bind(this,image), false);
                    });

                    hiImage.viewMode = 'grid';
                    break;
 selectImage(image){
        let hiImage = this._getImageSection().querySelector("hi-images");
        hiImage.dispatchEvent(new CustomEvent('selected-images', {
            detail: { image: image }
        }));

        image.removeEventListener('click', this.selectImage.bind(this,image));
    }

Thanks in advance for any help I can get still new at javascript and lit element.


Update: This is now what i currently have . this is the button click to switch to grid view

case 'grid':
                    this.changeViewState(buttonName);
                    imageItems.forEach((image) => {
                        const boundFn = this.selectImage.bind(this, image);
                        this.boundFnsByImage.set(image, boundFn);
                        image.addEventListener('click', boundFn);
                    });
                    hiImage.viewMode = 'grid';
                    break;

this is my button click to switch to single view this is where the event listeners should be removed.

case 'single':
                    console.log('gridswitch button was clicked');
                    this.boundFnsByImage.forEach((boundFn, image) => {
                        image.removeEventListener('click', boundFn);
                      });
                    this.changeViewState(buttonName);
                    hiImage.viewMode = 'single';
                    hiImage.dispatchEvent(new CustomEvent('set-nav-icons'));
                    break;

The SelectImage function which pushes the event dispatch

selectImage(image) {
        let hiImage = this._getImageSection().querySelector("hi-images");
        hiImage.dispatchEvent(new CustomEvent('selected-images', {
            detail: { image: image }
        }));
        console.log('click selected image');
    }

and the properties and constructor of this part of the component

static get properties() {
        return {
            name: { type: String }, // ID of the button
            tooltip: { type: String },// sets a tooltip for the button
            icon: { type: String }, // sets the icon of the button 
            for: { type: String }, // binds label to input control when it is used
            confirmationType: { type: Boolean },
            boundFnsByImage: { type: WeakMap }
        }
    }

    constructor() {
        super();
        this.confirmationType = false;
        this.boundFnsByImage = new WeakMap();
    }

What Im looking to do So when the gridswitch button is clicked it goes from single view to gridview- where I can select one or more images.

Then when the single view button is clicked it switches back to singleview and should remove the event listeners from grid view images.

So when I switch back to grid view I can select them again

What is happening still. The event listener is getting added to the images on the second and subsequent switiches to grid view meaning it is doing it more than once so when i click on it it fires twice.

what is not happening the event listeners are not being removed from the images so they can be properly set again when switching back to grid view .

DRW
  • 335
  • 1
  • 3
  • 17

1 Answers1

4

When you .bind a function, you create an entirely new function, one which is not === to any function that may have existed previously (even if created with the same procedure). So

this.selectImage.bind(this,image) === this.selectImage.bind(this,image)

will evaluate to false - the expressions on each side of the === reference different function objects.

Because selectImage looks to remove the listener, one option would be to instead use { once: true } to ensure the listener only runs once:

imageItems.forEach((image) => {
  image.addEventListener('click', this.selectImage.bind(this, image), { once: true });
});

The alternative would be to store the bound function somewhere so removeEventListener can be called with it later, eg

const boundFnsByImage = new Map();
// ... 
imageItems.forEach((image) => {
  const boundFn = this.selectImage.bind(this, image);
  boundFnsByImage.set(image, boundFn);
  image.addEventListener('click', boundFn);
});

and then retrieve it with

selectImage(image){
  let hiImage = this._getImageSection().querySelector("hi-images");
  hiImage.dispatchEvent(new CustomEvent('selected-images', {
    detail: { image: image }
  }));
  const boundFn = boundFnsByImage.get(image); // <----------------------
  // if you want to remove the listener when clicked:
  image.removeEventListener('click', boundFn);
}

Also note that there's no need to pass the third useCapture argument to addEventListener unless you want to use capturing - it'll default to false anyway.

If you want to remove all listeners at once (like during a view switch), then iterate through the Map:

boundFnsByImage.forEach((boundFn, image) => {
  image.removeEventListener('click', boundFn);
});
CertainPerformance
  • 356,069
  • 52
  • 309
  • 320
  • Thanks for the assistance, when I use the second option and i switch to grid view the second time I get the following error Action-Button.js:107 Uncaught ReferenceError: boundFnsByImage is not defined at HTMLElement.selectImage (Action-Button.js:107) – DRW Nov 05 '19 at 18:17
  • You need to make sure the `boundFnsByImage` variable is in scope at both points, both where you assign to it and where you retrieve from it. – CertainPerformance Nov 06 '19 at 00:29
  • I set this up correctly this time, but alas did not resolve the issue at had the event listener is still being added again after each switch of the views. Which is causing it to have like a double click action instead of one click. any other suggestions ? – DRW Nov 06 '19 at 00:47
  • It sounds like you not only want to remove the listener when clicked, but also have a separate function that iterates through all listeners and removes them? Call `forEach` on the map to iterate and remove – CertainPerformance Nov 06 '19 at 00:52
  • I ended up putting the boundFnsByImage into a property then set the boundFnsByImage.forEach in my button click to return to single view so it will run it when returning to single view and remove the listeners but I am getting the following error . -- Uncaught TypeError: this.boundFnsByImage.forEach is not a function at HTMLLabelElement. (Action-Button.js:87) – DRW Nov 06 '19 at 02:55
  • Sounds like a scope issue again - maybe your `this` context is wrong at that point, but without seeing the code, it's hard to say. See https://stackoverflow.com/questions/20279484/how-to-access-the-correct-this-inside-a-callback It'd be trivial to achieve it by just using a standalone variable, though, you don't have to put it as a property. – CertainPerformance Nov 06 '19 at 02:57
  • If it's not working for you, can you edit a [MCVE] into the question so that we can debug it? – CertainPerformance Nov 06 '19 at 03:52
  • I edited the question with the minimal code I could at this point. let me know if this helps – DRW Nov 07 '19 at 01:42
  • Oh crap, it's my fault. It's a WeakMap, not a Map, so it doesn't allow for Map.prototype.forEach. WeakMaps are better for garbage collection, but because of that, they aren't iterable. Use a Map instead of a WeakMap and you'll be able to invoke `Map.prototype.forEach` – CertainPerformance Nov 07 '19 at 05:31
  • I did end up changing it to map, but when i run through it, it looks like it is not hitting the remove part when switching. not sure why its not but yeah . so still in the same boat. Still working my way to a solution. – DRW Nov 07 '19 at 06:49
  • I think I see where the problem of the click event listener is not being removed. When setting the property this.boundFnsByImage through the grid click I see the items are set in the map. When I click the single this.boundFnsByImage is empty. So I am not sure why it's coming back empty when it was just set. Any Additional help would be great. – DRW Nov 10 '19 at 20:42
  • We have fixed the issue . we had to move from this to the action bar after doing that everything works as expected. Thank you again for the help. – DRW Nov 11 '19 at 19:00