ManuelDeLeon / viewmodel

MVVM for Meteor
https://viewmodel.org
MIT License
205 stars 23 forks source link

Tips needed: Weird event bind problem #214

Closed arggh closed 8 years ago

arggh commented 8 years ago

I'm just asking for a tip in which direction to start looking for the solution to the weirdest problem in a while.

I basically have a very similar module in my app as the Meteor Todos, but built with ViewModel. My "lists" are clickable (I have a VM click handler that sets the list as selected) and all the existing lists are truly clickable after page load, but if I add a new list, it's missing all the event handlers. Template is basically this simple:

<template name="listRow">
   <a {{b "click: setSelected"}}>
       {{listName}}
       <div>something else here</div>
   </a>
</template>

and on the parent, I iterate over them

{{#each lists}}
  {{>listRow}}
{{/each}}

I use share to share the selected list id between VM's.

When I inspect the DOM nodes, the newly inserted list is really missing all the event listeners. However, the ViewModel still gets created (I have a log message appear from the VM's onCreated() -function). The click binding just doesn't exist. If I refresh the page, the previously created list is clickable now.

I tried reproducing the issue on a small repro, but couldn't yet. It's working there. I'll try to rebuild it piece by piece until I can pinpoint the issue, but in the meanwhile, if anybody has ideas what could cause this.........

ManuelDeLeon commented 8 years ago

I'm sorry but I have no clue what could be causing it. A long shot might be that the objects don't have an _id and Blaze is rearranging them. The best I can say is keep working on a repro and of course I'll be happy to take a look at it. On Mar 28, 2016 6:59 PM, "arggh" notifications@github.com wrote:

I'm just asking for a tip in which direction to start looking for the solution to the weirdest problem in a while.

I basically have a very similar module in my app as the Meteor Todos, but built with ViewModel. My "lists" are clickable (I have a VM click handler that sets the list as selected) and all the existing lists are truly clickable after page load, but if I add a new list, it's missing all the event handlers. Template is basically this simple:

and on the parent, I iterate over them

{{#each lists}} {{>listRow}} {{/each}}

I use share to share the selected list id between VM's.

When I inspect the DOM nodes, the newly inserted list is really missing all the click handlers. However, the ViewModel still gets created (I have a log message appear from the VM's onCreated() -function). The click handler just doesn't exist. If I refresh the page, the previously created list is clickable now.

I tried reproducing the issue on a small repro, but couldn't yet. It's working there. I'll try to rebuild it piece by piece until I can pinpoint the issue, but in the meanwhile, if anybody has ideas what could cause this.........

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/ManuelDeLeon/viewmodel/issues/214

arggh commented 8 years ago

That might be something. I am using Astronomy and it's Sort-behaviour to make ordering of these "lists" possible. The helper that provides lists for the #each-iterator finds them sorted by this order-field.

Another idea that your answer created is the fact that Astronomy's doc.insertAt(index)-function doesn't return the inserted document's _id, so basically in the Meteor method I have to first insertAt(0) and then make a new query to actually get the _id and return it.

Thanks for this, I got a spark of hope and some energy to keep going :)

arggh commented 8 years ago

I just noticed, that also the newly added items of a list are missing the click event listener on the template's outermost parent node, but do exist on the same template's child nodes... this is getting stranger by the minute.

I just didn't notice this before, since clicking on the item's outer div had the same end-result as clicking the inner text element, but with careful positioning of the cursor I noticed that the outer div doesn't have the click binding, if it was inserted in to collection after page load.

fvpDev commented 8 years ago

Lemme see your vm code! I'll chew it up. Upload a .zip or write it inline here, I don't care

arggh commented 8 years ago

@ManuelDeLeon I managed to reproduce the problem: https://github.com/arggh/viewmodel-click-bind

Instructions: Add new lists. Refresh. Add a new list again, select another list, try to re-select the new list. Same with the list items.

ManuelDeLeon commented 8 years ago

Just your luck, I have to update to Meteor 1.3 and my Meteor installation is kinda wonky so it will take a little over forever. I'll work on it after the update.

fvpDev commented 8 years ago

Oh no... me too :tired_face:

arggh commented 8 years ago

On my project the issue appeared also with Meteor 1.2.1, if I recall correctly. I also tried downgrading ViewModel to 3.4.10 or similar with no luck.

fvpDev commented 8 years ago

Okay...so I added

    events: {
        'click a': function () { this.selectList(); }
    }

to list_row.js and it works beautifully. I've actually run across the same problem myself; must be something with the event binding in {{b "click: selectList"}}.

I've just resorted to keeping methods in the vm code as much as possible though

ManuelDeLeon commented 8 years ago

This is definitely not a problem with ViewModel. The problem is how you (Astronomy?) are modifying List.find. It works fine if you fetch the documents:

Template.todo_lists.viewmodel({

  todoLists: function() {
    return List.find({}, { sort: { order: 1 }}).fetch();
  },
fvpDev commented 8 years ago

@ManuelDeLeon <- Master Debugger over here

arggh commented 8 years ago

Fetching the documents "fixes" the problem, but I have no clue why it does so. I wouldn't want to make the computation depend on the entire query result set by fetching, especially since I don't understand why.

I would love to know what exactly goes haywire in this scenario: how Astronomy's modifications of the iterated documents manages to prevent VM's template bindings from being registered when adding new documents on the fly.

The way I see it, I should be able to iterate over any kind of documents, pass them as data for VM's and consume their properties without quirks like these: they're just objects after all.

The mechanics of this issue are beyond my comprehension, so if you have any kind of insight I could carry over to Astronomy's repo, please do elaborate!

ManuelDeLeon commented 8 years ago

Thank you for pushing back. Pick up v4.0.7

As far as I can tell this is the same issue as: https://github.com/ManuelDeLeon/viewmodel/issues/188#issuecomment-191922456

To be honest, I'm not entirely comfortable with the change because I still have no clue why packages like Astronomy or tap:i18n mess up onViewReady. I switched to afterFlush instead. It seems to work under all circumstances.

dnish commented 8 years ago

Thanks @ManuelDeLeon - just run today into this issue, on Chrome mobile my messages VMs stopped working, f.e. I've used:

  <div class="message pink accent-2" {{b "class:{own:isMe, blue:isMe}"}}>....</div>

Suddenly, "own" wasn't added anymore to newer messages. After upgrading to the current version everything works fine now :)

I've to say that this error only happened on Chrome Mobile 49, on Chrome 49 (Mac) I didn't have this issue.

arggh commented 8 years ago

Thanks @ManuelDeLeon ! I can confirm this fixed the issue in the repro and my actual project as well. I posted the same issue on Astronomy's repo as well to maybe get an opinion or an idea from it's creator - would be nice if everybody was entirely comfortable with the solution. I'll reopen this if anything does come up.

arggh commented 8 years ago

More trouble, possibly related to the latest update. Now, in my project, after I change the selected list, I get this:

Exception from Tracker afterFlush function:
meteor.js?hash=ec96c6f…:913 Error: Can't select in removed DomRange
    at DOMRange.$ (blaze.js?hash=38069f4…:683)
    at Blaze.TemplateInstance.$ (blaze.js?hash=38069f4…:3507)
    at viewmodel.coffee:189
    at Object.Tracker._runFlush (tracker.js?hash=b267c37…:515)
    at onGlobalMessage (meteor.js?hash=ec96c6f…:391)

and in viewmodel.coffee it's pointing to these lines:

    # Blaze.currentView.onViewReady fails for some packages like jagi:astronomy and tap:i18n
      Tracker.afterFlush ->
        element = currentViewInstance.$("[#{bindIdAttribute}='#{bindId}']")
        templateInstance.viewmodel.bind bindObject, templateInstance, element, bindings, bindId, currentView
ManuelDeLeon commented 8 years ago

Give me the repro... On Mar 29, 2016 9:19 PM, "arggh" notifications@github.com wrote:

Reopened #214 https://github.com/ManuelDeLeon/viewmodel/issues/214.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/ManuelDeLeon/viewmodel/issues/214#event-607656891

arggh commented 8 years ago

I'll try to introduce it to the repro, takes a while, just thought I'd give a heads up.

I went using the rest of my app and many other things now seem broken as well. Click bindings seem broken in many places at first inspection.

arggh commented 8 years ago

Actually, the old repro is fine for reproducing the same error. Just select a list, add a new item by typing to the input and hitting enter, then switch to another list. I get:

Exception from Tracker afterFlush function:
meteor.js?hash=ec96c6f…:913 Error: Can't select in removed DomRange
    at DOMRange.$ (blaze.js?hash=38069f4…:683)
    at Blaze.TemplateInstance.$ (blaze.js?hash=38069f4…:3507)
    at viewmodel.coffee:189
    at Object.Tracker._runFlush (tracker.js?hash=b267c37…:515)
    at onGlobalMessage (meteor.js?hash=ec96c6f…:391)

Just remember to run meteor update so you have the newest VM.

arggh commented 8 years ago

Click bindings seem broken in many places at first inspection.

This was a bit of an exacerbation (most of my project seems fine), but truly 4.0.7 caused one of my click binds to go wonky. I have two icons which simulate the effect of a checkbox by hide & show with click bindings. Clicking on the icon does nothing if using 4.0.7 until I make a couple of refreshes / rerenders on the page. Then it randomly starts working.

dnish commented 8 years ago

@arggh Just thought this DOMRemove thing was caused by my programming style, but yeah, I get it too.

 onRendered: function () {

        //Sometimes this causes a DOMRemove error
        this.templateInstance.$(".text-message").addClass("animated fadeIn");

})

I'm not really sure, but I think this happened already before the VM update.

ManuelDeLeon commented 8 years ago

@arggh Fixed on v4.0.8

@dnish Fixed on v4.0.9