search

clayallsopp / react.backbone

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

Allow use of BackboneMixin without arguments; fix controlled components and changing models #18

Closed EamonNerbonne closed 10 years ago

EamonNerbonne commented 10 years ago

This PR builds on @markijbema's https://github.com/usepropeller/react.backbone/pull/15.

It fixes two subtle bugs:

  1. When the model was changed due to a prop change, the event handlers were not rebound properly. This PR rebinds them correctly.
  2. When the model was updated, due to the debounce, controlled components would lose cursor positoin. This PR avoids the debounce for models, and only debounces collections. Note that this still isn't an ideal situation - really we'd want to fix this on the React side of things, such that forceUpdates aren't immediate, because it's just wasteful to re-render all the time (and this isn't the only way to cause re-renders).

It also supports using BackboneMixin without arguments.

markijbema commented 10 years ago

:+1:

clayallsopp commented 10 years ago

This is great - @markijbema did you ever revisit this? https://github.com/usepropeller/react.backbone/pull/15#issuecomment-39122480

markijbema commented 10 years ago

No sorry, didn't get around to doing that yet.

EamonNerbonne commented 10 years ago

However, I just did :-)

Behavior is slightly tricky (backwards compat often is, alas): change options are now those customized if available, otherwise the component (spec's) changeOptions, otherwise a default based on whether the model is a collection or just a model.

markijbema commented 10 years ago

:+1: this was exactly what I wanted to do, thanks @EamonNerbonne !

clayallsopp commented 10 years ago

It is done! Thank you both very much for all the work, apologies for the extraordinarily long delay :clap: