Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the use of jQuery's is() function for jQuery < 1.6 support. #1555

Closed
wants to merge 3 commits into from

Conversation

rooby
Copy link

@rooby rooby commented Sep 16, 2013

This change comes from HitmanInWis's issue at #1471

It removes the use of the jQuery is() function, which is different in jQuery < 1.6 so that a wider range of jQuery versions are support.

@@ -165,7 +165,8 @@ class Chosen extends AbstractChosen


test_active_click: (evt) ->
if @container.is($(evt.target).closest('.chosen-container'))
chosenContainer = $(evt.target).closest('.chosen-container')
if chosenContainer.size() > 0 and @container[0] == chosenContainer[0]
Copy link
Collaborator

Choose a reason for hiding this comment

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

.size() is deprecated. you sould be using chosenContainer.length

@alexhass
Copy link

Are you able to create a new release with this fix included, please?

Running newer jquery versions in Drupal 7 is really a mess and a Chosen version that works with jQuery 1.4.4 would be great.

@rooby
Copy link
Author

rooby commented Oct 17, 2013

@stof
I have changed size() to length now.

@koenpunt
Copy link
Collaborator

You can leave the > 0, as a value greater than zero is true'ish

@rooby
Copy link
Author

rooby commented Oct 18, 2013

@koenpunt
The > 0 is still there, or do you mean remove it? (I wasn't really paying much attention when I made the fix, it probably shouold omit the > 0)

@koenpunt
Copy link
Collaborator

Oh, typed too fast, should have been 'leave out' ;)

@rooby
Copy link
Author

rooby commented Oct 22, 2013

Committed that change. It should be good now.

@koenpunt
Copy link
Collaborator

If you squash it into one commit I think it should be good to go. What about you guys, @harvesthq/chosen-developers?

@stof
Copy link
Collaborator

stof commented Oct 22, 2013

it looks good, yes

@tjschuck
Copy link
Member

tjschuck commented Dec 2, 2013

@rooby Any chance you can rebase this against master and squash it into one commit? Instructions here.

Thanks!

adamdicarlo added a commit to opensourcery/chosen that referenced this pull request Dec 26, 2013
Adapted from rooby's pull request for issue harvesthq#1555.
@pfiller
Copy link
Contributor

pfiller commented Dec 28, 2013

Thanks, @rooby. Closing in favor of #1702 which condenses this to one commit and makes a nice variable name change.

@pfiller pfiller closed this Dec 28, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants