Skip to content
This repository has been archived by the owner on Jun 15, 2023. It is now read-only.

Wrap delegate and undelegate #852

Merged
merged 3 commits into from
Oct 13, 2015
Merged

Wrap delegate and undelegate #852

merged 3 commits into from
Oct 13, 2015

Conversation

84564221
Copy link
Contributor

See #851

Tested with:

  • chaplin 1.0.1
  • exoskeleton 0.7.0
  • Zepto 1.1.6
  • lodash 3.5.0

Example: https://github.com/opendradio/app

Matej Kieres added 3 commits March 23, 2015 21:03
Replace `Backbone.utilsBackbone.utils` with `Backbone.utils`
@84564221
Copy link
Contributor Author

I was blind in one eye for many hours. However, the travis build of the pull request has passed.

@@ -194,7 +194,7 @@ module.exports = class View extends Backbone.View
# e.g.
# @delegate('click', 'button.confirm', @confirm)
delegate: (eventName, second, third) ->
if Backbone.utils
if Backbone.utils and Backbone.utils?.delegate
Copy link
Contributor

Choose a reason for hiding this comment

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

This is doing the same thing as it already was.

I would move this change to the next line:

if Backbone.utils
  return Backbone.utils.delegate?(this, eventName, second, third)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Conclusion

Without determining whether Backbone.utils exists and Backbone.utils.delegate/Backbone.utils.undelegate are functions, any call of Chaplin.delegate or Chaplin.undelegate results in an error for me:

TypeError: Backbone.utils.undelegate is not a function

or

TypeError: Backbone.utils.undelegate is not a function

The Backbone.utils object exists - but without delegate and undelegate functions.

Please check the source: https://github.com/paulmillr/exoskeleton/blob/master/lib/utils.js

The latest release of exoskeleton does not include these functions (possibly deprecated?).

As already mentioned above, I want to use Chaplin with the latest release of exoskeleton. This could be a possible fix for paulmillr/exoskeleton#87, isn't it?

The solution

And as far I understand, Chaplin seems to have its own, native delegate and undelegate functions:

View.delegate: chaplin/src/chaplin/views/view.coffee#L202-L225
View.undelegate: chaplin/src/chaplin/views/view.coffee#L260-L276

Please correct me, if I am wrong.

Copy link
Contributor

Choose a reason for hiding this comment

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

The latest release of exoskeleton does not include these functions (possibly deprecated?).

I know, I'm the one who removed them :)

The right answer is to fix Chaplin to work with NativeView and not rely on utils.delegate / utils.undelegate. I started #780 last year but I haven't had a project that needed Chaplin in a long time and so I haven't had time to dedicate to fix all the edge cases. Feel free to contribute to that pull, or just use Exoskeleton 0.6.x which should work with Chaplin.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, I need several bug fixes from 0.7.0 - I can not downgrade.

Copy link
Contributor

Choose a reason for hiding this comment

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

Mind telling me what they are? We could maybe do a 0.6.4.

You might also want to try just using regular old Backbone. Why do you need Exoskeleton?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will stick to your recommendation. I want to provide a patch for NativeViews, since my app is based on Chaplin and I like it.

Exoskeleton runs faster on low-memory Firefox OS devices, in comparison to classic Backbone.

paulmillr added a commit that referenced this pull request Oct 13, 2015
Wrap delegate and undelegate
@paulmillr paulmillr merged commit c564041 into chaplinjs:master Oct 13, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants