akre54 / Backbone.D3View

A Backbone View powered by a D3 DOM wrapper
MIT License
25 stars 2 forks source link

event delegation w/ selector for elements that do not yet exist #4

Open jameslaneconkling opened 9 years ago

jameslaneconkling commented 9 years ago

Having trouble delegating events for elements that might not yet exist. E.g. the below code does not work, unless I bind the 'mouseenter' handler after the graph has already been created.

var GraphView = Backbone.D3View.extend({
  tagName: 'svg',

  initialize: function(){
    this.delegate('mouseenter', '.node', function(e){
      console.log(e);
    });

    this.buildGraph();
  }
};

Or am I misunderstanding the point of event delegation, and/or implementing it incorrectly?

akre54 commented 9 years ago

Right, so D3 doesn't do event delegation, only event binding.

We could do something similar to NativeView and use Element#matches (basically delegate an event on the View's el, then check if it matches the selector), or we could leave this in userland. I'd be happy to entertain a pull with tests if you'd like to give one a shot.

jameslaneconkling commented 9 years ago

Yeah, agreed. Let me take a crack at it and see if I can implement something.

Using D3View means there's a good, standard way in which to extend D3, so this one could be left up to the user. But it's contained enough that I'd vote to pull it in.

jameslaneconkling commented 9 years ago

OK, took a first swipe at it.

    delegate: function(eventName, selector, listener) {
      var el = this.el;

      if (listener === undefined) {
        listener = selector;
        selector = null;
      }

      // d3 needs `uniqueId` to delegate more than one listener per event type.
      var namespace = '.' + uniqueId++;

      var map = _eventsMap[this.cid] || (_eventsMap[this.cid] = {}),
          handlers = map[eventName] || (map[eventName] = []);

      handlers.push({selector: selector, listener: listener, namespace: namespace});

      el.addEventListener(eventName, function(event){
        var node = event.target,
            idx = 0;

        if(! selector){
          listener.call(node, event, node.__data__, idx++);
          return;
        }
        while(node && node !== el){
          if(node.matches(selector)){
            listener.call(node, event, node.__data__, idx++);
          }
          node = node.parentNode;
        }
      });
      return this;
    },

implemented in vanilla javascript. el.matches() has decent coverage--could add prefixes or rewrite, if necessary.

Thoughts?

akre54 commented 9 years ago

This looks great! Couple things.

  1. Send this as a pull, much easier to collaborate / annotate lines that way.
  2. listener should be called with the view's this context instead of node. I know we're balancing D3 conventions and Backbone conventions but let's try to fit with most assumption made by Views.
  3. We need some way to set the delgateTarget/bubblingTarget of the Event to tell the listener what element it was originally delegated to (though see https://github.com/akre54/Backbone.NativeView/issues/16 for some issues around naming. Any thoughts?)
  4. Make sure to set d3.event (and cache the old one). Many of D3's methods rely on it being set.
  5. Do we need namespace now? Mostly that's just a convention in Backbone to make undelegateEvents work easier. If we're binding our own events it shouldn't be necessary.
  6. el.matches is pretty easy to pull out for maximum browser coverage

Otherwise this looks great. Let's add some tests and get this merged in.