Deprecate .toggle( handler, handler, ... ) signature

Reported by:	gibson042	Owned by:
Priority:	low	Milestone:	1.8
Component:	event	Version:	1.7.2
Keywords:		Cc:
Blocked by:		Blocking:

Description

Did you know we have two .toggles? The one we like performs .show or .hide as appropriate, and is documented at http://api.jquery.com/toggle/. The other sets up round-robin .click handlers, and is documented at http://api.jquery.com/toggle-event/:

The .toggle() method is provided for convenience. It is relatively straightforward to implement the same behavior by hand, and this can be necessary if the assumptions built into .toggle() prove limiting. For example, .toggle() is not guaranteed to work correctly if applied twice to the same element. Since .toggle() internally uses a click handler to do its work, we must unbind click to remove a behavior attached with .toggle(), so other click handlers can be caught in the crossfire. The implementation also calls .preventDefault() on the event, so links will not be followed and buttons will not be clicked if .toggle() has been called on the element.

Let's put it out of our misery.

Attachments (0)

Change History (8)

Changed May 19, 2012 02:15PM UTC by dmethvin comment:1

Here are some thoughts:

Against removing:

It's been around a while, and will break code if removed.

For removing:

The name .toggle() is ambiguous, and of the two uses this is by far the more obscure.
All the strange quirks mentioned in the docs.
It doesn't depend on internals and could be a plugin.
Saves 132 bytes gzipped. (That surprised me!)

Changed May 19, 2012 02:26PM UTC by rwaldron comment:2

Dammit Dave! You blitzed me!

Changed June 12, 2012 12:30PM UTC by dmethvin comment:3

component:	unfiled → event
milestone:	None → 1.8
priority:	undecided → low
resolution:	→ fixed
status:	new → closed

As @gibson042 mentions, the documentation is practically a suicide note. Marked for deprecation in the docs, we'll create a new ticket for removal when that happens.

Changed January 13, 2013 09:55AM UTC by mplungjan comment:4

Can someone please guide me to the official deprecation notices?

I understand the need to disambiguate but where are the warnings? Neither in the documentation [here](http://api.jquery.com/toggle-event/) nor [here](http://jquery.com/upgrade-guide/1.9/)

http://stackoverflow.com/questions/14301935/where-has-fn-toggle-handlereventobject-handlereventobject-gone

Changed January 13, 2013 03:09PM UTC by dmethvin comment:5

Unfortunately the "Deprecated" category at api.jquery.com got lost during the transition to a new server in the past couple of weeks. I think kswedberg is working on restoring that.

The missing mention in the upgrade guide is my oversight, I'll put one in now. The jQuery Migrate plugin does have a warning for it, you'll see it described here.

https://github.com/jquery/jquery-migrate/blob/master/warnings.md

Changed January 13, 2013 08:43PM UTC by Mplungjan comment:6

Would it not be great to re-use the name in the Migrate and add it back in as fn.toggler ?

Changed January 14, 2013 02:50AM UTC by dmethvin comment:7

_comment0:	It's not really an appropriate core function. If someone has code that uses is already the Migrate plugin will let them continue to do so without changing any code. The function with another name would still have the drawbacks it does today, including not working with delegation and having no obvious way to remove it. If someone wants the code it's there in the Migrate plugin and can be extracted. → 1358132288807895

_comment0:

It's not really an appropriate core function. If someone has code that uses is already the Migrate plugin will let them continue to do so without changing any code. The function with another name would still have the drawbacks it does today, including not working with delegation and having no obvious way to remove it. If someone wants the code it's there in the Migrate plugin and can be extracted. → 1358132288807895

It's not really an appropriate core function. If someone has code that uses it already the Migrate plugin will let them continue to do so without changing any code. The function with another name would still have the drawbacks it does today, including not working with delegation and having no obvious way to remove it. If someone wants the code it's there in the Migrate plugin and can be extracted.

Changed April 21, 2013 04:58PM UTC by cougar.b@gmail.com comment:8

Just a comment from a newbie. I was tearing my hair out for hours and hours, comparing code that worked with that which didn't and finding no difference, before cluing in to the idea that I had upgraded my requirejs, and that this might be the problem. After eventually finding that toggle had been deprecated and removed, I searched the internet for a fix, and nothing worked. So I read your change log, clicked on this ticket number, and there, in comment #5, is a reference to the Migrate plugin. But the Github link was unhelpful.

I searched jQuery and found the Migrate plugin link, which provided me with a fix: adding one line that referenced the Migrate plugin. But I still don't know what a direct path to this knowledge would have been. Maybe I'm just too much of a newbie, but my opinion is that if you deprecate and remove things that you know will break code, there should be more obvious links to Migrate. All in all, this cost me 15 hours and six gray hairs--at least on the top of my head...

I now know that when I eventually replace the offending code with a module, I'm going to have to completely rewrite the functionality, and I can do that in a sandbox, rather than breaking things. It would have been nice to have more help with preventing this problem in the first place.

Side navigation

#11786 closed enhancement (fixed)