Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-sorting filtered collectionObservable modifies the original collection and removes all filtered-out items #84

Closed
agroskin opened this issue Aug 21, 2013 · 1 comment

Comments

@agroskin
Copy link

There is a problem if you use knockout-sortable with dynamically filtered and sorted arrays. Here is an example:

panels = new Backbone.Collection([ { dock: 'left', order: 0 }, { dock: 'right', order: 1 }, ..... ]);

panelsLeft = kb.collectionObservable(panels, {
filters: function (panel) { return panel.get("dock") != "left"; },
sort_attribute: "order"
});

panelsRight = kb.collectionObservable(panels, {
filters: function (panel) { return panel.get("dock") != "right"; },
sort_attribute: "order"
});

<div data-bind="sortable: panelsLeft"> ... template

<div data-bind="sortable: panelsRight"> ... template

You sort the left panels, and all right panels disappear, and vice versa.

collectionObservable is listening to changes and updating the source collection, but it doesn't take filters into account.

I am trying to fix it, but not sure how yet.

@agroskin
Copy link
Author

agroskin commented Sep 5, 2013

I found and fixed a bug in your implementation of CollectionObservable._onObservableArrayChange.

Your code resets the source Backbone collection when you re-order any of filtered ObservableCollections (having the same source), and therefore it loses all filtered-out models:

            this.in_edit++;
            (models_or_view_models.length === view_models.length) || observable(view_models);
            _.isEqual(collection.models, models) || collection.reset(models);
            this.in_edit--;

I added this piece of merging code for filtered collections, and everything works properly now:

            this.in_edit++;
            (models_or_view_models.length === view_models.length) || observable(view_models);
            if (has_filters && collection.models.length > 0 && models.length > 0) {
                var _i = 0, _j = 0, newModels = models, left, right;
                models = [];
                var condition = function () {
                    left = _i < collection.models.length ? collection.models[_i] : null; 
                    right = _j < newModels.length ? newModels[_j] : null;
                    return left || right;
                };
                while (condition()) {
                    if (left == right) {
                        _i++;
                        _j++;
                        models.push(left);
                        continue;
                    };
                    if (left) {
                        _i++;
                        if (this._modelIsFiltered(left)) {
                            models.push(left);
                        }
                        continue;
                    }
                    if (right) {
                        _j++;
                        models.push(right);
                    }
                }
            }
            _.isEqual(collection.models, models) || collection.reset(models);
            this.in_edit--; 

@agroskin agroskin closed this as completed Sep 5, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant