- Notifications
You must be signed in to change notification settings - Fork20.6k
ajax: post and get accept config object [issue #1986]#1995
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
I love it when someone delivers. 🌟 |
| url: url, | ||
| type: method, | ||
| dataType: type, | ||
| data: data, | ||
| success: callback | ||
| }); | ||
| }, (jQuery.isPlainObject(url) && url))); |
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.
Should be spaces inside the parens here per our style guide.
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.
Trying to find it in the style guide...you mean it should be( jQuery.isPlainObject(url) && url )))?
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.
Although this allows a mixed call like$.ajax({ options }, data) I think we'd only want to document a singleoptions object.
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.
( jQuery.isPlainObject( url ) && url ) ) )
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 outer parens aren't needed tho, right?
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.
No I don't think they're needed, just make it clearer that its a single expression. I'll remove. Agree about only documenting a singleoptions object
@togakangaroo Be sure that you've signed our CLA,https://contribute.jquery.org/CLA/ |
@dmethvin I signed it, thanks |
I'm not super sure where/how to update the docs btw. Really mostly where |
| @@ -2034,6 +2034,25 @@ module( "ajax", { | |||
| }); | |||
| }); | |||
| asyncTest( "jQuery[get|post]( config ) - simple with xml", 2, function() { | |||
| jQuery.when.apply(jQuery, | |||
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 same remark about spaces; there should be a space after(,[ and before),] etc.
I createdjquery/api.jquery.com#620 , The docs are athttp://github.com/jquery/api.jquery.com/#readme, they're processed from XML so you'll want to go through the setup listed there or pull down the preconfigured VM so that you can be sure the markup looks good. |
Seethis issue for more discussion.