From 038d414b4e2ed774d5ccf45d49a2bb42847d2071 Mon Sep 17 00:00:00 2001 From: Sarin Udompanish Date: Wed, 9 Nov 2022 14:05:01 +0700 Subject: [PATCH 1/5] fix(tree): force update all parent items on uncheckAllItems method --- .../elements/src/tree/__test__/tree.test.js | 54 +++++++++++++++++++ .../src/tree/managers/tree-manager.ts | 8 ++- 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/packages/elements/src/tree/__test__/tree.test.js b/packages/elements/src/tree/__test__/tree.test.js index 8ea94a0c49..41df092fd5 100644 --- a/packages/elements/src/tree/__test__/tree.test.js +++ b/packages/elements/src/tree/__test__/tree.test.js @@ -384,6 +384,60 @@ describe('tree/Tree', () => { expect(el.values).to.deep.equal(['1.1', '1.2', '4']); }); + it('Uncheck all items correctly with deep nested data', async () => { + const el = await fixture(''); + el.data = deepNestedData; + await elementUpdated(el); + el.uncheckAll(); + el.expandAll(); + await elementUpdated(el); + const item = el.children[3]; + const itemChild = el.children[4]; + itemChild.click(); + await elementUpdated(el); + el.uncheckAll(); + await elementUpdated(el); + expect(item.checkedState).to.equal(0); + el.uncheckAll(); + await elementUpdated(el); + el.values = ['1.3.1.1']; + await elementUpdated(el); + el.uncheckAll(); + await elementUpdated(el); + expect(item.checkedState).to.equal(0); + expect(itemChild.checkedState).to.equal(0); + }); + + it('Can uncheck all item with disabled parent', async () => { + const el = await fixture(''); + el.data = [ + { + label: 'Item 1', + value: '1', + disabled: true, + expanded: true, + items: [ + { + label: 'Item 1.1', + value: '1.1' + }, + { + label: 'Item 1.2', + value: '1.2' + } + ] + } + ]; + await elementUpdated(el); + const item = el.children[0]; + const itemChild = el.children[1]; + itemChild.click(); + await elementUpdated(el); + el.uncheckAll(); + await elementUpdated(el); + expect(item.checkedState).to.equal(0); + }) + it('Can set values programmatically', async () => { const el = await fixture(''); el.data = nestedData; diff --git a/packages/elements/src/tree/managers/tree-manager.ts b/packages/elements/src/tree/managers/tree-manager.ts index 2847e2a8e2..3126e85ff7 100644 --- a/packages/elements/src/tree/managers/tree-manager.ts +++ b/packages/elements/src/tree/managers/tree-manager.ts @@ -437,6 +437,12 @@ export class TreeManager { * @returns {void} */ public uncheckAllItems (): void { - this.editableItems.forEach(item => this.uncheckItem(item)); + this.items.forEach(item => { + if (this.canUncheckItem(item)) { + this.composer.setItemPropertyValue(item, 'selected', false); + } + }); + // Force update of all visible parent items, making sure any indeterminate states are remove. + this.parentItems.forEach(item => this.updateItem(item)); } } From 3dcbfa72e16555b8d0aaff3788d690797eb7ae6d Mon Sep 17 00:00:00 2001 From: Sarin Udompanish Date: Thu, 10 Nov 2022 14:15:49 +0700 Subject: [PATCH 2/5] fix(tree): remove functionality to uncheck item with disabled parent --- .../elements/src/tree/__test__/tree.test.js | 30 ------------------- .../src/tree/managers/tree-manager.ts | 6 ++-- 2 files changed, 2 insertions(+), 34 deletions(-) diff --git a/packages/elements/src/tree/__test__/tree.test.js b/packages/elements/src/tree/__test__/tree.test.js index 41df092fd5..acea151bd0 100644 --- a/packages/elements/src/tree/__test__/tree.test.js +++ b/packages/elements/src/tree/__test__/tree.test.js @@ -408,36 +408,6 @@ describe('tree/Tree', () => { expect(itemChild.checkedState).to.equal(0); }); - it('Can uncheck all item with disabled parent', async () => { - const el = await fixture(''); - el.data = [ - { - label: 'Item 1', - value: '1', - disabled: true, - expanded: true, - items: [ - { - label: 'Item 1.1', - value: '1.1' - }, - { - label: 'Item 1.2', - value: '1.2' - } - ] - } - ]; - await elementUpdated(el); - const item = el.children[0]; - const itemChild = el.children[1]; - itemChild.click(); - await elementUpdated(el); - el.uncheckAll(); - await elementUpdated(el); - expect(item.checkedState).to.equal(0); - }) - it('Can set values programmatically', async () => { const el = await fixture(''); el.data = nestedData; diff --git a/packages/elements/src/tree/managers/tree-manager.ts b/packages/elements/src/tree/managers/tree-manager.ts index 3126e85ff7..86e53ef729 100644 --- a/packages/elements/src/tree/managers/tree-manager.ts +++ b/packages/elements/src/tree/managers/tree-manager.ts @@ -437,10 +437,8 @@ export class TreeManager { * @returns {void} */ public uncheckAllItems (): void { - this.items.forEach(item => { - if (this.canUncheckItem(item)) { - this.composer.setItemPropertyValue(item, 'selected', false); - } + this.editableItems.forEach(item => { + this.composer.setItemPropertyValue(item, 'selected', false); }); // Force update of all visible parent items, making sure any indeterminate states are remove. this.parentItems.forEach(item => this.updateItem(item)); From 15b7db384c50a8a133137762ae0535ad3e1bf5f1 Mon Sep 17 00:00:00 2001 From: Sarin Udompanish Date: Thu, 10 Nov 2022 18:22:12 +0700 Subject: [PATCH 3/5] refactor(tree): improve uncheckAllItems method performance --- packages/elements/src/tree/managers/tree-manager.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/elements/src/tree/managers/tree-manager.ts b/packages/elements/src/tree/managers/tree-manager.ts index 86e53ef729..ab928ddc4f 100644 --- a/packages/elements/src/tree/managers/tree-manager.ts +++ b/packages/elements/src/tree/managers/tree-manager.ts @@ -410,6 +410,7 @@ export class TreeManager { this.forceUpdateOnPath(item); this.getItemDescendants(item).forEach(descendant => this._uncheckItem(descendant, false)); } + this.updateItem(item); return true; } return false; @@ -437,10 +438,8 @@ export class TreeManager { * @returns {void} */ public uncheckAllItems (): void { - this.editableItems.forEach(item => { - this.composer.setItemPropertyValue(item, 'selected', false); - }); - // Force update of all visible parent items, making sure any indeterminate states are remove. - this.parentItems.forEach(item => this.updateItem(item)); + // uncheck items from top levels when manage relationships to avoid redundant call to force re-render item + const items = this.manageRelationships ? this.composer.queryItems(() => true, 0) : this.checkedItems; + items.forEach(item => this.uncheckItem(item)); } } From 7057f4417acbe4a4a1bb5a96a113883713c6a1cb Mon Sep 17 00:00:00 2001 From: Sarin Udompanish Date: Thu, 10 Nov 2022 18:25:12 +0700 Subject: [PATCH 4/5] chore(tree): improve method description --- packages/elements/src/tree/managers/tree-manager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/elements/src/tree/managers/tree-manager.ts b/packages/elements/src/tree/managers/tree-manager.ts index ab928ddc4f..7c99a169f4 100644 --- a/packages/elements/src/tree/managers/tree-manager.ts +++ b/packages/elements/src/tree/managers/tree-manager.ts @@ -438,7 +438,7 @@ export class TreeManager { * @returns {void} */ public uncheckAllItems (): void { - // uncheck items from top levels when manage relationships to avoid redundant call to force re-render item + // uncheck items from top levels when manage relationships to avoid redundant re-renders const items = this.manageRelationships ? this.composer.queryItems(() => true, 0) : this.checkedItems; items.forEach(item => this.uncheckItem(item)); } From 674c3c71452ce23d6ec78660685af1db5b6306e2 Mon Sep 17 00:00:00 2001 From: Sarin Udompanish Date: Thu, 17 Nov 2022 15:44:36 +0700 Subject: [PATCH 5/5] test(tree): add check and uncheck in noRelation mode test case --- .../elements/src/tree/__test__/tree.test.js | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/packages/elements/src/tree/__test__/tree.test.js b/packages/elements/src/tree/__test__/tree.test.js index acea151bd0..8601fa47c1 100644 --- a/packages/elements/src/tree/__test__/tree.test.js +++ b/packages/elements/src/tree/__test__/tree.test.js @@ -408,6 +408,26 @@ describe('tree/Tree', () => { expect(itemChild.checkedState).to.equal(0); }); + it('check/uncheck all items correctly in no-relation with deep nested data', async () => { + const el = await fixture(''); + el.data = deepNestedData; + await elementUpdated(el); + el.uncheckAll(); + el.expandAll(); + await elementUpdated(el); + const item = el.children[3]; + const itemChild = el.children[4]; + itemChild.click(); + await elementUpdated(el); + el.uncheckAll(); + await elementUpdated(el); + expect(item.checkedState).to.equal(0); + el.checkAll(); + await elementUpdated(el); + expect(item.checkedState).to.equal(1); + expect(itemChild.checkedState).to.equal(1); + }); + it('Can set values programmatically', async () => { const el = await fixture(''); el.data = nestedData;