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

Dropped change() support #247

Closed
softshape opened this issue Oct 9, 2014 · 5 comments
Closed

Dropped change() support #247

softshape opened this issue Oct 9, 2014 · 5 comments

Comments

@softshape
Copy link

The latest Dropkick version dropped element.change(...) support. It used to work and I rely on it in production. For now the only documented way to process onChange event for the select is to call dropkick constructor with 'change' parameter.

On my site I style all the selects by default -

if($("select").length) {
$("select").dropkick();
}

So calling dropkick() again looks strange, especially as 'this' is pointed to different object. Should I rewrite all the select onchange handlers in my code? Or you plan to restore onchange support?

@softshape
Copy link
Author

Moreover, it's impossible now to call element.change() from JS. Well it's possible but useless.

@Korri
Copy link
Contributor

Korri commented Nov 6, 2014

Critical issue there, using dropkick should't destroy basic functionalities like that.
For now we fixed it like that:

$('select').dropkick({
    change: function () {
        $(this.data.select).change();
    }
});

@wwilsman
Copy link
Collaborator

wwilsman commented Nov 6, 2014

I couldn't think of a reasonable solution to this since it appeared to be jQuery specific. Upon closer inspection adding an event listener on the original select also did not trigger. Testing revealed that programatic changes to the select do not trigger the 'change' event on said select; so I've updated Dropkick's select method to manually trigger it. Check out the develop branch, It needs to be tested in IE 8 before we merge it into master. From adding that single line, both jQuery and .addEventListener appear to work as expected.

@Robdel12
Copy link
Owner

@softshape @Korri I'm going to close this ticket. If you guys have any other questions/concerns feel free to comment/reopen!

@Korri
Copy link
Contributor

Korri commented Nov 18, 2014

Works perfectly now, thanks !

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

No branches or pull requests

4 participants