rniemeyer / knockout-sortable

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

IE 8 - JavaScript runtime error: Cannot find closing comment tag to match: ko if: ... #78

Closed joe912 closed 10 years ago

joe912 commented 10 years ago

Hi,

I came across this error when testing in IE8 mode (using IE11) and replicated on IE8 browserstack:

JQuery UI version is v1.9.1

ko-error

Here is simple example showing a "foreach" data bind & a "sortable" data bind. The "foreach" bind works fine:

<!DOCTYPE html>
<html xmlns="http://www.w3.org/1999/xhtml">
    <head>
        <title></title>
        <script src="Scripts/jquery-1.10.2.js"></script>
        <script src="Scripts/jquery-ui.js"></script>

        <script src="http://knockoutjs.com/downloads/knockout-3.0.0.js"></script>

        <script src="Scripts/knockout-sortable.js"></script>

    </head>
    <body>
        <h1>Ko Test</h1>
        <h2>foreach</h2>
        <ul data-bind="foreach: operators">
            <!-- ko if: Op() == 'plus' && Icon() != '' -->
            <li><span>Has Icon</span><input type="text" data-bind="value: Icon" /></li>
            <!-- /ko -->
            <!-- ko if: Op() == 'plus' && Icon() == '' -->
            <li><span>No Icon</span><input type="text" data-bind="value: Op" /></li>
            <!-- /ko -->
        </ul>
        <h2>sortable</h2>
        <ul data-bind="sortable: operators">
            <!-- ko if: Op() == 'plus' && Icon() != '' -->
            <li><span>Has Icon</span><input type="text" data-bind="value: Icon" /></li>
            <!-- /ko -->
            <!-- ko if: Op() == 'plus' && Icon() == '' -->
            <li><span>No Icon</span><input type="text" data-bind="value: Op" /></li>
            <!-- /ko -->
        </ul>
    </body>
</html>

<script>
    var MyOperator = function () {
        this.Op = ko.observable();
        this.Icon = ko.observable();
    };

    function viewModel() {
        var self = this;

        self.operators = ko.observableArray([
            new MyOperator().Op("plus").Icon("+"),
            new MyOperator().Op("plus").Icon("")]);
    };

    ko.applyBindings(new viewModel());
</script>
rniemeyer commented 10 years ago

For the sortable functionality to work properly you need to have a single element inside of your sortable, so that the index of the element can be used to determine the position in the observableArray:

So you would want a structure more like:

    <li>
    <!-- ko if: Op() == 'plus' && Icon() != '' -->
    <span>Has Icon</span><input type="text" data-bind="value: Icon" />
    <!-- /ko -->
    <!-- ko if: Op() == 'plus' && Icon() == '' -->
    <span>No Icon</span><input type="text" data-bind="value: Op" />
    <!-- /ko -->
    </li>

Sample: http://jsfiddle.net/rniemeyer/fuy9r/

There is some functionality in the sortable binding that tries to strip leading and trailing non-elements and it is causing an issue in your scenario. This functionality could be refined to at least not generate the error that you are seeing, but without a single element you would likely see some issues with it not updating the appropriate index.

I will try to call this out in the documentation.

joe912 commented 10 years ago

Ok thanks for the info.
By the way - thanks for creating this plugin! keep up the good work