rniemeyer / knockout-sortable

A Knockout.js binding to connect observableArrays with jQuery UI sortable functionality
MIT License
547 stars 128 forks source link

KnockoutJS 3.3 #138

Closed JD-Robbs closed 9 years ago

JD-Robbs commented 9 years ago

Hi RN,

I'm loving this plugin!

Unfortunately, however, this does not appear to work anymore with the latest KnockoutJS. Would you know of a work-around? I'm having trouble understanding the convoluted console output.

rniemeyer commented 9 years ago

@JD-Robbs - I am happy to help you out. Is there a way for me to reproduce your problem? I updated one of the samples to use KO 3.3 and it appears to be working fine (http://jsfiddle.net/rniemeyer/UdXr4/)

algor commented 9 years ago

Sorry, my comment is not exactly on the topic, but does relate to the release 3.3 of the Knockout library. Is it possible to incorporate the means to specify template as an array of nodes (as it is now supported by knockout) and not only as named template or an anonymous template of the container sortable element. That is to to modify respective part of the prepareTemplateOptions() like this:

    //build our options to pass to the template engine
    if (options.data) {
        result[dataName] = options.data;
        if (typeof options.template === "string") {
            result.name = options.template;
        }
        result.nodes = options.template && options.template.nodes;
    } else {
        result[dataName] = valueAccessor();
    }

Let me clarify my use case. I have a component that encapsulates two connected sortable lists. And I'd like to be able to provide single item template for both lists as anonymous template under the outer component declaration that renders both lists with encapsulated logic:

image

And component initialization code gets componentInfo.templateNodes and uses it as an itemTemplate for both lists. Does the case justifies the changes?

And, sorry for the lengthy comment, but I also have a question on the behavior I noticed when I implement the logic as I described. I noticed that when using the same array of nodes for both lists, if I do not clone the list of nodes then only the list that renders its template first is rendered. The second one does not. Was not able to debug it today but if, by chance, you already know the answer then could you share it? Thanks a lot for the sortable control. It saved me a lot of work.

rniemeyer commented 9 years ago

@algor - The template nodes functionality does remove the nodes from their original parent, so it is likely that you are running into that behavior. I would be willing to put a change like yours into the library. Are you interested in creating a pull request? If not, then I'll look to add the additional functionality when I get a chance.

algor commented 9 years ago

@rniemeyer Thank you for the answer! Yes sure, I'll try to create a pull request the first thing once I get acquainted with git ). Regarding the removing the nodes from their original parent, yes, I read the comment both in knockout documentation and in source codes of the library but it does not explains me the situation. Will create a jsFiddle to localize the issue. Thank you once again!

rniemeyer commented 9 years ago

@algor - the nodes options has been included in the latest version (0.10.0) @JD-Robbs - please let me know if you still are having any problems with KO 3.3

JD-Robbs commented 9 years ago

Apologies for the delay @rniemeyer.

I now believe that something else must have changed in 3.3 (as it worked with 3.2) - something that is not related to sortable.

Uncaught SyntaxError: Unable to process binding "template: function (){return { name:'app-template'} }"
Message: Unable to process binding "foreach: function (){return Tabs }"
Message: Unable to process binding "sortable: function (){return {data:Fields,options:{ handle:'.cursor-move'},isEnabled:$root.isEditable,afterMove:$data.afterMove,beforeRemove:$root.hideRow,afterAdd:$root.showRow} }"
Message: Unable to process binding "if: function (){return Type()=="multi" }"
Message: Unable to process binding "template: function (){return { name:'option-editor-template'} }"
Message: Unable to process binding "sortable: function (){return {data:Options,beforeMove:beforeMove,afterMove:afterMove,options:{ handle:'.cursor-move'},connectClass:'options_list',beforeRemove:$root.hideRow,afterAdd:$root.showRow} }"
Message: Unable to parse bindings.
Bindings value: css {'col-xs-8 col-sm-10' : $parent.UseValues, 'col-lg-12' : !$parent.UseValues() }
Message: Unexpected identifier

Should I find-out why this is happening, I'll give you a shout (it's probably just my code). Thanks!

rniemeyer commented 9 years ago

@JD-Robbs - Sure thing. Let me know if you see anything related to sortable.

JD-Robbs commented 9 years ago

Well, the app breaks where sortable is used for the first time. But I think it's a change in how expressions/bindings are evaluated. I'm triple-reading the 3.3 release notes now. :)