From 68510cb586b07bfd80de3772142a5012bae2a886 Mon Sep 17 00:00:00 2001 From: Cy Klassen Date: Fri, 24 Aug 2018 11:57:11 -0700 Subject: [PATCH 1/5] Failing test for stale rowMeta index when prepending row --- tests/unit/-private/collapse-tree-test.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/unit/-private/collapse-tree-test.js b/tests/unit/-private/collapse-tree-test.js index 98ea7c493..8819a6bdf 100644 --- a/tests/unit/-private/collapse-tree-test.js +++ b/tests/unit/-private/collapse-tree-test.js @@ -173,6 +173,19 @@ module('Unit | Private | CollapseTree', function(hooks) { } }); + test('rowMeta index works', function(assert) { + let rows = generateTree([1, [2, 3, [4, 5], 6]]); + tree = CollapseTree.create({ rows, rowMetaCache, enableTree: true }); + + let nodes = tree.toArray(); + nodes.forEach((node, i) => assert.equal(metaFor(node).get('index'), i)); + + rows.insertAt(0, { value: 0 }); + + let firstNode = run(() => tree.objectAt(0)); + [firstNode].concat(nodes).forEach((node, i) => assert.equal(metaFor(node).get('index'), i)); + }); + test('can disable tree', function(assert) { tree = CollapseTree.create({ rows: generateTree([0, [1, 2]]), From ce135b0cf45d3f9253d06fc12cd1f25f061ebe77 Mon Sep 17 00:00:00 2001 From: Cy Klassen Date: Fri, 14 Sep 2018 15:30:54 -0700 Subject: [PATCH 2/5] Row meta computes index from _lastKnownIndex and _prevSiblingMeta.index When computing a row's meta index, the last known index (i.e., from last accessed) is reconciled with its previous sibling's index. For example, if its previous sibling's index is the same as its own, this assumes a shift is needed as a result of a lower index insertion. This then cascades forward. --- addon/-private/collapse-tree.js | 34 +++++++++++++++++++---- tests/unit/-private/collapse-tree-test.js | 12 ++++++-- 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/addon/-private/collapse-tree.js b/addon/-private/collapse-tree.js index d982432df..f8e08c28f 100644 --- a/addon/-private/collapse-tree.js +++ b/addon/-private/collapse-tree.js @@ -95,6 +95,18 @@ export class TableRowMeta extends EmberObject { return parentMeta ? get(parentMeta, 'depth') + 1 : 0; } + @computed('_lastKnownIndex', '_prevSiblingMeta.index') + get index() { + let prevSiblingIndex = get(this, '_prevSiblingMeta.index'); + let lastKnownIndex = get(this, '_lastKnownIndex'); + + if (lastKnownIndex === prevSiblingIndex) { + return lastKnownIndex + 1; + } + + return lastKnownIndex; + } + @computed('_tree.length') get first() { if (get(this, '_tree.length') === 0) { @@ -692,18 +704,28 @@ export default class CollapseTree extends EmberObject.extend(EmberArray) { @return {{ value: object, parents: Array }} */ objectAt(index) { - if (index >= get(this, 'length') || index < 0) { + let length = get(this, 'length'); + if (index >= length || index < 0) { return undefined; } + let root = get(this, 'root'); + let rowMetaCache = this.get('rowMetaCache'); + // We add a "fake" top level node to account for the root node let normalizedIndex = index + 1; - let result = get(this, 'root').objectAt(normalizedIndex); - let meta = this.get('rowMetaCache').get(result); + let result = root.objectAt(normalizedIndex); + let meta = rowMetaCache.get(result); - // Set the perceived index on the meta. It should be safe to do this here, since - // the row will always be retrieved via `objectAt` before being used. - set(meta, 'index', index); + // Set the last known index on the meta and link the next siblings meta + // so that its index can recompute in case it conflicts from shifting + set(meta, '_lastKnownIndex', index); + + if (index < length - 1) { + let nextSibling = root.objectAt(normalizedIndex + 1); + let nextMeta = rowMetaCache.get(nextSibling); + set(nextMeta, '_prevSiblingMeta', meta); + } return result; } diff --git a/tests/unit/-private/collapse-tree-test.js b/tests/unit/-private/collapse-tree-test.js index 8819a6bdf..1b9a92462 100644 --- a/tests/unit/-private/collapse-tree-test.js +++ b/tests/unit/-private/collapse-tree-test.js @@ -174,16 +174,22 @@ module('Unit | Private | CollapseTree', function(hooks) { }); test('rowMeta index works', function(assert) { - let rows = generateTree([1, [2, 3, [4, 5], 6]]); + let rows = generateTree([1, [2, 3, [4, 5], 6], 7]); tree = CollapseTree.create({ rows, rowMetaCache, enableTree: true }); let nodes = tree.toArray(); nodes.forEach((node, i) => assert.equal(metaFor(node).get('index'), i)); - rows.insertAt(0, { value: 0 }); + rows.unshiftObject({ value: 0 }); let firstNode = run(() => tree.objectAt(0)); - [firstNode].concat(nodes).forEach((node, i) => assert.equal(metaFor(node).get('index'), i)); + nodes = [firstNode].concat(nodes); + nodes.forEach((node, i) => assert.equal(metaFor(node).get('index'), i)); + + rows.pushObject({ value: 8 }); + + let lastNode = run(() => tree.objectAt(8)); + nodes.concat(lastNode).forEach((node, i) => assert.equal(metaFor(node).get('index'), i)); }); test('can disable tree', function(assert) { From 8aad500746da45139d7db554a627c9d2c9754867 Mon Sep 17 00:00:00 2001 From: Cy Klassen Date: Tue, 13 Nov 2018 13:18:00 -0800 Subject: [PATCH 3/5] Update prev and next to use _lastKnownIndex --- addon/-private/collapse-tree.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/addon/-private/collapse-tree.js b/addon/-private/collapse-tree.js index f8e08c28f..99d65e87e 100644 --- a/addon/-private/collapse-tree.js +++ b/addon/-private/collapse-tree.js @@ -124,18 +124,18 @@ export class TableRowMeta extends EmberObject { @computed('_tree.length') get next() { let tree = get(this, '_tree'); - if (get(this, 'index') + 1 >= get(tree, 'length')) { + if (get(this, '_lastKnownIndex') + 1 >= get(tree, 'length')) { return null; } - return tree.objectAt(get(this, 'index') + 1); + return tree.objectAt(get(this, '_lastKnownIndex') + 1); } @computed('_tree.length') get prev() { - if (get(this, 'index') === 0) { + if (get(this, '_lastKnownIndex') === 0) { return null; } - return get(this, '_tree').objectAt(get(this, 'index') - 1); + return get(this, '_tree').objectAt(get(this, '_lastKnownIndex') - 1); } toggleCollapse() { From 3b3af41a9097fe4be7693ffe220021017fd0ad3c Mon Sep 17 00:00:00 2001 From: Cy Klassen Date: Sun, 2 Dec 2018 13:08:55 -0800 Subject: [PATCH 4/5] Initialize _lastKnownIndex with null value --- addon/-private/collapse-tree.js | 1 + 1 file changed, 1 insertion(+) diff --git a/addon/-private/collapse-tree.js b/addon/-private/collapse-tree.js index 99d65e87e..f8b86c888 100644 --- a/addon/-private/collapse-tree.js +++ b/addon/-private/collapse-tree.js @@ -27,6 +27,7 @@ export class TableRowMeta extends EmberObject { */ _cellMetaCache = new Map(); _isCollapsed = false; + _lastKnownIndex = null; @computed('_rowValue.isCollapsed') get isCollapsed() { From 43e742f73201d418d651189e27eeed3fbecbc923 Mon Sep 17 00:00:00 2001 From: Cy Klassen Date: Sun, 2 Dec 2018 13:10:28 -0800 Subject: [PATCH 5/5] Clarify "rowMeta index works" test name --- tests/unit/-private/collapse-tree-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/-private/collapse-tree-test.js b/tests/unit/-private/collapse-tree-test.js index 1b9a92462..e7c2d18a7 100644 --- a/tests/unit/-private/collapse-tree-test.js +++ b/tests/unit/-private/collapse-tree-test.js @@ -173,7 +173,7 @@ module('Unit | Private | CollapseTree', function(hooks) { } }); - test('rowMeta index works', function(assert) { + test('rowMeta index is recomputed when row is added or removed', function(assert) { let rows = generateTree([1, [2, 3, [4, 5], 6], 7]); tree = CollapseTree.create({ rows, rowMetaCache, enableTree: true });