Rails Ruby on Rails

Comments and changes to this ticket

• You flagged this item as spam.
  

  Mislav August 11th, 2010 @ 12:02 PM

  Hey Andrew,

  It's a good idea to try to solve this, but I don't agree this is a good approach.

  Prototype form serialization is done by the Form#serializeElements method, which—as you noticed—by default includes the first submit button because it can't know which one was pressed. However, it supports an optional argument submit with which you can specify the name of the submit button you want to include instead of the first button.

  So, in your case we would want the serialize method in rails.js to get invoked like this:

  params = element.serialize({ submit: 'delete' });
  

  Now the problem remains how do we know which submit button was pressed? There are 3 ways a user can trigger form submit that I can think of:

  1. hitting the Return key while in a text input element: unless it was a TEXTAREA, this submits the form as if the first submit button was pressed. because Prototype already serializes by this, we're OK.
     
  2. pressing the submit button with the mouse or finger: a mouseup or touch event on a button means it got pressed.
     
  3. activating the submit button with keyboard: a keypress of the Return (and possibly Spacebar?) key means the currently focused submit button got pressed.

  My suggestion is to observe all these events on submit buttons which can trigger for submit, attach a piece of metadata on the form with Element#store indicating the name of the button, then use that information when calling serialize:

  params = element.serialize({ submit: element.retrieve('rails-pressed-button') });
  

• You flagged this item as spam.
  

  Andrew Kaspick August 11th, 2010 @ 03:37 PM

  Great feedback. Some bits of prototype I wasn't aware of there. I'll rework this with your suggestions as soon as I can. Thanks.

• You flagged this item as spam.
  

  Andrew Kaspick August 13th, 2010 @ 07:45 PM

  I've attached a new version.

  In my testing a mouse click, space bar press and enter key press all register the 'click' event. A mouseup event doesn't mean a button was pressed though. I can click anywhere on the page, hold the button down and move it over a button and release it for a mouseup. Either way, the 'click' event appears to be the only reliable event that is triggered for the space bar, enter key and mouse. I've also changed the code to use the 'store'/'retrieve' method in prototype.

  Please let me know if any changes to method names, code style, logic, etc need to be changed.

  • multiple_submit.diff 2.2 KB

• You flagged this item as spam.
  

  Larry Sprock August 13th, 2010 @ 09:16 PM

  Andrew,

  Using your latest patch I am getting nothing returned. Line 23 should be: return submit.name;

• You flagged this item as spam.
  

  Larry Sprock August 13th, 2010 @ 09:27 PM

  Oops 79 not 23, was looking at the patch not the file but just to be clear...

  function whichSubmitButton(form, reset) {
    var submit = form.select('input[type=submit]').detect(function(input){
      return input.retrieve('rails-submit-button') === true;
    });
    if (reset === true) {submit.store('rails-submit-button', null);}
    return submit.name; //added .name to submit
  }
  

• You flagged this item as spam.
  

  Andrew Kaspick August 14th, 2010 @ 01:12 PM

  Thanks Larry. It appeared to be working for me locally, but looking at the code for serializeElements in prototype, it does seem to do a comparison with the 'name' value.

  I've attached a diff with the newest commit. My change is slightly different and does a check for the submit element before returning the name attribute.

  Please review and let me know if things are working ok.

  Thanks

  • multiple_submit.diff 3.1 KB

• You flagged this item as spam.
  

  Mislav August 14th, 2010 @ 03:22 PM

  This wasn't exactly what I had in mind. If you store the name of the submit button on the form rather than on the button itself, you don't need the whichSubmitButton function.

  document.on('click', 'form input[type=submit]', function(event, button) {
    // register the pressed submit button
    event.findElement('form').store('rails:submit-button', button.name || false);
  });
  
  // and then:
  params = element.serialize({ submit: element.retrieve('rails:submit-button') });
  

  The complete solution is much less complex and handles the case when the submit button didn't have a "name" to start with (no submit button value should be serialized in that case).

• You flagged this item as spam.
  

  Andrew Kaspick August 14th, 2010 @ 07:38 PM

  Storing on the form makes much more sense. I'll get another patch together soon. Thanks.

