no_double_submit_jquery.coffee

  $('#quick_entry form').live 'submit', (event) ->
    event.preventDefault()  # just moved up!
    event.stopPropagation() # just moved up!
    return false if @beenSubmitted
    @beenSubmitted = true
    clearNotice()
    $.ajax({
      type: 'POST'
      url: @action
      dataType: 'json'
      data: $(@).serialize()
      success: (response) ->
        setNotice(response.bill)
        resetAndFocusForm()
      error: (response) ->
        if response.status == 422
          setErrorFields JSON.parse(response.responseText)['error_fields']
        else
          alert("An error occurred!")
      complete: =>
        @beenSubmitted = false
    })

why @beenSubmitted? I guess to avoid multiple submit but I think it's better to disable submit button instead no?

I need to avoid double submissions, either with enter or click on the form; I read somewhere that just disabling the submit button wouldn't always handle the enter key.

Would you advise doing it another way?

Also I hear the standalone @ is being removed from future versions of cs. 'this' may be more expressive anyway, but that is just an opinion.

@PlasticLizard I wasn't aware of that - thanks for the notice! I'm personally really ok with both, but prefer to use the one that will stay :)

Just a return false in the end should be enough to replace event.preventDefault() & event.stopPropagation(). I think it is a much cleaner approach too.

@druvasagar actually I moved both at the very beginning after your comment, following the advice provided here.

I would do it that way in case any of my code before the final return false raises an error.

Would you have any docs/pointers illustrating the good points of using return false at the end?

I dont use event.preventDefault()and event.stopPropagation()

just return false

About disabling submit which is best UX behavior I think, it requires a method for that depending on what you wanna do. Display a spinner, disable button, change text, ...)

@sgruhier I will as well disable submit, as long as enter is disabled also; I guess I'll mix different approaches.

Here's the final workflow I think I'll use: disable submit, disable enter on any field (including on IE), show a spinner but only after 1sec, in case of network delay, as advised here).

Thanks for the feedback everyone!

one more thing I'm not fan of live, I prefer delegate. Just a detail!

@sgruhier thanks, I wasn't really aware of the differences; I will use delegate. Here's an http://www.alfajango.com/blog/the-difference-between-jquerys-bind-live-and-delegate/ to illustrate the differences, in case someone else is interested.

My point on return false: http://stackoverflow.com/questions/1357118/javascript-event-preventdefault-vs-return-false/4771084#4771084

Yea I was reading through it, I think this - http://stackoverflow.com/questions/5873221/jquery-event-bubbling-and-how-click-live-click-stoppropagation-and-ret also demonstrates the same reasonably well.

@dhruvasagar thanks for the link!

Could you rewrite lines 4-7 from

if @beenSubmitted
  return false
else
  @beenSubmitted = true
... rest of the code...

to

return false if @beenSubmitted

@beenSubmitted = true
... rest of the code...

@alexrothenberg very good point, thanks

As other commenters have suggested: Just put false at the end rather than putting event.preventDefault() and event.stopPropagation() at the beginning.

You said you were concerned that if an exception is thrown, the return false won't be reached. But unless clearNotice might throw an exception, there's no chance of that; any error that occurs in $.ajax and the callbacks you give it will happen asynchronously—after the function has already returned.