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

ExtraParameters are swallowed in trigger(click) for checkboxes #4139

Closed
minj opened this Issue Jul 30, 2018 · 3 comments

Comments

Projects
None yet
5 participants
@minj
Copy link

commented Jul 30, 2018

Description

It's impossible to pass extraParameters for checkbox click handlers.

Duplicate of #2768

Source of error:

this.click();

I did not have time to look into the full history of why this was added but it does not seem right to me. If you want to make sure checked state is set consistently, do so explicitly.

Link to test case

https://jsfiddle.net/xpvt214o/509284/

What do you expect to happen?

"Hello, world" to be displayed, using extraParameters.

What actually happens?

ExtraParameters are discarded.

@mgol

This comment has been minimized.

Copy link
Member

commented Jul 30, 2018

Thanks for the report.

For some time it didn't work on radios as well but now it does: It required us to revert a fix for #3423 in PR #3581 which is not ideal but, otherwise, we'd have a breaking change.

There is an old @gibson042's PR that fixes it (#1367) but it's quite large so we were holding off on accepting it. Since we plan a big event refactor in jQuery 4 that will most likely make more use of the native event system, we need to see how feasible passing data is in this new model so that we don't add it only to revert it shortly thereafter.

Am I right in my description of the situation, @dmethvin or @gibson042?

Should we leave this issue open until we figure out what the story for jQuery 4 is? This is getting re-reported a lot compared to other issues so people are really running into this frequently.

@dmethvin

This comment has been minimized.

Copy link
Member

commented Jul 30, 2018

We can document that it doesn't work, or even deprecate and remove it eventually, but I suspect the people using it have discovered some code snippet elsewhere on the internet that uses it. @minj where/how did you learn about the extraParameters feature? Would you be able to use something simple like a closure to pass the data?

This is one of those features that I think will prevent us from leaning more heavily on native event plumbing. To get that second argument there has to be some shim between the native handler and the jQuery one.

@minj

This comment has been minimized.

Copy link
Author

commented Jul 31, 2018

@dmethvin it is in the docs, of course. I would think people always turn to that first when looking for information, no? 😉

One may want to use with a click listener in a completely different JS file, so closure is not always a valid solution.

@timmywil timmywil removed the Needs review label Aug 13, 2018

@timmywil timmywil added this to the 4.0.0 milestone Sep 3, 2018

@gibson042 gibson042 modified the milestones: 4.0.0, 3.4.0 Jan 15, 2019

@gibson042 gibson042 self-assigned this Jan 15, 2019

gibson042 added a commit to gibson042/jquery that referenced this issue Jan 15, 2019

mgol added a commit to gibson042/jquery that referenced this issue Mar 20, 2019

@mgol mgol closed this in 669f720 Mar 20, 2019

@GulajavaMinistudio GulajavaMinistudio referenced this issue Mar 21, 2019

Merged

Bottom sheet scrolling #79

0 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.