RyanMullins / angular-hammer

Hammer.js v2 support for AngularJS
http://ryanmullins.github.io/angular-hammer/
MIT License
188 stars 55 forks source link

Don't depend on window.Hammer #8

Closed lookfirst closed 9 years ago

lookfirst commented 9 years ago

I use requirejs modules to load hammer. Therefore, hammer does not initialize itself into window.Hammer. Your code depends on window.Hammer. I am sad. :cry:

RyanMullins commented 9 years ago

I haven't done much with RequireJS. If I just use a reference to Hammer will that address your issue?

lookfirst commented 9 years ago

You can't depend on window.Hammer being there, it might be defined as a module. Take a look at how Hammer is setting itself up in the context of a loader and then look for the Hammer object in those places too.

lookfirst commented 9 years ago

Mind tagging 2.1.1 so that it shows up in bower? =)

RyanMullins commented 9 years ago

Did I not push the tags... whoops. Should be there now.

lookfirst commented 9 years ago

Thanks for pushing the tag. Unfortunately, your change doesn't work properly. If there is no Hammer on line 330 (as in my case), then javascript fails with 'Hammer is not defined'. You're also assuming that require() is also defined, so that will fail for others who use your project without require().

(Hammer || require('Hammer') || window.Hammer)

lookfirst commented 9 years ago

The same issue exists for the line above with angular.

RyanMullins commented 9 years ago

Suggestions?

lookfirst commented 9 years ago

I think you're just going to have to do a bit more work to determine if these things exist or not. I'd start with passing in the strings 'angular' and 'Hammer' and then doing a similar dance to what is at the bottom of the hammer.js library to actually resolve the locations of these objects.

RyanMullins commented 9 years ago

Yeah... I'll add an issue for v2.1.2.

lookfirst commented 9 years ago

if (typeof Hammer !== 'undefined')...

lookfirst commented 9 years ago

Sorry it is such a pain... damn javascript. ;-)

RyanMullins commented 9 years ago

Sometimes I really think they just didn't think anything through.

RyanMullins commented 9 years ago

Check this out. I think it fixes this issue, thought I don't have any RequireJS projects to test on.

https://github.com/RyanMullins/angular-hammer/commit/42d21b093b9ee5bea26e94a023b0c71cc22eaa32

Give it a try and if it works I'll push it as v2.1.2

lookfirst commented 9 years ago

https://github.com/RyanMullins/angular-hammer/pull/13