jQuery < 1.4.3 compatibility #37

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
@swiftyone

Before 1.4.3 data() did not include HTML5 data-* attributes.
Tested with jQuery 1.4.2

Added jQuery < 1.4.3 compatibility
Before 1.4.3 data() did not include HTML5 data-* attributes.
Tested with jQuery 1.4.2
@tuupola

This comment has been minimized.

Show comment
Hide comment
@tuupola

tuupola May 30, 2012

Owner

1.4.2 is over two years old. Are there still some legacy systems which force using old jQuery?

Owner

tuupola commented May 30, 2012

1.4.2 is over two years old. Are there still some legacy systems which force using old jQuery?

@swiftyone

This comment has been minimized.

Show comment
Hide comment
@swiftyone

swiftyone Jun 1, 2012

Not sure.
I'm currently working on a project that started in 2010 and have to use 1.4.2 for compatibility reasons..
But still, using attr() is significantly faster than data(): (http://jsperf.com/jq-attr-vs-data) and the use of attr() is sufficient in this case, as the additional features (as parsing) are not needed.

Not sure.
I'm currently working on a project that started in 2010 and have to use 1.4.2 for compatibility reasons..
But still, using attr() is significantly faster than data(): (http://jsperf.com/jq-attr-vs-data) and the use of attr() is sufficient in this case, as the additional features (as parsing) are not needed.

@TMcKinley

This comment has been minimized.

Show comment
Hide comment
@TMcKinley

TMcKinley Aug 21, 2012

I did this change as well before I saw this Pull Request. Good to know that I'm not crazy. :-/

I did this change as well before I saw this Pull Request. Good to know that I'm not crazy. :-/

@gillyb

This comment has been minimized.

Show comment
Hide comment
@gillyb

gillyb Jan 10, 2013

I would also like to see this change, since i am using this plugin, and the system im working on uses jquery 1.4 on some pages, and some pages has the new jquery. (I know it sounds crazy, but it still exists)

@swiftyone - What i don't understand though, is why you kept both implementations in your code commit. If you're using the one you added (using the attr method) then you don't need to check if data exists, since it will work on all jquery versions.

Hence, the code should look like this :
.attr("src", $self.attr('data-' + settings.data_attribute));

btw - I made this change locally, and it works on both the jquery versions we're using. No reason it wouldn't work on all versions.

gillyb commented Jan 10, 2013

I would also like to see this change, since i am using this plugin, and the system im working on uses jquery 1.4 on some pages, and some pages has the new jquery. (I know it sounds crazy, but it still exists)

@swiftyone - What i don't understand though, is why you kept both implementations in your code commit. If you're using the one you added (using the attr method) then you don't need to check if data exists, since it will work on all jquery versions.

Hence, the code should look like this :
.attr("src", $self.attr('data-' + settings.data_attribute));

btw - I made this change locally, and it works on both the jquery versions we're using. No reason it wouldn't work on all versions.

@swiftyone

This comment has been minimized.

Show comment
Hide comment
@swiftyone

swiftyone Jan 10, 2013

@gillyb Yeah, it sure works without the old implementation. But if you keep it, you can also use data which you added via .data() yourself, as those changes are not updated on the DOM node. I just like to keep it, even though I probably will never use it...

@gillyb Yeah, it sure works without the old implementation. But if you keep it, you can also use data which you added via .data() yourself, as those changes are not updated on the DOM node. I just like to keep it, even though I probably will never use it...

@tuupola

This comment has been minimized.

Show comment
Hide comment
@tuupola

tuupola Oct 24, 2013

Owner

Related to #101

Owner

tuupola commented Oct 24, 2013

Related to #101

@tuupola

This comment has been minimized.

Show comment
Hide comment
@tuupola

tuupola Nov 16, 2013

Owner

Modified merged in 1.9.x.

Owner

tuupola commented Nov 16, 2013

Modified merged in 1.9.x.

@tuupola tuupola closed this Nov 16, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment