Handle multiple binding when using CoffeeScript & Turbolinks

Hi,
After watching the screencast about extracting JS code to CS classes, I tried to follow the same pattern.
The problem I am facing is that the page loads new items through AJAX and in order for them to respond to clicks like other object, i'm using the page:update event

$(document).on "page:update", ->
  DWiz.newAnswerBtns = $.map $("[data-behavior='new-answer-btn']"), (item, i) ->
    new DWiz.NewAnswerBtn(item)

the classes above have their events handling (listening to on 'click' etc).
The problem is that on every load of new items, the events are registered again and hence fire multiple times.
I am also using the jquery-turbolinks gem but it's not helping.
Any idea on how to approach this?
Thanks in advance,
Roy

The main reason why this is happening is because anytime the page updates, you're adding another listener to the existing items too.

What you'll actually need to do is scope your click event to the parent with jQuery. Here's a better description of how to do that: http://stackoverflow.com/a/8752376/277994

This way you won't be applying the click event to every individual item, but to the parent and then detecting the click on any of the children (which allows you to automatically target the newly added html).

Thanks Chris.
It implies quite a lot of changes now. it makes me feel that making classes now while most event listeners are on the parent is not the "cleanest" way.
I was wondering what would be your approach to the following scenario:

1. I have a Q&A site for which I have answers container
2. for each answer I have comment list (quite similar to FB threads with comments
3. I load new answers with comments when scrolling down

The latter is loaded with AJAX and are inserted into the DOM with create.js.erb (not returning JSON and handling it on the client right now)
Those loaded answers should have functionality of expending comment list etc so as you wrote above, the listeners seems to be on the container which is loaded at the beginning.

Question is, how would you model it CS classes in this case (I used to have class for almost any object: answer, comment, answerList, commentList etc...)

Thanks,
Roy

Hey Roy, Would you mind sharing what CS you've got right now? I can probably give you better pointers with that and show how I'd probably refactor it. That might make things easier.

Hi Chris.
Below is what I came up with. basically, I have an answers container which has list of answers. each answer is a container for the comments on it, so hierarchy is: answer-list => answer => comments-list => comments

class DWiz.AnswerList
  constructor: (@answerList) ->
    @setEvents()

  setEvents: =>
    @answerList.on 'click', "[data-behavior='show-comments']", (e) => @toggleCommentsAndSetFocus(e, false)
    @answerList.on 'click', "[data-behavior='new-comment-link']", (e) => @toggleCommentsAndSetFocus(e, true)

  toggleCommentsAndSetFocus: (e, setFocus) =>
    answer = $(e.target).parentsUntil('[data-behavior="answer"]')
    commentList = new DWiz.CommentList $(answer).find("[data-behavior='comments']")
    comment = new DWiz.Comment $(answer).find("[data-behavior='comment']")
    commentList.toggleComments()
    comment.toggleComment()
    if setFocus
      newCommentLinkTopPos = $(answer).find("[data-behavior='new-comment-link']").offset().top
      comment.setFocus(newCommentLinkTopPos)

$(document).on "page:change", (e) ->
  answerButtons = $.map $("[data-behavior='new-answer-btn']"), (item, i) -> new DWiz.NewAnswerBtn(item)
 answersList = new DWiz.AnswerList $("[data-behavior='answers-list']")

to clarify some more, in the answers I have two links (show comments & comment), they both do quite the same (open the loaded comments and optionally sets focus on new comment box)
below is the code for comments:

class DWiz.CommentList
  constructor: (el) ->
    @container = $(el)

  toggleComments: =>
    @container.collapse('toggle')


class DWiz.Comment
  constructor: (el) ->
    @container = $(el)
    @textInput = @container.find("[data-behavior='comment-input']")
    @submitBtn = @container.find("[data-behavior='comment-submit']")
    @setEvents()

  setEvents: =>
    @submitBtn.on 'click', @handleSubmit

  toggleComment: =>
    @container.toggleClass('list-group-item').collapse('toggle')

  setFocus: (elPos) =>
    if @container.hasClass 'list-group-item'
      dist = @textInput.offset().top - elPos
      $('body').animate { scrollTop: elPos + dist }, 500
      @textInput.focus()

  handleSubmit: (e) =>
    e.preventDefault()
    comment = @textInput.val()
    return unless comment
    data = comment: {}
    data['comment']['answer_id'] = @textInput.data('answer')
    data['comment']['comment'] = comment
    $.post '/comments', data

As you can see, the handler for showing the comments is on the parent (AnswerList) which creates an instance of comment. The problem with that is that if I click show comments multiple times, I end up having multiple instances of the comment class which when I try to submit a comment, it will be submitted as the number of instances...
pretty long but I hope it clarifies.
Cheers

Ah! I got you. I would actually move things up and create a CS class that wraps the entire answer and then delegates down. Take a look at this.

class DWiz.Answer
  constructor: (@answer) ->
    @answerList = new DWiz.AnswerList @answer.find("[data-behavior='answers-list']")
    @comments   = new DWiz.CommentList @answer.find("[data-behavior='comments']")
    @comment    = new DWiz.Comment @answer.find("[data-behavior='comment']")
    @new_comment_link = @answer.find("[data-behavior='new-comment-link']")

    @setEvents()

  setEvents: =>
    @answer.on 'click', "[data-behavior='show-comments']", (e) => @answerList.toggleCommentsAndSetFocus(e, @answer, false)
    @answer.on 'click', "[data-behavior='new-comment-link']", (e) => @answerList.toggleCommentsAndSetFocus(e, @answer, true)

class DWiz.AnswerList
  constructor: (@answerList) ->
    @setEvents()

  toggleCommentsAndSetFocus: (e, answer, setFocus) =>
    answer.commentList.toggleComments()
    answer.comment.toggleComment()

    if setFocus
      answer.comment.setFocus(answer.offset().top)

$(document).on "page:change", (e) ->
  #answerButtons = $.map $("[data-behavior='new-answer-btn']"), (item, i) -> new DWiz.NewAnswerBtn(item)
  #answersList = new DWiz.AnswerList $("[data-behavior='answers-list']")
  @answers = $.map $("[data-behavior='answers']"), (item, i) -> new DWiz.Answer(item)

It really doesn't need the "AnswerList" class anymore unless you plan on doing more complex things with it. You could simply move that method up to the Answer class. I kind of like having them separated because it shows much more clearly who handles what.

The giveaway for this refactoring was that you were doing parentsUntil you got to the answer. That meant to me that you should actually have the CS class accept the answer instead, and then you could create references to all the children and they could talk to each other much more nicely. Anytime you catch yourself doing "find" outside the constructor, you can usually pull those back up to the constructor and reference them later.

Does that make more sense?

Also note, I haven't tested any of this code, so I doubt it works without a couple fixes here and there.

Hi Chris.
Thanks for the prompt reply. This is what I actually started with which is bringing back the issue of new answers loaded through AJAX.
If I just "push" those loaded new answers into the DOM. there won't be any class for them and no "parent" to listen to the click events on them (this is why I moved things up to a parent container). Another solution for that would probably be to create those classes when AJAX completes the loading of the new answers (i.e., load new answer and then create a new Answer class for each)
In the meantime, I did something a bit different: In my AnswerList, I manage classes that were already created (keeping track on them with an array). this way, if nothing is clicked, no classes are instantiated and if it is clicked, I push them to array and use them in subsequent clicks. this way it also takes care of newly loaded items. The problem with this approach is that it still feels that the AnswerList knows too much about classes it shouldn't.
BTW - parentsUntil was the wrong one, I changed to closest ;)

Oh, and BTW, by looking at the suggested code (I know you don't have the full picture so it's very hard) I see that in the Answer class you're creating in the container a new AnswerList (@answerList = new DWiz.AnswerList @answer.find("[data-behavior='answers-list']") which will end up having answerlist item for each answer that doesn't make sense.
The AnswerList should be the container of all Answer

Cheers

I see your point there. I'd probably have a better answer playing with the code. ;)

Another solution for that would probably be to create those classes when AJAX completes the loading of the new answers (i.e., load new answer and then create a new Answer class for each)

That's probably what I would have suggested as well. Create a new Answer instance for the element you just inserted. If you're injecting the Answer HTML into the page in a create.js.erb file you could probably just do it there as long as your CS class is globally available.

And yeah, AnswerList should contain all of those. Hard to name things without seeing the HTML as well. :)

Thanks a lot for taking the time looking at this, highly appreciated!

Anytime! Always happy to take a look or brainstorm with you.