Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow custom types to be created and run #22

Merged
merged 4 commits into from
Oct 9, 2014
Merged

Conversation

lukekarrys
Copy link
Contributor

The API for ampersand-dom-bindings would be:

domBindings({
  'model.key': {
    type: 'customType',
    selector: '.thing'
  }
}, {
  customType: function (el, value, previousValue) { /* ... */ }
})

The idea is that ampersand-view could have a bindingTypes hash that would get passed to ampersand-dom-bindings so that they could be used like this:

View.extend({
  bindings: {
    'model.name': {
        type: 'customNameValue',
        hook: 'name'
    }
  },
  bindingTypes: {
    customNameValue: function (el, value, previousValue) { /* ... */ }
  }
});

Closes #21

@HenrikJoreteg
Copy link
Member

Even though I like the flexibility idea of this, i'm not thrilled about extending view in order to support this.

I wonder if we shouldn't do this entirely just by letting users pass a function instead of a string "type"

Then this would be the only thing needed for adding custom binding types:

var View = require('ampersand-view');
var myBindingFunc = require('my-binding-func');


View.extend({
  bindings: {
    'model.name': {
      type: myBindingFunc,
      hook: 'name'
    }
  }
});

@HenrikJoreteg HenrikJoreteg mentioned this pull request Oct 4, 2014
@lukekarrys
Copy link
Contributor Author

@HenrikJoreteg I agree that adding to the view API isn't really necessary here since the example you showed is pretty simple and easy to grasp.

It was already possible with this PR to pass in a function as the type but I removed all the customTypes stuff. So the API is just:

domBindings({
  'model.key': {
    type: function (el, value, previousValue) { /* ... */ },
    selector: '.thing'
  }
});

@@ -174,6 +174,13 @@ function getBindingFunc(binding) {
});
}
};
} else if (typeof type === 'function') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the minor feedback i mentioned is i'd rather we determine at the top while we're doing the "parsing" anyway whether or not this is a custom binding and if so applying it. Rather than doing that as the last check since we're comparing all the others against strings.

basically determininng if it's a function or string first.

lukekarrys added a commit that referenced this pull request Oct 9, 2014
allow custom types to be created and run
@lukekarrys lukekarrys merged commit 0fc7196 into master Oct 9, 2014
@lukekarrys lukekarrys deleted the custom-bindings branch October 9, 2014 17:41
@elainemacurdy
Copy link

Hi there, quick follow up to this. Any chance we could get the binding context passed through to the custom binding function too? I need to do something like the following:

var bindingToggleFormField = function(el, value, previousValue, bindingContext) {
  var fieldName = el.getAttribute('data-hook');
  if (this._fieldViews[fieldName]) {
    var newValue = value && bindingContext.showWhen;
    this._fieldViews[fieldName].set('isVisible', newValue);
  }
}

bindings: {
  isFoo: {
    type: bindingToggleFormField,
    hook: 'barField',
    showWhen: [true || false]
  }
}

props: {
  isFoo: 'boolean'
}

I'm sure there's a different way to implement it, this just occurred to me as pretty elegant if it could be supported.

Thanks!
Elaine

@bear
Copy link
Contributor

bear commented Mar 25, 2015

@elainemacurdy you may want to file a seperate issue for your question - it was made on a closed PR and that will reduce it's visibility

@elainemacurdy
Copy link

Oh fair enough, sorry I missed that it was closed. I'll do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom binding types
4 participants