Skip to content

Commit

Permalink
Fix conditions when dom-repeat needs to re-sort or re-filter.
Browse files Browse the repository at this point in the history
* For efficiency do not process `items.length` changes
* Ensure observing `foo` is not triggered by a `foobar` etc.
* Ensure assignments like `items.Polymer#2` etc. will trigger a resort/-filter

Fixes Polymer#3626.
  • Loading branch information
kaste committed May 2, 2016
1 parent c0e0a73 commit caabc41
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 23 deletions.
73 changes: 50 additions & 23 deletions src/lib/template/dom-repeat.html
Original file line number Diff line number Diff line change
Expand Up @@ -342,9 +342,10 @@
this._debounceTemplate(this._render);
},

_observeChanged: function() {
this._observePaths = this.observe &&
this.observe.replace('.*', '.').split(' ');
_observeChanged: function(observe) {
// All observed paths are handled as wildcard/deep observation. So if
// the user specifies a wildcard, we just strip it.
this._observePaths = observe && observe.replace('.*', '').split(' ');
},

_itemsChanged: function(change) {
Expand All @@ -366,29 +367,57 @@
this._keySplices = this._keySplices.concat(change.value.keySplices);
this._indexSplices = this._indexSplices.concat(change.value.indexSplices);
this._debounceTemplate(this._render);
} else if (change.path === 'items.length') {
// nothing to do
} else { // items.*
// slice off 'items.' ('items.'.length == 6)
var subpath = change.path.slice(6);
this._forwardItemPath(subpath, change.value);
this._checkObservedPaths(subpath);
var headTail = this._splitHeadTail(subpath);
var key = headTail.head;
var path = headTail.tail;
this._forwardItemPath(key, path, change.value);
if (this._shouldResortOrFilter(key, path)) {
// TODO(kschaaf): interim solution: ideally this is just an
// incremental insertion sort of the changed item
this._needFullRefresh = true;
if (this.delay) {
this.debounce('render', this._render, this.delay);
} else {
this._debounceTemplate(this._render);
}
}
}
},

_checkObservedPaths: function(path) {
if (this._observePaths) {
path = path.substring(path.indexOf('.') + 1);
_splitHeadTail: function(path) {
var dot = path.indexOf('.');
if (dot === -1) {
return {head: path};
} else {
var head = path.slice(0, dot);
var tail = path.slice(dot + 1);
return {head: head, tail: tail};
}
},

_shouldResortOrFilter: function(key, path) {
if (!this._sortFn && !this._filterFn) {
return;
}
if (this._needFullRefresh) {
return;
}

if (path === undefined) {
return true;
} else if (this._observePaths) {
var paths = this._observePaths;
for (var i=0; i<paths.length; i++) {
if (path.indexOf(paths[i]) === 0) {
// TODO(kschaaf): interim solution: ideally this is just an incremental
// insertion sort of the changed item
this._needFullRefresh = true;
if (this.delay) {
this.debounce('render', this._render, this.delay);
} else {
this._debounceTemplate(this._render);
}
return;
var observedPath = paths[i];
if (path === observedPath ||
observedPath.indexOf(path + '.') === 0 ||
path.indexOf(observedPath + '.') === 0) {
return true;
}
}
}
Expand Down Expand Up @@ -767,15 +796,13 @@

// Called as a side effect of a host items.<key>.<path> path change,
// responsible for notifying item.<path> changes to inst for key
_forwardItemPath: function(path, value) {
_forwardItemPath: function(key, path, value) {
if (this._keyToInstIdx) {
var dot = path.indexOf('.');
var key = path.substring(0, dot < 0 ? path.length : dot);
var idx = this._keyToInstIdx[key];
var inst = this._instances[idx];
if (inst && !inst.isPlaceholder) {
if (dot >= 0) {
path = this.as + '.' + path.substring(dot+1);
if (path !== undefined) {
path = this.as + '.' + path;
inst._notifyPath(path, value, true);
} else {
inst.__setProperty(this.as, value, true);
Expand Down
54 changes: 54 additions & 0 deletions test/unit/dom-repeat.html
Original file line number Diff line number Diff line change
Expand Up @@ -3252,6 +3252,60 @@ <h4>x-repeat-chunked</h4>
});
});

suite('re-trigger sort or filter', function() {

var repeater;

setup(function() {
var el = document.createElement('x-primitive-large');
repeater = el.$.repeater;
repeater.sort = function() {};
repeater.filter = function() {};
repeater.render();
});

test('no observe', function() {
assert.isTrue(repeater._shouldResortOrFilter('#2'));
assert.notOk(repeater._shouldResortOrFilter('#1', 'subproperty'));
});

test('observe property', function() {
repeater.observe = 'foo';

assert.isTrue(repeater._shouldResortOrFilter('#2'));
assert.isTrue(repeater._shouldResortOrFilter('#2', 'foo'));
assert.isTrue(repeater._shouldResortOrFilter('#2', 'foo.bar'));
assert.notOk(repeater._shouldResortOrFilter('#2', 'foobar'));
assert.notOk(repeater._shouldResortOrFilter('#1', 'subproperty'));
});

test('observe property 2', function() {
repeater.observe = 'foo.bar';

assert.isTrue(repeater._shouldResortOrFilter('#2'));
assert.isTrue(repeater._shouldResortOrFilter('#2', 'foo'));
assert.isTrue(repeater._shouldResortOrFilter('#2', 'foo.bar'));
assert.isTrue(repeater._shouldResortOrFilter('#2', 'foo.bar.baz'));

assert.notOk(repeater._shouldResortOrFilter('#2', 'foobar'));
assert.notOk(repeater._shouldResortOrFilter('#2', 'foo.baz'));
assert.notOk(repeater._shouldResortOrFilter('#1', 'subproperty'));
});

test('observe deep property', function() {
repeater.observe = 'foo.bar.*';

assert.isTrue(repeater._shouldResortOrFilter('#2'));
assert.isTrue(repeater._shouldResortOrFilter('#2', 'foo'));
assert.isTrue(repeater._shouldResortOrFilter('#2', 'foo.bar'));
assert.isTrue(repeater._shouldResortOrFilter('#2', 'foo.bar.baz'));

assert.notOk(repeater._shouldResortOrFilter('#2', 'foobar'));
assert.notOk(repeater._shouldResortOrFilter('#2', 'foo.baz'));
assert.notOk(repeater._shouldResortOrFilter('#1', 'subproperty'));
});
});

suite('repeater API', function() {

test('modelForElement', function() {
Expand Down

0 comments on commit caabc41

Please sign in to comment.