jamesmacaulay / react-bacon

A little module for using React with Bacon.js
MIT License
118 stars 7 forks source link

Add 'with' function to attach custom props to the event #2

Closed leoasis closed 10 years ago

leoasis commented 10 years ago

This PR adds a 'with' function to the handler fn so that you can set custom properties in the event before dispatching it into the Bacon.Bus. A typical use case for this is when you have a component that is simple enough to have the children rendered inline, and we have a handler in each particular child:

/** @jsx React.DOM */
...
componentWillMount: function() {
  this.eventStream('itemClicks').
    doAction('.preventDefault').
    map('.itemId'). // I have access to the item id that was clicked
    ...
},
render: function() {
  var items = this.props.items.map(this.renderItem);
  return <ul>{items}</ul>
},

renderItem: function(item) {
  return <li><a onClick={ this.itemClicks.with({ itemId: item.id }) }></a></li>
}

I think this is a handy feature, and I've already needed something like this when using this lib, which is awesome by the way.

jamesmacaulay commented 10 years ago

Being able to easily extend React event handlers like you're suggesting seems really useful. However, it also seems like something that would be equally applicable for any event handler on any React component and doesn't necessarily belong only with eventStream handlers in this Bacon.js-specific mixin.

You could achieve a similar thing with a separate wrapper function that you could use with any event handler:

function callWithCustomProperties(func, customEventProps) {
  return function(event) {
    for (var prop in customEventProps) {
      event[prop] = customEventProps[prop];
    }

    return func(event);
  };
};
// <a onClick={ callWithCustomProperties(this.itemClicks, {itemId: item.id}) }></a>

If you want the .with syntax, you could make a React mixin which extends all functions on a component with the .with method.

jamesmacaulay commented 10 years ago

Sorry, I meant to provide a bit more context in that comment. It's a nice idea that I think works well with this mixin, but I'd prefer to keep this library as small and as focused on Bacon-specific issues as possible.

Glad you're finding this mixin useful, and thank you very much for the contributions!

leoasis commented 10 years ago

Yeah you're right this can be achieved without adding extra code to the lib. Actually I like the wrapping a little more, feels more like JavaScript hehe.

In other subject, would you like me to contribute with some tests for the lib? I'm already working on that in my fork.

Glad to contribute, this Mixin is a neat idea!

On Thursday, April 24, 2014, James MacAulay notifications@github.com wrote:

Sorry, I meant to provide a bit more context in that comment. It's a nice idea that I think works well with this mixin, but I'd prefer to keep this library as small and as focused on Bacon-specific issues as possible.

Glad you're finding this mixin useful, and thank you very much for the contributions!

— Reply to this email directly or view it on GitHubhttps://github.com/jamesmacaulay/react-bacon/pull/2#issuecomment-41350564 .

jamesmacaulay commented 10 years ago

Tests would be great, thank you!