Skip to content

Instantly share code, notes, and snippets.

@thbar
Created September 14, 2011 08:53
Show Gist options
  • Select an option

  • Save thbar/1216136 to your computer and use it in GitHub Desktop.

Select an option

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
})
@thbar

thbar commented Sep 14, 2011

Copy link
Copy Markdown
Author

@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
Copy Markdown

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

thbar commented Sep 14, 2011

Copy link
Copy Markdown
Author

@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
Copy Markdown

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

thbar commented Sep 14, 2011

Copy link
Copy Markdown
Author

@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
Copy Markdown

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

@thbar

thbar commented Sep 14, 2011

Copy link
Copy Markdown
Author

@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

thbar commented Sep 14, 2011

Copy link
Copy Markdown
Author

@dhruvasagar

Copy link
Copy Markdown

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

thbar commented Sep 14, 2011

Copy link
Copy Markdown
Author

@dhruvasagar thanks for the link!

@alexrothenberg

Copy link
Copy Markdown

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

thbar commented Sep 14, 2011

Copy link
Copy Markdown
Author

@alexrothenberg very good point, thanks

@TrevorBurnham

Copy link
Copy Markdown

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