clayallsopp / react.backbone

Plugin for React to make Backbone migration easier
MIT License
839 stars 60 forks source link

added details to multiple models example #25

Closed bitmage closed 10 years ago

bitmage commented 10 years ago

I added some details in the README for the multiple models example. I wasn't sure how this worked at first, and had to read the code to find out, so I figured some clarification might help.

On a tangent, have you thought about a syntax like this?

var CommentView = React.createBackboneClass({
    expectModels: {
        user: "change:name",
        comment: null
        modelName: {opt1: 'bar', opt2: 'baz'} // maybe?  if additional params are needed
    }
    //, ...
});

Internally it could still wire up the mixins. I don't know if it fits your style/design goals, but just figured I would throw it out there.

-Brandon

clayallsopp commented 10 years ago

Thanks for the clarifications!

Hm I think we usually err on encouraging use of multiple mixins instead of adding another API, so probably aren't looking for a change like that

markijbema commented 10 years ago

Thanks for the PR! It's really great getting PR's on documentation. When you write code it's hard to miss what isn't clear, so it's really great getting feedback, and even better getting PRs :)

bitmage commented 10 years ago

No prob!

On Jul 12, 2014, at 1:28 AM, Mark IJbema notifications@github.com wrote:

Thanks for the PR! It's really great getting PR's on documentation. When you write code it's hard to miss what isn't clear, so it's really great getting feedback, and even better getting PRs :)

— Reply to this email directly or view it on GitHub.