• You flagged this item as spam.
  

  Larry Sprock August 14th, 2010 @ 08:01 PM

  Here's a patch. Put Andrews name on it since he's been doing the work.

• You flagged this item as spam.
  

  Mislav August 14th, 2010 @ 08:40 PM

  • Assigned user set to “josh”

  Indenting looks off, apart from that it's fine. I'd like you guys to test this in browsers that Prototype supports (as well as touch devices if you have any) and report here so whoever commits this to core knows that it's a real solution.

• You flagged this item as spam.
  

  Larry Sprock August 14th, 2010 @ 09:54 PM

  Yeah, funky stuff with tabs in textmate. Format correction attached.

  • multi_submit_v2.patch 1.2 KB

• You flagged this item as spam.
  

  Larry Sprock August 14th, 2010 @ 11:36 PM

  Created a little rails app for testing in different browsers. It just returns the name and value from the button pressed pulled from the params in the controller.

  http://ajax-button-test.heroku.com

  iphone iOS4: working
  safari 5.0.1: working
  firefox 3.6.8: working

  These are all the browsers I could test at the moment.

  Also I had to perform a little hakery in rails.js to get the responseText. Tried using observe ajax:success but was only getting [object Event] returned which was probably fire... what is the best way to get the response from ajax:success using the current implementation of rails.js?

• You flagged this item as spam.
  

  Andrew Kaspick August 15th, 2010 @ 06:18 AM

  I'm away traveling so am a bit staged with getting online. I noticed Larry attached a new patch. I have a new patch as well, but don't do the "|| false" part as that stops any of the "submit" values from being serialized, is that what we are going for if nothing is found then?

  I'll attach my patch as well just in case, but if we're leaving the || false part it then we can just use Larry's latest.

  • multiple_submit_without_false_default.diff 1.8 KB

• You flagged this item as spam.
  

  Mislav August 15th, 2010 @ 11:41 PM

  @Andrew: the || false fallback is what we want if the button doesn't have a name attribute. In this case, { submit: false } will be given to serialize method and no button value should be serialized. I'm pretty sure this is native browser behavior, too.

  @Larry: rails.js fires custom events at all stages of an Ajax request. To access the responseText property, observe the "ajax:success" event and access its memo property:

  document.on("ajax:success", function(e) {
    $('responseText').update(e.memo.responseText)
  })
  

  The memo property on a custom event is any object that the code which fired the event in the first place wished to pass on. In rails.js case, this is the Ajax request object.

• You flagged this item as spam.
  

  Andrew Kaspick August 16th, 2010 @ 09:02 AM

  Fair enough. I just wanted to make sure that's what the plan for the submit button was. Besides that, Larry's patch is what mine would be then too.

• You flagged this item as spam.
  

  Mislav August 16th, 2010 @ 10:06 AM

  Pushed to prototype-ujs, thanks guys.

• You flagged this item as spam.
  

  Andrew Kaspick September 28th, 2010 @ 03:19 AM

  I noticed that the rails.js file that comes with rails 3 is still out of date and doesn't include this fix...

  http://github.com/rails/rails/blob/master/railties/lib/rails/genera...

  Shouldn't the original patch be committed to rails master to reflect this update? Otherwise why would the current rails.js be left as is in master?

• You flagged this item as spam.
  

  Andrew Kaspick September 28th, 2010 @ 03:20 AM

  To avoid any confusion, I didn't mean to say "original" patch there, just meant to say "patch" by itself... the one that was pushed to prototype-ujs.

• You flagged this item as spam.
  

  Mislav September 29th, 2010 @ 06:55 PM

  The prototype-ujs project should be the source of edge rails.js for people that wish to update, not Rails master. I expect prototype-ujs to be updated much more frequently than it would make sense to merge back into Rails.

• You flagged this item as spam.
  

  Andrew Kaspick September 29th, 2010 @ 08:00 PM

  So everything committed to prototype-ujs will be merged for the next rails point release? I just thought it would have been merged for the final rails 3 release as well.

  This ticket might as well be closed now.

  Thanks

• 

  Jeff Kreeftmeijer October 10th, 2010 @ 08:23 PM

  • State changed from “new” to “resolved”
  • Importance changed from “” to “Low”