intercellular / cell

A self-driving web app framework
https://www.celljs.org
MIT License
1.5k stars 93 forks source link

suggestion: custom eventlistener #33

Open codehz opened 7 years ago

codehz commented 7 years ago

I think binding event listener should use addEventListener rather than set property directly....

devsnek commented 7 years ago

this would require either keeping a list of all known events or blindly attaching anything that starts with on which could be bad.

Caffeinix commented 7 years ago

Polymer's approach to this is to use on- (with a dash) as the prefix in HTML, or a listeners object in JavaScript.

If you say onload in an HTML file, you are setting the onload HTML attribute, which is going to expect a string. If you say on-load, you're attaching an event listener for the load event, which is going to expect a function. In JavaScript, meanwhile, you can use a listeners object to map between event names and the functions they should invoke. In Polymer, you only use this to set event listeners on the parent component, because all the children are in the HTML file, but I could imagine Cell adapting it for use on any component (since all cells are standalone). So something like:

Item = {
  $cell: true,
  $type: "button",
  $text: "Clicky?",
  $listeners: {
    click: function(event) { console.log("Clicky!"); }
  }
}

I like this approach because of its parallel with the $components array (though given the discussion in #111, maybe $listeners should be $behaviors or something to be suitably generic). Another nice and unexpected feature is that as of ES6, object key names can be read from variables. So if you had a library that gave you untypoable constants for built-in (or custom) JavaScript event names, you could do:

Item = {
  $cell: true,
  $type: "button",
  $text: "Clicky?",
  $listeners: {
    [Event.CLICK]: function(event) { console.log("Clicky!"); }
  }
}
gliechtenstein commented 7 years ago

This was intentional because I wanted to keep it simple. Adding this seemingly innocent feature will introduce a lot of complexities IMO. First of all we need to be able to keep track of all the event handlers so that they don't get attached multiple times, and also do garbage collection. Also implementing this in a decentralized manner will require a lot of thought. Lastly by doing this we will have added another set of syntax to remember.

That said, I actually did think about this point before releasing the library, but couldn't think of actual real-world use cases where you won't be able to do something because of this issue. I actually wanted a discussion like this to arise because I wanted to ask where this may be needed.

So if anyone has an idea about why this may be necessary (as in "we can't do _____ if we don't have this feature") please share. I would really appreciate it.

Caffeinix commented 7 years ago

Let's have a look at what would be required to manage event listeners without any custom solution. I'm going to suggest that, because the onfoo attributes are old and sort of fraught, we instead use the addEventListener API. So:

Item = {
  $cell: true,
  _model: [],
  _template: function(model) {
    return {
      $components: [{
        $text: model.description,
      }, {
        $type: 'button',
        $text: model.actionText,
        class: 'clicky-button',
      }]
    }
  },
  _clickHandler: function(event) {
    // Event handling logic goes here.
  },
  $init: function() {
    this._model =  [{
      description: 'Hey look, a thing!', 
      actionText: 'Click the thing!',
    }];
  },
  $update: function() {
    for (let button of this.querySelectorAll('clicky-button')) {
      button.removeEventListener(this._clickHandler.bind(this));
    }
    this.$components = this._model.map(this._template);
    for (let button of this.querySelectorAll('clicky-button')) {
      button.addEventListener(this._clickHandler.bind(this));
    }
  }
}

(Note: I have not run this, it may be buggy.)

This isn't too bad, but I've had to architect things rather carefully to make it this easy: instead of populating _model declaratively, I had to add an $init method and populate it imperatively to trigger $update for the initial render. Had I not done that, I'd have had to add the event listeners in $init as well, and then rely on $update to remove them when it regenerates the DOM.

So to answer the question as best I can: I'm not aware of anything that can't be done without native support for event listeners, but having it allows us to do a lot of imperative things declaratively instead. Which, I think, is also true of cell itself, so perhaps that is a compelling argument. :)