Skip to content

Instantly share code, notes, and snippets.

@thbar
Created September 14, 2011 08:53
Show Gist options
  • Save thbar/1216136 to your computer and use it in GitHub Desktop.
Save thbar/1216136 to your computer and use it in GitHub Desktop.
$('#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
})
@PlasticLizard
Copy link

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.

@thbar
Copy link
Author

thbar commented Sep 14, 2011

@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 :)

@dhruvasagar
Copy link

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.

@thbar
Copy link
Author

thbar commented Sep 14, 2011

@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?

@sgruhier
Copy link

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, ...)

@thbar
Copy link
Author

thbar commented Sep 14, 2011

@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!

@sgruhier
Copy link

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

@thbar
Copy link
Author

thbar commented Sep 14, 2011

@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.

@thbar
Copy link
Author

thbar commented Sep 14, 2011

@dhruvasagar
Copy link

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.

@thbar
Copy link
Author

thbar commented Sep 14, 2011

@dhruvasagar thanks for the link!

@alexrothenberg
Copy 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...

@thbar
Copy link
Author

thbar commented Sep 14, 2011

@alexrothenberg very good point, thanks

@TrevorBurnham
Copy link

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.

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