flightjs / flight

A component-based, event-driven JavaScript framework from Twitter
http://flightjs.github.io/
MIT License
6.53k stars 547 forks source link

Add contextSelector to attachTo #254

Closed gc-victor closed 9 years ago

gc-victor commented 10 years ago

After participate in a discussion at the Flight's Google Group, "Component granulation", I have decided to pull request a possible improvement to attachTo, adding a second argument as contextSelector.

component.js

function attachTo(selector/*, contextSelector, options */) {
  var args, last, options, componentInfo;

  if (!selector) {
    throw new Error('Component needs to be attachTo\'d a jQuery object, native node or selector string');
  }

  args = arguments;
  last = args.length - 1;
  options = $.type(args[last]) === 'object' ? args[last] : {};
  componentInfo = registry.findComponentInfo(this);

  $(selector).each(function(i, node) {
    if (componentInfo && componentInfo.isAttachedTo(node)) {
      return;
    }

    this.prototype.contextSelector = (typeof args[1] === 'string') ?
      args[1] : null;

    (new this).initialize(node, options);
  }.bind(this));
}

base.js

this.select = function(attributeKey) {
  var ctx, len, exist, el;

  len = arguments.length;
  exist = this.attr[arguments[0]] || this.attr[arguments[1]];
  if (len !== 0 && len < 3 && exist) {
    ctx = this.contextSelector || this.node;
    if (len === 1) {
      el = $(ctx).find(this.attr[arguments[0]]);
    } else if (arguments[0].target) {
      el = $(arguments[0].target).closest(ctx)
        .find(this.attr[arguments[1]]);
    } else {
      el = arguments[0].closest(ctx)
        .find(this.attr[arguments[1]]);
    }
  }

  return el || [];
};

The test shows the error: 'should merge multiple options arguments correctly'. The way we are using Flight do not have any sense to use more than one option argument. Does that test validation have sense for someone? Thanks.

tgvashworth commented 9 years ago

Hey @gc-victor — thanks for this, sorry about the delay in getting to it.

Could you link back to the Flight mailing list discussion if it's still relevant? Would like some more context. Thanks!