-
Notifications
You must be signed in to change notification settings - Fork 264
delegate: Document the method was deprecated in jQuery 3 #948
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
Conversation
What about undelegate, bind & unbind? |
I assumed it was missing from delegate only because of the issue. I'm going to update the PR. |
@@ -34,7 +34,7 @@ | |||
</signature> | |||
<desc>Attach a handler to an event for the elements.</desc> | |||
<longdesc> | |||
<p>As of jQuery 1.7, the <a href="/on/"><code>.on()</code></a> method is the preferred method for attaching event handlers to a document. For earlier versions, the <code>.bind()</code> method is used for attaching an event handler directly to elements. Handlers are attached to the currently selected elements in the jQuery object, so those elements <em>must exist</em> at the point the call to <code>.bind()</code> occurs. For more flexible event binding, see the discussion of event delegation in <a href="/on/"><code>.on()</code></a> or <a href="/delegate/"><code>.delegate()</code></a>.</p> | |||
<p>As of jQuery 3.0, <code>.bind()</code> has been deprecated. It was superseded by the <a href="/on/"><code>.on()</code></a> method for attaching event handlers to a document since jQuery 1.7, so its use was already discouraged. For earlier versions, the <code>.bind()</code> method is used for attaching an event handler directly to elements. Handlers are attached to the currently selected elements in the jQuery object, so those elements <em>must exist</em> at the point the call to <code>.bind()</code> occurs. For more flexible event binding, see the discussion of event delegation in <a href="/on/"><code>.on()</code></a> or <a href="/delegate/"><code>.delegate()</code></a>.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't direct people to docs for deprecated APIs so I'd remove the reference to https://api.jquery.com/delegate/.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method is available anyway, so removing the link only add frustration in having to open a new tab, searching for the method, and then read the doc. We've already stated the correct method to use is .on()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But why would anyone search for docs for this method? Most likely because they use .delegate()
which can't be said about all the readers of the .bind()
docs.
I think we shouldn't link to pages about deprecated APIs, they should serve first as a warning to not use the API and second as docs for the API for people that already use it in their code. If there is useful info about event delegation there that's not available on the .on()
page it should just be moved to the .on()
page so that it's not required to look at pages for deprecated APIs to learn something generally useful.
(to clarify, I wanted to have the .delegate()
mention removed completely, not just the link to the page)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AurelioDeRosa Hey, do you have any remarks to my comment? For me this is the only thing left before this can be landed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AurelioDeRosa Ping? We need this PR to land to unbreak the build.
@jquery/core @jquery/content any opinion on my comments? I need someone to agree with either of us so that we can land it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main reason someone might search specifically for .delegate()
would be to figure out how to convert a call to .on()
. That wouldn't apply for this reference. Any explanation of delegated events in .delegate()
should be duplicated in .on()
, I think it already should be there but if not we can open a new ticket to fix it.
If #971 can be addressed adequately (e.g. via a watermark) there wouldn't be any doubt about the method being deprecated and/or removed. As I mentioned in #972 I think there might be good reasons to keep, for example, the click
-specific information in the .click()
method docs, but that doesn't apply here either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dmethvin As I understand you, you agree with me it's best not to link to .delegate()
here? I'll go ahead & remove the link and land it then so that we have an unbroken build.
A few minor remarks, otherwise it's good. |
Fixes gh-945