Issue with addEventListener("click") on link

[olliekav]

This is a strange one, and I'm having trouble finding the solution.

I have a Responsive menu which I call by adding an eventListener to a menu link and setting a body class.

<%= link_to("Menu", "#", class: 'resp-nav', data: { no_turbolink: true }) %>

var ready;
ready = function() {
   var menuLink = document.querySelector('.resp-nav')
   menuLink.addEventListener("click", function(e) {
      e.preventDefault();
      console.log('clicked');
      if (hasClass(html, 'resp')) {
        removeClass(html, 'resp')
        removeClass(menuLink, 'open')
      }
      else {
        addClass(html, 'resp')
        addClass(menuLink, 'open')
      }
    }, false);
}

document.addEventListener("DOMContentLoaded", function() {
    ready();
  }, false);

document.addEventListener("page:load", function() {
    ready();
  });

Yet this click event is never fired, I have other functions in ready that work fine. I've tried all manner of workarounds to get it to work but can't find a solution. I've added it as an issue here because if I turn off reactize and just use vanilla turbolinks the above works which leads me to believe it's ether a problem with reactize or maybe react. Once I've navigated to another page and page:load is fired it works as well. Any ideas?

[olliekav]

Okay, after playing around with this I found a solution. If I attached an event listener higher up the chain and use event delegation to bubble the event it would work.

window.addEventListener("click", function(e) {
      console.log('clicked');
      if(e.target && e.target.className == "resp-nav") {
        console.log('Here again')
        if (hasClass(html, 'resp')) {
          removeClass(html, 'resp')
        }
        else {
          addClass(html, 'resp')
        }
        e.preventDefault();
      }
    }, false);

And then I attach this to the page:fetch event rather than page:load it works.

[olliekav]

Actually I don't think this works, investigating further.

[ssorallen]

Interesting... I will try to reproduce the issue. If you figure out a small test case in the mean time, please post it.

[olliekav]

I've managed to get this working now by doing the below. It seems to be trying to rebind the eventlistener each time I run the page:load events. So I only run it once on initial DOMContentLoaded. I'm still a bit unclear as to way this happens as I'm still a bit of a novice with vanilla JS but at least its working now.

ready = function() {

    var body = document.querySelector('body'),
        html = document.querySelector('html'),
        wrapper = document.getElementById("wrapper"),
        menuLink = document.querySelector('.resp-nav');

    respNav = function(e) {
     if(e.target && e.target.className === "resp-nav") {
        e.preventDefault();
        if(hasClass(html, 'resp')) {
          removeClass(html, 'resp')
        } else {
          addClass(html, 'resp')
        }
      }
    }
}

document.addEventListener("DOMContentLoaded", function() {
    ready();
    window.addEventListener("click", respNav, false);
  }, false);

  document.addEventListener("page:load", function() {
    ready();
    window.scrollTo(0,0);
  }, false);

The site is now up at http://olliekav.com/ so you can have a look.

[ssorallen]

Your site looks great!

I noticed you tried to enable the Turbolinks transition cache, and that might be causing problems. I will try your example and figure out how to best fix this. Thanks for the detailed bug info.

[olliekav]

Great, thanks!

[ssorallen]

Your last post seems to touch on what's breaking. Because turbo-react keeps DOM nodes around for a long time, you need to manage your event listeners differently than you normally would. When the page is about to unload, you need to unbind your event listeners because the DOM node might stick around.

Turbolinks fires a page:before-unload right before the old page is going to be removed, so I recommend removing your listener there and using page:load to bind.

function ready() {
  ...
}

function respNav() {
  ...
}

document.addEventListener("page:load", function() {
  ready();
  window.addEventListener("click", respNav, false);
  window.scrollTo(0,0);
});

// Clean up any event listeners so `page:load` can bind on a
// fresh set of elements and not double bind.
document.addEventListener("page:before-unload", function() {
  window.removeEventListener("click", respNav);
});

[ssorallen]

Your initial problem happened because Reactize was running on load, which fires after DOMContentLoaded. It makes more sense for Reactize to run in DOMContentLoaded, and it should make your first example work as expected.

Try out v0.7.0 if you get a chance, and let me know if it works as expected.

[olliekav]

I still need to keep...

document.addEventListener("DOMContentLoaded", function() {
    ready();
    window.addEventListener("click", respNav);
  }, false);

around for initial first site load though, as the page:load doesn't fire then.

Works as expected though with 0.7.0, thanks for looking into this.

[ssorallen]

Aha yup, the DOMContentLoaded listener still makes sense.

I will put together a best practices section for adding and removing standard event listeners so other people can benefit from the things you pointed out here.

Thanks for the feedback, and let me know if you run into other issues. I am going to investigate SVGs further.

[olliekav]

Will do, thanks!