ded / reqwest

browser asynchronous http requests
2.94k stars 343 forks source link

Populate ajax request from Form node #60

Closed pier-oliviert closed 12 years ago

pier-oliviert commented 12 years ago

I use qwery to get the form node, I would like to pass it the node and reqwest to create a default ajax reqwest based on all the information provided by the form.

If you want it in and can't find time for it, I can create a pull request for it.

ded commented 12 years ago

there's already a $(form).serialize() which you can pass to the request as the data

pier-oliviert commented 12 years ago

Well, $(form).serialize() only serialize your parameter. What I mean is that it uses all the parameter from form

<form action="/post" method="POST">
<!-- form elements //-->
</form>
reqwest(form); //Equal to

reqwest({
  url: "/post",
  method: "post",
  data: $(form).serialize() // form elements serialized
});

That would be helpful to reduce duplication of code.

rvagg commented 12 years ago

Like Prototype's Form.request(). Very handy, I've used that a lot. http://api.prototypejs.org/dom/Form/request/ Why don't you see if you can code it up (with tests) if it's not too much work and submit a PR for @ded to consider (unless he wants to rule it out now). I reckon if it doesn't add much bulk to Reqwest then it'd be a nice addition. It'd need an optional options parameter to be helpful though so you can at least watch success and error.

pier-oliviert commented 12 years ago

Yeah, I've got an idea of how I would build it for this specific project, but before getting started I wanted to test the water to see if it's something @ded would accept as a pull request.

I'll probably come up with something tonight or tomorrow and we'll work from there ;)

ded commented 12 years ago

you could always just add this to your app...

reqwest.form = function (f) {
  reqwest({
    url: "/post",
    method: "post",
    data: $(f).serialize()
  })
}
rvagg commented 12 years ago

Ah, but you're missing out on the best bit about passing in a form: you can read the method & action attributes, which of course means you can bind URLs to your HTML rather than your JS. So, you'd have something along these lines at the top of init():

if (o.nodeType && (/form/i).test(o.nodeName)) {
  o = {
      url: o.action || window.location.href
    , method: o.method || 'GET'
    , data: serializeQueryString(o)
  }
}

but you'd also want to be able to read the second parameter as an options object to override any of those or add new options, or a callback for success.

Then in Ender you can bind ajax to boosh and you get to do:

$('#form').ajax(function () { $('#success').text('yee haw!') })
// or
$('#form').ajax({
    headers: {
      'X-My-Custom-Header': 'SomethingImportant'
    }
  , error: function (err) { }
  , success: function (res) { }
})

The only issue would be what to do with non-forms, you could throw if you try to call without a form, or you could just test for nodeType and not bother with the nodeName test and you'd end up with defaults for url and method and serializeQueryString() will blindly handle non-forms and serialize anything that can be serialized (the element and descendent elements).

So, $('input[type="text"]').ajax() would still work but it'd default to a GET and will use the current window.location.href which could make sense (note that in this case you'd want the Ender bridge to pass in all elements to Reqwest, not just the first, so Reqwest would have to be able to handle collections).

Whaddya reckon? I'm excited!

pier-oliviert commented 12 years ago

This is pretty much what I had in mind! What I was thinking was to pass an hash to set/overwrite whatever settings that is set in the form.

$("#form").ajax({
url: "http://google.com", //would overwrite the url set by form
success: function(res) { //success},
error: function(err) { //error}
})

I think first of all, we should just add collections since it would be the easiest to implement. From there, if supports for other alternatives is shown, we could add convenience constructor (Only pass a success function like your example)

Also, I was going to work on this issue this morning, and tried to run the test.

But I get a failure, since I'm not a real expert in the node.js universe, it could be a mistake I made. @rvagg would you have any idea why this is happening?

Thank you :)