From 01b9721d94b22ce3f1cf658fa6e920ac6e143db8 Mon Sep 17 00:00:00 2001 From: volnyagin Date: Wed, 15 Apr 2020 17:43:25 +0300 Subject: [PATCH 1/2] DataGrid: Rework selectionFilter simplification if deferred selection is used (T814753, T874992) --- .../selection/selection.strategy.deferred.js | 89 ++++++++++----- .../DevExpress.ui.widgets/selection.test.js | 103 +++++++++++++++++- 2 files changed, 161 insertions(+), 31 deletions(-) diff --git a/js/ui/selection/selection.strategy.deferred.js b/js/ui/selection/selection.strategy.deferred.js index 8ec932794b06..b1d3de9a2d28 100644 --- a/js/ui/selection/selection.strategy.deferred.js +++ b/js/ui/selection/selection.strategy.deferred.js @@ -77,7 +77,7 @@ module.exports = SelectionStrategy.inherit({ return !!dataQuery([itemData]).filter(selectionFilter).toArray().length; }, - _processSelectedItem: function(key) { + _getFilterByKey: function(key) { const keyField = this.options.key(); let filter = [keyField, '=', key]; @@ -96,13 +96,13 @@ module.exports = SelectionStrategy.inherit({ }, addSelectedItem: function(key) { - const filter = this._processSelectedItem(key); + const filter = this._getFilterByKey(key); this._addSelectionFilter(false, filter); }, removeSelectedItem: function(key) { - const filter = this._processSelectedItem(key); + const filter = this._getFilterByKey(key); this._addSelectionFilter(true, filter); }, @@ -155,21 +155,19 @@ module.exports = SelectionStrategy.inherit({ _addSelectionFilter: function(isDeselect, filter, isSelectAll) { const that = this; - let needAddFilter = true; const currentFilter = isDeselect ? ['!', filter] : filter; const currentOperation = isDeselect ? 'and' : 'or'; + let needAddFilter = true; let selectionFilter = that.options.selectionFilter || []; selectionFilter = that._denormalizeFilter(selectionFilter); if(selectionFilter && selectionFilter.length) { that._removeSameFilter(selectionFilter, filter, isDeselect, isSelectAll); - const lastOperation = that._removeSameFilter(selectionFilter, filter, !isDeselect); + const filterIndex = that._removeSameFilter(selectionFilter, filter, !isDeselect); + const isKeyOperatorsAfterRemoved = this._isKeyFilter(filter) && this._isKeyOperatorsFromIndex(selectionFilter, filterIndex); - if(lastOperation && (lastOperation !== 'or' && isDeselect || lastOperation !== 'and' && !isDeselect)) { - needAddFilter = false; - selectionFilter = []; - } + needAddFilter = filter.length && !isKeyOperatorsAfterRemoved; if(needAddFilter) { selectionFilter = that._addFilterOperator(selectionFilter, currentOperation); @@ -193,46 +191,85 @@ module.exports = SelectionStrategy.inherit({ }, _removeFilterByIndex: function(filter, filterIndex, isSelectAll) { - let lastRemoveOperation; + const operation = filter[1]; if(filterIndex > 0) { - lastRemoveOperation = filter.splice(filterIndex - 1, 2)[0]; + filter.splice(filterIndex - 1, 2); } else { - lastRemoveOperation = filter.splice(filterIndex, 2)[1] || 'undefined'; + filter.splice(filterIndex, 2); } - if(isSelectAll && lastRemoveOperation === 'and') { + if(isSelectAll && operation === 'and') { filter.splice(0, filter.length); } + }, + _isSimpleKeyFilter: function(filter, key) { + return filter.length === 3 && filter[0] === key && filter[1] === '='; + }, + _isKeyFilter: function(filter) { + if(filter.length === 2 && filter[0] === '!') { + return this._isKeyFilter(filter[1]); + } + const keyField = this.options.key(); - return lastRemoveOperation; + if(Array.isArray(keyField)) { + if(filter.length !== keyField.length * 2 - 1) { + return false; + } + for(let i = 0; i < keyField.length; i++) { + if(i > 0 && filter[i] !== 'and') { + return false; + } + if(!this._isSimpleKeyFilter(filter[i * 2], keyField[i])) { + return false; + } + } + return true; + } + + return this._isSimpleKeyFilter(filter, keyField); }, + _isKeyOperatorsFromIndex: function(selectionFilter, filterIndex) { + if(filterIndex >= 0) { + for(let i = filterIndex; i < selectionFilter.length; i++) { + if(typeof selectionFilter[i] !== 'string' && !this._isKeyFilter(selectionFilter[i])) { + return false; + } + } + + return true; + } + return false; + }, _removeSameFilter: function(selectionFilter, filter, inverted, isSelectAll) { filter = inverted ? ['!', filter] : filter; - const filterIndex = this._findSubFilter(selectionFilter, filter); - if(JSON.stringify(filter) === JSON.stringify(selectionFilter)) { selectionFilter.splice(0, selectionFilter.length); - return 'undefined'; + return 0; } + const filterIndex = this._findSubFilter(selectionFilter, filter); + if(filterIndex >= 0) { - return this._removeFilterByIndex(selectionFilter, filterIndex, isSelectAll); + this._removeFilterByIndex(selectionFilter, filterIndex, isSelectAll); + return filterIndex; } else { for(let i = 0; i < selectionFilter.length; i++) { - const lastRemoveOperation = Array.isArray(selectionFilter[i]) && selectionFilter[i].length > 2 && this._removeSameFilter(selectionFilter[i], filter, false, isSelectAll); - - if(lastRemoveOperation) { - if(!selectionFilter[i].length) { - this._removeFilterByIndex(selectionFilter, i, isSelectAll); - } else if(selectionFilter[i].length === 1) { - selectionFilter[i] = selectionFilter[i][0]; + if(Array.isArray(selectionFilter[i]) && selectionFilter[i].length > 2) { + const filterIndex = this._removeSameFilter(selectionFilter[i], filter, false, isSelectAll); + if(filterIndex >= 0) { + if(!selectionFilter[i].length) { + this._removeFilterByIndex(selectionFilter, i, isSelectAll); + } else if(selectionFilter[i].length === 1) { + selectionFilter[i] = selectionFilter[i][0]; + } + return filterIndex; } - return lastRemoveOperation; } } + return -1; } }, diff --git a/testing/tests/DevExpress.ui.widgets/selection.test.js b/testing/tests/DevExpress.ui.widgets/selection.test.js index 6c34e3912f44..0202a80fb792 100644 --- a/testing/tests/DevExpress.ui.widgets/selection.test.js +++ b/testing/tests/DevExpress.ui.widgets/selection.test.js @@ -1227,9 +1227,7 @@ QUnit.test('changeItemSelection with shift key two times', function(assert) { selection.changeItemSelection(2, { shift: true }); // assert - assert.deepEqual(selection.selectionFilter(), [ - [['id', '=', 2], 'and', ['!', ['id', '=', 5]], 'and', ['!', ['id', '=', 4]]], - 'or', ['id', '=', 3]], 'selection filter'); + assert.deepEqual(selection.selectionFilter(), [['id', '=', 2], 'or', ['id', '=', 3]], 'selection filter'); }); QUnit.test('selectAll when filter is empty', function(assert) { @@ -1529,7 +1527,30 @@ QUnit.test('selectAll -> deselect items -> select item -> deselect item -> selec selection.selectAll(); // assert - assert.deepEqual(selection.selectionFilter(), [[['id', '=', 3], 'and', ['!', ['id', '=', 4]]], 'or', ['age', '>', 15]], 'selection filter'); + assert.deepEqual(selection.selectionFilter(), ['age', '>', 15], 'selection filter'); + assert.strictEqual(selection.getSelectAllState(), true, 'select all is true'); + selection.getSelectedItemKeys().done(function(keys) { + selectedKeys = keys; + }); + assert.deepEqual(selectedKeys, [2, 3, 4, 5, 6, 7], 'selected keys'); +}); + +QUnit.test('select item -> selectAll -> deselect items -> select item -> deselect item -> select All', function(assert) { + let selectedKeys; + const selection = this.createDeferredSelection(this.data); + + // act + this.dataSource.filter(['age', '>', 15]); + selection.changeItemSelection(4, { control: true }); // select item + selection.selectAll(); + selection.changeItemSelection(1, { control: true }); // deselect item + selection.changeItemSelection(2, { control: true }); // deselect item + selection.changeItemSelection(2, { control: true }); // select item + selection.changeItemSelection(3, { control: true }); // deselect item + selection.selectAll(); + + // assert + assert.deepEqual(selection.selectionFilter(), [[['id', '=', 5], 'and', ['!', ['id', '=', 2]], 'and', ['!', ['id', '=', 4]]], 'or', ['age', '>', 15]], 'selection filter'); assert.strictEqual(selection.getSelectAllState(), true, 'select all is true'); selection.getSelectedItemKeys().done(function(keys) { selectedKeys = keys; @@ -1560,6 +1581,32 @@ QUnit.test('selectAll -> deselect items -> select/deselect item -> select All', assert.deepEqual(selectedKeys, [2, 3, 4, 5, 6, 7], 'selected keys'); }); +// T814753 +QUnit.test('selectAll -> deselect/select items -> deselect item -> select All -> deselect item', function(assert) { + let selectedKeys; + const selection = this.createDeferredSelection(this.data); + + // act + this.dataSource.filter(['age', '>', 15]); + selection.selectAll(); + selection.changeItemSelection(1, { control: true }); // deselect item + selection.changeItemSelection(2, { control: true }); // deselect item + selection.changeItemSelection(1, { control: true }); // select item + selection.changeItemSelection(2, { control: true }); // select item + selection.changeItemSelection(2, { control: true }); // deselect item + selection.selectAll(); + selection.changeItemSelection(1, { control: true }); // deselect item + + // assert + assert.deepEqual(selection.selectionFilter(), [['age', '>', 15], 'and', ['!', ['id', '=', 2]]], 'selection filter'); + assert.strictEqual(selection.getSelectAllState(), undefined, 'select all is true'); + selection.getSelectedItemKeys().done(function(keys) { + selectedKeys = keys; + }); + assert.deepEqual(selectedKeys, [3, 4, 5, 6, 7], 'selected keys'); +}); + + QUnit.test('Deselect one item after selectAll', function(assert) { const selection = this.createDeferredSelection(this.data); @@ -1720,6 +1767,7 @@ QUnit.test('select 3 items, deselect 3 items', function(assert) { assert.deepEqual(selection.selectionFilter(), [], 'selectionFilter is []'); }); +// T874992 QUnit.test('select 2 items, deselect item', function(assert) { let selectedKeys; const selection = this.createDeferredSelection(this.data); @@ -1735,7 +1783,52 @@ QUnit.test('select 2 items, deselect item', function(assert) { // assert assert.deepEqual(selectedKeys, [1], 'selected keys'); - assert.deepEqual(selection.selectionFilter(), [['id', '=', 1], 'and', ['!', ['id', '=', 2]]], 'selectionFilter'); + assert.deepEqual(selection.selectionFilter(), ['id', '=', 1], 'selectionFilter'); +}); + +// T874992 +QUnit.test('select 3 items, deselect 2 item, select item', function(assert) { + let selectedKeys; + const selection = this.createDeferredSelection(this.data); + + selection.changeItemSelection(0, { control: true }); + selection.changeItemSelection(1, { control: true }); + selection.changeItemSelection(2, { control: true }); + selection.changeItemSelection(0, { control: true }); + selection.changeItemSelection(1, { control: true }); + selection.changeItemSelection(3, { control: true }); + + // act + selection.getSelectedItemKeys().done(function(keys) { + selectedKeys = keys; + }); + + // assert + assert.deepEqual(selectedKeys, [3, 4], 'selected keys'); + assert.deepEqual(selection.selectionFilter(), [['id', '=', 3], 'or', ['id', '=', 4]], 'selectionFilter'); +}); + +// T874992 +QUnit.test('select all, deselect 3 items, select 2 item, dselect item', function(assert) { + let selectedKeys; + const selection = this.createDeferredSelection(this.data); + + selection.selectAll(); + selection.changeItemSelection(0, { control: true }); + selection.changeItemSelection(1, { control: true }); + selection.changeItemSelection(2, { control: true }); + selection.changeItemSelection(0, { control: true }); + selection.changeItemSelection(1, { control: true }); + selection.changeItemSelection(3, { control: true }); + + // act + selection.getSelectedItemKeys().done(function(keys) { + selectedKeys = keys; + }); + + // assert + assert.deepEqual(selectedKeys, [1, 2, 5, 6, 7], 'selected keys'); + assert.deepEqual(selection.selectionFilter(), [['!', ['id', '=', 3]], 'and', ['!', ['id', '=', 4]]], 'selectionFilter'); }); QUnit.test('select 2 items, deselect and select 1 item', function(assert) { From 08fc71f4617ae3dbb1d0bd73d41744af2a592cc9 Mon Sep 17 00:00:00 2001 From: volnyagin Date: Mon, 20 Apr 2020 11:02:49 +0300 Subject: [PATCH 2/2] Rename after review --- js/ui/selection/selection.strategy.deferred.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/js/ui/selection/selection.strategy.deferred.js b/js/ui/selection/selection.strategy.deferred.js index b1d3de9a2d28..51cb5444c27f 100644 --- a/js/ui/selection/selection.strategy.deferred.js +++ b/js/ui/selection/selection.strategy.deferred.js @@ -165,7 +165,7 @@ module.exports = SelectionStrategy.inherit({ if(selectionFilter && selectionFilter.length) { that._removeSameFilter(selectionFilter, filter, isDeselect, isSelectAll); const filterIndex = that._removeSameFilter(selectionFilter, filter, !isDeselect); - const isKeyOperatorsAfterRemoved = this._isKeyFilter(filter) && this._isKeyOperatorsFromIndex(selectionFilter, filterIndex); + const isKeyOperatorsAfterRemoved = this._isKeyFilter(filter) && this._hasKeyFiltersOnlyStartingFromIndex(selectionFilter, filterIndex); needAddFilter = filter.length && !isKeyOperatorsAfterRemoved; @@ -229,7 +229,7 @@ module.exports = SelectionStrategy.inherit({ return this._isSimpleKeyFilter(filter, keyField); }, - _isKeyOperatorsFromIndex: function(selectionFilter, filterIndex) { + _hasKeyFiltersOnlyStartingFromIndex: function(selectionFilter, filterIndex) { if(filterIndex >= 0) { for(let i = filterIndex; i < selectionFilter.length; i++) { if(typeof selectionFilter[i] !== 'string' && !this._isKeyFilter(selectionFilter[i])) {