For discussion: Extract overridable methods to Backbone.Stickit

[trabianmatt]

Our use of stickit in titanium-backbone requires the ability to override the way elements are queried and updated. This commit moves isObservable (which replaces isFormEl($el) || isContenteditable($el)), updateEl, and getElVal to Backbone.Stickit, which can be changed by a library that uses Backbone.stickit.

For example, in titanium-backbone we can then do:

_.extend Backbone.Stickit,

  isObservable: ($el) ->
    $el[0]._viewName in ['TextField', 'TextArea']

  updateEl: ($el, val, config) ->

    updateMethod = config.updateMethod || 'text';

    switch $el[0]._viewName
      when 'TextField', 'TextArea'
        $el.val val
      else
        $el[updateMethod] val

  getElVal: ($el) ->

    switch $el[0]._viewName
      when 'TextField', 'TextArea'
        $el.val()

Is this idea worth pursuing further? I can certainly keep it in a separate fork but if others find value then I'd be happy to do any needed cleanup work (it passes all tests as-is but could probably be cleaned up a bit).

[delambo]

Whoa, that's a lot of changes, @trabianmatt! I like the general direction of this pull and I think we could end up with some extensions like this, but I would like to make it more additive/focused.

Instead of overriding and forcing users to rewrite the core functionality of Stickit, why not give users the ability to add observables/elements or to override existing elements (like input[type=text]). I was thinking of an interface like the following (rough draft):

// Add handling for <img> tags - bind the `src` attribute to the model attribute value.
Backbone.Stickit.addObservable({
  selector: 'img',
  update: function($el, val, config) {
    $el.attr('src', val);
  }
});

// Override Stickit's handling of `input[type="text"]` with a custom implementation.
Backbone.Stickit.addObservable({
  selector: 'input[type="text"]',
  events: ['change'],
  update: function($el, val, config) {
    $el.val('custom: ' + val);
  },
  getVal: function($el) {
    return $el.val().replace('custom: ', '');
  }
});

Would this work for you @trabianmatt? It's still rough, but I really like the flexibility and customizability this will give Stickit. Recently, @andriijas and @mehcode asked to extend Stickit with button groups and img handling - this addition would provide an immediate solution for these feature requests (although, I'm still thinking about adding them to the core handling).

[trabianmatt]

This is the direction I was initially heading but thought it might be beyond the scope of what you'd consider. I like the direction you proposed -- up for some help?

[delambo]

I would love some help, but first, let's iron-out some of the details.

I would like to nitpick addObservable since observing is mostly associated with model attributes. I think addDirective might be a better name and it would be nice if addDirective accepted a single directive/object configuration or a collection of directives.

Directives should have the same api configuration as bindings and should be extended by binding configurations, where binding configuration keys override. To achieve this and for consistency, @mehcode had two suggestions in #41 which I think we should add into this pull. Since we are using events in this context, we should deprecate eventsOverride in the bindings api for events. Also, we will need to add update to the bindings api. An example of how bindings can override a directive:

// Add directive for <img> tags - bind the `src` attribute to the model attribute value.
Backbone.Stickit.addDirective({
  selector: 'img',
  update: function($el, val, config) {
    $el.attr('src', val);
  }
});

// ... in some Backbone.View ...

bindings: {
  // This binding configuration will extend the <img> directive and override `update`.
  'img': {
    observe: 'href',
    update: function($el, val, attrName, config) {
      // Bad example since this work should be done in `onGet`, but you get the idea...
      $el.attr('src', val.replace('.jpg', '.png'));
    }
  }
}

How should directives be maintained/used? It might be best to use an ordered list so that they can be iterated in order starting with the last-one-in. After finding all of the directives that match a binding selector element, extend them in the order that they were found with the bindings. To demonstrate with underscore.js what this might look like:

var finalBindingConfiguration = _.extend({}, ..., secondDirectiveMatch, firstDirectiveMatch,  bindingConfig);

Lastly, we will (most likely) need to rafactor existing handling, like handling for input[type="text"], into directives so that the code is uniform and based around these configurations.

Still in rough draft mode but I really like where this is headed. This pull is going to involve a lot of refactoring so I would like to get some discussion going. Thoughts?

[trabianmatt]

Looks straightforward, and I think a good starting point would be to refactor the existing handlers to use this structure.

What do you think of addHandler instead of addDirective?

Also, and this is completely optional - since we're going to be touching most of the code with this, would you be up for changing the coding style to be more consistent with Backbone itself? 2 spaces instead of tabs for indentation is the first thing that jumps out at me.

[delambo]

Yes - I would like to start using 2 spaces - I'm a recent convert. handler would be a second choice for me, but we can debate/change that later.

[delambo]

Closing this and starting with a clean slate in #65. Thanks again @trabianmatt.