knockout
knockout copied to clipboard
knockout
Add templating callback 'beforeAdd'; add callback arg 'isFirstExecution'
I've added this callback 'beforeAdd' to the foreach/templating bindings, so that KO clients can be informed before elements are added to the DOM. This was necessary to make my Isotope binding for Knockout work properly (Isotope needs to be able to filter elements before they are added to the DOM, otherwise new elements are briefly visible before Isotope hides them).
I've written a corresponding spec for the foreach binding, and verified that all tests pass (by running build.sh, so that tests are run under Node and PhantomJS).
This would be a great thing to add to the main branch, as this actually applies to other plugins as well. I am using aknuds1's KnockOut-Isotope binding and would prefer to stay on the main branch of knockout instead of his forked code.
Thanks!
I would love to see some traction on this, but I don't know how often pull requests are considered for this project.
This is interesting, but we generally have to be careful when adding new interfaces. For example, beforeAdd would appear to be a parallel to afterAdd, but the way you've implemented it, it's not. afterAdd is only called for items added after the first execution, whereas your beforeAdd is called for all items during the first execution.
@mbest Thanks for the feedback. I'm sure it could be implemented better, but I don't have innate knowledge of KO's design. Do you have any suggestions as to improvements that could be made?
@mbest Perhaps you could see what @aknuds1 is trying to do over at his Isotope binding repo and suggest a better way to accomplish the beforeAdd?
@mbest I concur with @adrienne on this one, as implementing isotope with afterAdd does not seem to work (if the hours pulling my hair out are any indication). Using @aknuds1 modified code, it worked immediately. Any suggestions on how to get Isotope working with Knockout another way would be welcome for sure. Until then, I will need to keep using his modified Knockout.js.
Thanks!
The particular issue this modification fixes is that Isotope isn't notified before KO adds elements to the DOM, thus they are briefly visible before Isotope's filtering is applied. By calling out to the client (my binding) before an element is added to the DOM, it can be pre-filtered.
@mbest Do you think it might be a better idea to add a callback for creating DOM elements, so that the client (i.e. my binding) has full control over this aspect? Having mulled it over a bit, I figure this would be a more generally applicable solution, as you'd support the cases where the client is forced to control this fully (e.g., if Isotope insisted on creating elements itself).
@adrienne I wish this would get merged too :( I think I'll start another discussion on the mailing list, to see if we can get somewhere. See also my other pull request. In the meantime, I've started publishing builds of Knockout-Isotope with my Knockout fork included, currenctly synced with Knockout 2.3.0.
@adrienne I've initiated a discussion on the mailing list. Hopefully something will come out of that.
Does Knockout 3.0 make this work without any custom hacking, by any chance?
@adrienne Unfortunately, it doesn't appear that way. I've tried to start a discussion on the mailing list previously, but I got little in the way of replies :( I could give it another shot, if you'd like to join in?
I'd like to help get better support for using Knockout with Isotype. Can one of you put together some examples that work with @aknuds1's existing tools and demonstrate the desired usage?
@mbest Hiya Michael, I'm very glad to hear that you're willing to help. I'm far from a front-end wizard so it might not look much, but I made a jsFiddle a while ago that demonstrates how Knockout-Isotope allows you to drive Isotope from Knockout. Basically, the purpose of Knockout-Isotope is to enable defining elements for Isotope to lay out as a ko.observableArray. The contents of the observableArray at any time should be laid out automatically by Isotope. As you should be able to tell from the jsFiddle, Isotope updates the layout immediately as the observableArray changes. That's really all there is to Knockout-Isotope, the rest of the magic is up to Isotope itself.
@mbest If it helps, I have also written extensive Jasmine specs for the binding. You might find them useful if you need to glean the details.
So I had a go at creating a new Knockout-Isotope binding. Because of the issues with using the foreach binding, I did not try to use it. Instead, i decided to take advantage of the new "arrayChange" notifications in Knockout 3.0 to efficiently update Isotope. This means the binding can tailor the updates to work best with Isotope.
Binding: https://gist.github.com/mbest/7577891 jsFiddle Example: http://jsfiddle.net/mbest/ACSGx/
For comparison, here's the same jsFiddle based on @aknuds1's binding: http://jsfiddle.net/mbest/fHW3t/50/
The examples also include my Deferred Updates plugin so they can easily test combined changes, since it automatically coalesces multiple updates into a single one.
While working and testing on this binding, I noticed that with the original binding, the Isotope view and the observableArray can get out of sync. If you use my links above, you can see this clearly if you first click "Shuffle", and then click in the container to add an item. The new item should be added at the end, but instead it's added somewhere in the middle. Also, if you reload and try the "Add/remove some" button, the resulting items should always be in numeric order (which they are in the array), but often they aren't in the view.
@mbest Good work on the new binding and good catch on the view and the model getting out of sync.
Have you considered running my Jasmine specs against your new version though? I just created a branch of Knockout-Isotope based on your version of the binding, and the specs seem to break mostly because you've removed support for passing parameters to Isotope.
I chose not to fully copy your interface. Specifically, it wasn't important for my binding to know about filtering or other classes, and I allowed the caller to overwrite any of the Isotope options.
Actually I don't understand why you need so complicated binding and even a custom knockout version. Here is a plunker (http://plnkr.co/30oQkwlfOE0rKTKSgXyc) where I created masonry binding based on shapeshift jquery plugin with knockout version 2.3.0. I believe this binding can be used with different masonry-like plugins but I've not checked them all (checked with shapeshift, jquery.masonry and jquery.packery).
@stalniy I had to customize Knockout in order to be able to hide elements via Isotope before they were added to the DOM. I couldn't see any other way of achieving this.
For what it's worth, I was able to write a binding for Isotope that doesn't depend on a beforeAdd that seems to work fairly well. It simply extends the foreach binding and uses afterRender as mentioned above. It's available on github, with a working demo on JSFiddle.
@dougludlow Going forward, I suspect that @mbest's implementation is our best option as it is based on Knockout 3 features.