Popup won't close

Question

Popup won't close when i click close button, i tried debugging with console.log and it looks like closeButton.onclick function doesn't run at all for some reason.

When running close() function manually from the console everything works fine.

class Popup {
    constructor(content){
        this.div = document.createElement("div");
        this.div.className = "block";
        //tried positioning popup into the center of the screen, doesn't work yet
        this.div.style.position = "fixed";
        this.div.style.margin = "auto auto";
        
        //caption

        this.caption = document.createElement("div");
        this.caption.style.textAlign = "right";
        
        //closeButton

        this.closeButton = document.createElement("button");
        this.closeButton.textContent = "X";
        this.closeButton.onclick = this.close;

        document.body.appendChild(this.div);
        this.div.appendChild(this.caption);
        this.caption.appendChild(this.closeButton);
        this.div.innerHTML += content;
    }
    close(){
        this.div.parentNode.removeChild(this.div);
        delete this;
    }
}

new Popup("close me");

That's how it looks like:

var popup = new Popup("hm hello");

---

SOLUTION:

The issue was happening because:

• I was appending content of the popup right into main div using +=. That made DOM refresh and onclick trigger reset.

• this.closeButton.onclick = this.close; here onclick trigger will execute close function and also will overwrite this keyword, so it contains a button that called trigger, not the Popup object. I decided to put Popup into a variable that is visible to onclick function. Now everything works fine.

class Popup {
    constructor(content){
        this.div = document.createElement("div");
        this.div.className = "block";
        this.div.style.position = "fixed";
        this.div.style.margin = "auto auto";
        
        //делоем капшон

        this.caption = document.createElement("div");
        this.caption.style.textAlign = "right";
        
        //кнопка закрытия

        this.closeButton = document.createElement("button");
        this.closeButton.textContent = "X";
        let popup = this;
        this.closeButton.onclick = function(){popup.close()};

        this.content = document.createElement("div");
        this.content.innerHTML = content;

        this.caption.appendChild(this.closeButton);
        this.div.appendChild(this.caption);
        this.div.appendChild(this.content);
        document.body.appendChild(this.div);
    }
    close(){
        this.div.parentNode.removeChild(this.div);
        delete this;
    }

    
}

new Popup("hello guys");

Answer 1

I've finally found a final solution. As Scott Marcus mentioned in his answer, i will have troubles inside close function, so i decided to put Popup object into a variable that is visible to close function. Everything works fine without applying classes. Though it may look like a bad code.

class Popup {
    constructor(content){
        this.div = document.createElement("div");
        this.div.className = "block";
        this.div.style.position = "fixed";
        this.div.style.margin = "auto auto";
        
        //делоем капшон

        this.caption = document.createElement("div");
        this.caption.style.textAlign = "right";
        
        //кнопка закрытия

        this.closeButton = document.createElement("button");
        this.closeButton.textContent = "X";
        let popup = this;
        this.closeButton.onclick = function(){popup.close()};

        this.content = document.createElement("div");
        this.content.innerHTML = content;

        this.caption.appendChild(this.closeButton);
        this.div.appendChild(this.caption);
        this.div.appendChild(this.content);
        document.body.appendChild(this.div);
    }
    close(){
        this.div.parentNode.removeChild(this.div);
        delete this;
    }

    
}

new Popup("hello guys")

P.S.

What's the point of this restriction?

Answer 2

The issue is that right here:

this.div.innerHTML += content;

When you assign a value to .innerHTML, the entire previous value is overwritten with the new value. Even if the new value contains the same HTML string as the previous value, any DOM event bindings on elements in the original HTML will have been lost. The solution is to not use .innerHTML and instead use .appendChild. To accomplish this in your case (so that you don't lose the existing content), you can create a "dummy" element that you could use .innerHTML on, but because of performance issues with .innerHTML, it's better to set non-HTML content up with the .textContent property of a DOM object.

You were also going to have troubles inside close() locating the correct parentNode and node to remove, so I've updated that.

class Popup {
  constructor(content){
    this.div = document.createElement("div");
    this.div.className = "block";
    this.div.style.position = "fixed";
    this.div.style.margin = "auto auto";
    
    //caption
    this.caption = document.createElement("div");
    this.caption.style.textAlign = "right";
    
    //closeButton   
    this.closeButton = document.createElement("button");
    this.closeButton.textContent = "X";
    this.closeButton.addEventListener("click", this.close);
    
    this.caption.appendChild(this.closeButton);
    this.div.appendChild(this.caption); 
    
    // Create a "dummy" wrapper that we can place content into
    var dummy = document.createElement("div");
    dummy.textContent = content;
    
    // Then append the wrapper to the existing element (which won't kill
    // any event bindings on DOM elements already present).
    this.div.appendChild(dummy);
    document.body.appendChild(this.div);
  }  
  close() {
      var currentPopup = document.querySelector(".block");
      currentPopup.parentNode.removeChild(currentPopup);
      delete this;
  }
}
    
var popup = new Popup("hm hello");