Skip to content

Commit

Permalink
strip top-level comments as well as text nodes in anonymous templates
Browse files Browse the repository at this point in the history
fixes #58
  • Loading branch information
rniemeyer committed Aug 17, 2013
1 parent 4649a8c commit 7bdb343
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 8 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
</script>
```

Note: The sortable binding assumes that the child "templates" have a single container element. You cannot use containerless bindings (comment-based) bindings at the top-level of your template, as the jQuery draggable/sortable functionality needs an element to operate on.

**Additional Options**

* **connectClass** - specify the class that should be used to indicate a droppable target. The default class is "ko_container". This value can be passed in the binding or configured globally by setting `ko.bindingHandlers.sortable.connectClass`.
Expand Down
6 changes: 3 additions & 3 deletions build/knockout-sortable.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// knockout-sortable 0.8.1 | (c) 2013 Ryan Niemeyer | http://www.opensource.org/licenses/mit-license
// knockout-sortable 0.8.2 | (c) 2013 Ryan Niemeyer | http://www.opensource.org/licenses/mit-license
;(function(factory) {
if (typeof define === "function" && define.amd) {
// AMD anonymous module
Expand Down Expand Up @@ -87,9 +87,9 @@
sortable = {},
startActual, updateActual;

//remove leading/trailing text nodes from anonymous templates
//remove leading/trailing non-elements from anonymous templates
ko.utils.arrayForEach(element.childNodes, function(node) {
if (node && node.nodeType === 3) {
if (node && node.nodeType !== 1) {
node.parentNode.removeChild(node);
}
});
Expand Down
4 changes: 2 additions & 2 deletions build/knockout-sortable.min.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "knockout-sortable",
"version": "0.8.1",
"version": "0.8.2",
"devDependencies": {
"grunt": "~0.4.1",
"grunt-contrib-uglify": "0.x.x",
Expand Down
36 changes: 36 additions & 0 deletions spec/knockout-sortable.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,42 @@ describe("knockout-sortable", function(){
expect(children.eq(2).text()).toEqual("3");
});

it("should strip top-level text nodes", function() {
var children,
options = {
elems: $("<ul data-bind='sortable: items'> <li data-bind='text: $data'> </li></ul>"),
vm: { items: ko.observableArray([1, 2, 3]) }
};

setup(options);

//all children including text/comment nodes
children = options.root.contents();

expect(children.length).toEqual(3);
expect(children.eq(0).text()).toEqual("1");
expect(children.eq(1).text()).toEqual("2");
expect(children.eq(2).text()).toEqual("3");
});

it("should strip top-level comment nodes", function() {
var children,
options = {
elems: $("<ul data-bind='sortable: items'><!-- hello --><li data-bind='text: $data'> </li><!-- good bye --></ul>"),
vm: { items: ko.observableArray([1, 2, 3]) }
};

setup(options);

//all children including text/comment nodes
children = options.root.contents();

expect(children.length).toEqual(3);
expect(children.eq(0).text()).toEqual("1");
expect(children.eq(1).text()).toEqual("2");
expect(children.eq(2).text()).toEqual("3");
});

describe("when using 'as' to name the context", function() {
it("should allow referring to child items by 'as' name", function() {
var children,
Expand Down
4 changes: 2 additions & 2 deletions src/knockout-sortable.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,9 @@
sortable = {},
startActual, updateActual;

//remove leading/trailing text nodes from anonymous templates
//remove leading/trailing non-elements from anonymous templates
ko.utils.arrayForEach(element.childNodes, function(node) {
if (node && node.nodeType === 3) {
if (node && node.nodeType !== 1) {
node.parentNode.removeChild(node);
}
});
Expand Down

0 comments on commit 7bdb343

Please sign in to comment.