search

brandonaaron / jquery-cssHooks

Collection of cssHooks that work with jQuery 1.4.3+
Other
478 stars 80 forks source link

Do we need to check that $.cssHooks exists #19

Closed tomgrohl closed 13 years ago

tomgrohl commented 13 years ago

In the cssHooks I wrote I check that the browsers supports the feature to be used ( border radius, border image..), but I also check to see if $.cssHooks exists to avoid errors when trying to use it with older versions of jQuery.

Wasn't sure if this was needed though, any thoughts?

louisremi commented 13 years ago

This test won't save much processing to the browser and with or without the test, the hook will simply not work. But it won't break anything. So it's not really needed.

tomgrohl commented 13 years ago

I thought that but a test I did gives me an error saying that cssHooks is undefined ( in firefox and chrome) and object expected ( in IE ).

By the looks of it, it may be best to use them, that way it would avoid throwing any errors.

louisremi commented 13 years ago

Oh, of course. On the other hand this can be a good clue that something is wrong with the jQuery version...

louisremi commented 13 years ago

Instead of failing silently you will see that cssHooks is missing somewhere, I mean.

tomgrohl commented 13 years ago

Ah ok. Maybe a check is need before hand then. Something like:

if( !$.cssHooks ) { $.error("The version of jQuery you are using doesn't support cssHooks"); }

Might be good to have in.

pdokas commented 13 years ago

To prevent the error of assigning to a property of undefined I'd say that yeah, the check should be performed.

louisremi commented 13 years ago

It is not a common practice to do this kind of things in jQuery plugins. You should rather indicate in the source code of your file that the plugin depends on jQuery 1.4.3+, see skeleton.js for an example: https://github.com/brandonaaron/jquery-cssHooks/blob/master/skeleton.js

An error will be thrown anyway if jQuery.cssHooks is not defined, and it should not take long for the developer to find out what the problem is.

tomgrohl commented 13 years ago

Yeah your right. Developers would be used to seeing that kind of error so they should be able to find out what the problem is with ease.

I'll update my cssHooks and remove this check and add comments.