Skip to content

Added note for Global Ajax Event Handlers#797

Closed
AurelioDeRosa wants to merge 3 commits into
jquery:masterfrom
AurelioDeRosa:global-ajax
Closed

Added note for Global Ajax Event Handlers#797
AurelioDeRosa wants to merge 3 commits into
jquery:masterfrom
AurelioDeRosa:global-ajax

Conversation

@AurelioDeRosa
Copy link
Copy Markdown
Member

Fixes gh-479

Comment thread notes.xsl Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it 1.9 or 1.8? Descriptions currently say 1.8.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

In 1.8 you should, in 1.9 you must. Is that clear enough?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah, I see. I think what you have is fine then. We typically use "As of jQuery x.x" instead of "Starting with x.x." Would you mind changing that phrase?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Updated based on your suggestion :)

@kswedberg
Copy link
Copy Markdown
Member

Thanks @AurelioDeRosa. Are you going to also remove the notes that currently appear in the <longdesc>? (for example, "As of jQuery 1.8, the .ajaxStop() method should only be attached to document.")

@AurelioDeRosa
Copy link
Copy Markdown
Member Author

I think your comment is related to #798. Can you check that issue I raised?

@kswedberg
Copy link
Copy Markdown
Member

Yeah, that's what I saw. What I'm suggesting is, if you're adding the "global" note to each method, you should also remove the "local" note.

@AurelioDeRosa
Copy link
Copy Markdown
Member Author

In this case, should I highlight that from 1.8 is recommended, but from 1.9 is mandatory?

@kswedberg
Copy link
Copy Markdown
Member

Never mind. I (finally) see what you're getting at. Maybe just change the global note from "must" to "must" (using <em></em>).

If you do that, then it LGTM, so feel free to merge it..

@AurelioDeRosa
Copy link
Copy Markdown
Member Author

@kswedberg updated based on your comment :)

@kswedberg
Copy link
Copy Markdown
Member

Excellent. Thanks, @AurelioDeRosa ! Wanna merge it in?

@AurelioDeRosa
Copy link
Copy Markdown
Member Author

@kswedberg Done!

@AurelioDeRosa AurelioDeRosa deleted the global-ajax branch August 20, 2015 22:33
@kswedberg
Copy link
Copy Markdown
Member

Thanks, @AurelioDeRosa !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants