diff --git a/src/sap.m/src/sap/m/plugins/CellSelector.js b/src/sap.m/src/sap/m/plugins/CellSelector.js index c6214ada0560..4ee6c11ffb1c 100644 --- a/src/sap.m/src/sap/m/plugins/CellSelector.js +++ b/src/sap.m/src/sap/m/plugins/CellSelector.js @@ -155,7 +155,10 @@ sap.ui.define([ } oEvent.preventDefault(); - } else if (this._bSelecting && isKeyCombination(oEvent, KeyCodes.SPACE, false, true)) { + } + /* + Deactivate as feature did not work anyways and will be handled in separate BLI + else if (this._bSelecting && isKeyCombination(oEvent, KeyCodes.SPACE, false, true)) { if (!this._inSelection(oEvent.target)) { // If focus is on cell outside of selection, select focused column var oInfo = this.getConfig("getCellInfo", this.getControl(), oEvent.target); @@ -167,6 +170,7 @@ sap.ui.define([ oEvent.preventDefault(); } + */ }, onmousedown: function(oEvent) { if (oEvent.isMarked?.() || oEvent.button != 0) { @@ -203,6 +207,10 @@ sap.ui.define([ this._scrollSelect(this._oSession.scrollForward, this._oSession.isVertical, oEvent); } else { this._selectCells(); + if (!this._oSession.mSource || !this._oSession.mTarget) { + return; + } + this._drawSelection(this._oSession.mSource, this._oSession.mTarget); } }.bind(this); this._fnOnMouseEnter = this._onmouseenter.bind(this); @@ -301,7 +309,7 @@ sap.ui.define([ return null; } - var mSelectionRange = this._getNormalizedBounds(this._oSession.mSource, this._oSession.mTarget, true); + var mSelectionRange = this._getNormalizedBounds(this._oSession.mSource, this._oSession.mTarget); if (isNaN(mSelectionRange.from.rowIndex) || isNaN(mSelectionRange.to.rowIndex)) { return null; } @@ -600,26 +608,21 @@ sap.ui.define([ mFrom = mFrom ? mFrom : this._oSession.mSource; mTo = mTo ? mTo : this._oSession.mTarget; - var mBounds = this._getNormalizedBounds(mFrom, mTo); if (mTo.rowIndex == Infinity || mFrom.rowIndex == Infinity) { - this.getConfig("loadContexts", this.getControl(), mBounds.from.rowIndex, this.getRangeLimit()); + this.getConfig("loadContexts", this.getControl(), Math.max(Math.min(mFrom, mTo), 0), this.getRangeLimit()); } - this._drawSelection(mBounds); + this._drawSelection(mFrom, mTo); this._oSession.mSource = mFrom; this._oSession.mTarget = mTo; }; - /** - * Draws the selection for the given bounds. - * @param {Object} mBounds object containing the bounds information (from, to) - * @param {Object} mBounds.from from position - * @param {Object} mBounds.to to position - * @private - */ - CellSelector.prototype._drawSelection = function (mBounds) { + CellSelector.prototype._drawSelection = function (mFrom, mTo) { + const bAdjustBounds = !isFinite(mFrom.rowIndex) || !isFinite(mTo.rowIndex); + const mBounds = this._getNormalizedBounds(mFrom, mTo, bAdjustBounds); + if (!mBounds.from || !mBounds.to) { return; } @@ -765,23 +768,20 @@ sap.ui.define([ /** * Returns an object containing normalized coordinates for the given bounding area. - * from will contain the coordinates for the upper left corner of the bounding area, - * while to contains the coordinates of the lower right corner of the bounding area. - * @param {Object} mFrom - * @param {int} mFrom.rowIndex row index - * @param {int} mFrom.colIndex column index - * @param {Object} mTo - * @param {int} mTo.rowIndex row index - * @param {int} mTo.colIndex column index - * @returns object containing coordinates for from and to + * from contains the coordinates for the upper left corner of the bounding area, + * to contains the coordinates of the lower right corner of the bounding area. + * @param {Object} mFrom Source cell coordinates + * @param {Object} mTo Target cell coordinates + * @param {boolean} bAdjustBounds bounds are adjusted to fit into limit/table boundaries (e.g. range selection) + * @returns {object} Object containing coordinates for the bounding area */ - CellSelector.prototype._getNormalizedBounds = function(mFrom, mTo, bKeepBounds) { - const iMaxColumns = this.getConfig("getVisibleColumns", this.getControl()).length; + CellSelector.prototype._getNormalizedBounds = function(mFrom, mTo, bAdjustBounds) { + const iMaxColumns = this.getConfig("numberOfColumns", this.getControl()); const iMaxRows = this.getRangeLimit() == 0 ? this.getConfig("getRowCount", this.getControl()) : this.getRangeLimit(); let toRowIndex = Math.max(mFrom.rowIndex, mTo.rowIndex), toColIndex = Math.max(mFrom.colIndex, mTo.colIndex); - if (!bKeepBounds) { - toRowIndex = Math.min(iMaxRows - 1, toRowIndex); + if (bAdjustBounds) { + toRowIndex = Math.min(iMaxRows, toRowIndex); toColIndex = Math.min(iMaxColumns, toColIndex); } diff --git a/src/sap.m/test/sap/m/qunit/plugins/CellSelector.qunit.js b/src/sap.m/test/sap/m/qunit/plugins/CellSelector.qunit.js index 015edc3f3873..d1e7b67c5752 100644 --- a/src/sap.m/test/sap/m/qunit/plugins/CellSelector.qunit.js +++ b/src/sap.m/test/sap/m/qunit/plugins/CellSelector.qunit.js @@ -71,26 +71,42 @@ sap.ui.define([ this.oTable.attachEventOnce("rowsUpdated", () => { var oBinding = this.oTable.getBinding("rows"); - var oGetContextsSpy = sinon.spy(oBinding, "getContexts"); + // var oGetContextsSpy = sinon.spy(oBinding, "getContexts"); assert.ok(oBinding.getLength() > this.oCellSelector.getRangeLimit()); var oCell = getCell(this.oTable, 1, 0); // first cell of first row qutils.triggerKeydown(oCell, KeyCodes.SPACE); // select first cell of first row assert.equal(oBinding.getAllCurrentContexts().length, this.oTable.getThreshold() + this.oTable.getRowMode().getRowCount()); - qutils.triggerKeyup(oCell, KeyCodes.SPACE, false, false, true /* Ctrl */); // enlarge selection to all rows and cells - assert.equal(oGetContextsSpy.callCount, 1); - assert.ok(oGetContextsSpy.calledWithExactly(0, this.oCellSelector.getRangeLimit(), 0, true)); - assert.deepEqual(this.oCellSelector.getSelectionRange(), {from: {rowIndex: 0, colIndex: 0}, to: {rowIndex: Infinity, colIndex: 0}}); + // qutils.triggerKeyup(oCell, KeyCodes.SPACE, false, false, true /* Ctrl */); // enlarge selection to all rows and cells + // assert.equal(oGetContextsSpy.callCount, 1); + // assert.ok(oGetContextsSpy.calledWithExactly(0, this.oCellSelector.getRangeLimit(), 0, true)); + // assert.deepEqual(this.oCellSelector.getSelectionRange(), {from: {rowIndex: 0, colIndex: 0}, to: {rowIndex: Infinity, colIndex: 0}}); - oBinding.attachEventOnce("dataReceived", setTimeout.bind(0, () => { - assert.equal(oBinding.getAllCurrentContexts().length, this.oCellSelector.getRangeLimit()); - assert.equal(this.oCellSelector.getSelectedRowContexts().length, this.oCellSelector.getRangeLimit()); - assert.deepEqual(this.oCellSelector.getSelectedRowContexts(), oBinding.getAllCurrentContexts().slice(0, this.oCellSelector.getRangeLimit())); + qutils.triggerKeydown(oCell, KeyCodes.ARROW_RIGHT, true, false, false); + qutils.triggerKeyup(oCell, KeyCodes.ARROW_RIGHT, true, false, false); - oGetContextsSpy.restore(); - done(); - })); + oCell = getCell(this.oTable, 1, 1); + qutils.triggerKeydown(oCell, KeyCodes.ARROW_DOWN, true, false, false); + qutils.triggerKeyup(oCell, KeyCodes.ARROW_DOWN, true, false, false); + + // Commenting as Column Selection feature will be changed/adjusted in a separate BLI + // qutils.triggerKeyup(oCell, KeyCodes.SPACE, false, false, true /* Ctrl */); // enlarge selection to all rows and cells + // assert.equal(oGetContextsSpy.callCount, 1); + // assert.ok(oGetContextsSpy.calledWithExactly(0, this.oCellSelector.getRangeLimit(), 0, true)); + + assert.deepEqual(this.oCellSelector.getSelectionRange(), {from: {rowIndex: 1, colIndex: 0}, to: {rowIndex: 2, colIndex: 1}}); + assert.deepEqual(this.oCellSelector.getSelectedRowContexts(), oBinding.getAllCurrentContexts().slice(1, 3)); + + // Commenting as Column Selection feature will be changed/adjusted in a separate BLI + // oBinding.attachEventOnce("dataReceived", () => { + // assert.equal(oBinding.getAllCurrentContexts().length, this.oCellSelector.getRangeLimit()); + // assert.equal(this.oCellSelector.getSelectedRowContexts().length, this.oCellSelector.getRangeLimit()); + // assert.deepEqual(this.oCellSelector.getSelectedRowContexts(), oBinding.getAllCurrentContexts().slice(0, this.oCellSelector.getRangeLimit())); + // assert.deepEqual(this.oCellSelector.getSelectedRowContexts(), oBinding.getAllCurrentContexts().slice(1, 2)); + + // oGetContextsSpy.restore(); + done(); }); }); diff --git a/src/sap.m/test/sap/m/qunit/plugins/opa/CellSelector/test/CellSelectorOPA.qunit.js b/src/sap.m/test/sap/m/qunit/plugins/opa/CellSelector/test/CellSelectorOPA.qunit.js index 7d46af13bee8..43c53c70551f 100644 --- a/src/sap.m/test/sap/m/qunit/plugins/opa/CellSelector/test/CellSelectorOPA.qunit.js +++ b/src/sap.m/test/sap/m/qunit/plugins/opa/CellSelector/test/CellSelectorOPA.qunit.js @@ -312,20 +312,6 @@ sap.ui.define([ When.Keyboard.iRemoveSelection(true); Then.iSeeCellsSelected(); - // Trying to clear with 2x CTRL + A with no Select All => nothing happens - selectBlock(); - When.iFocusCell(1, 3); - When.Keyboard.iSelectAll(); - Then.iSeeCellsSelected({ rowIndex: 1, colIndex: 1 }, { rowIndex: 1, colIndex: 3 }); - Then.iSeeRowsSelected(); - - When.Keyboard.iSelectAll(); - Then.iSeeCellsSelected({ rowIndex: 1, colIndex: 1 }, { rowIndex: 1, colIndex: 3 }); - Then.iSeeRowsSelected(); - - When.Keyboard.iRemoveSelection(true); - Then.iSeeCellsSelected(); - // Trying to clear with 2x CTRL + A with Select All => clears selection, also clears cells Given.iChangeSelectAllState(true); @@ -344,6 +330,8 @@ sap.ui.define([ Given.iChangeSelectAllState(false); }); + /* + Commenting as Column Selection feature will be changed/adjusted in a separate BLI opaTest("Column Selection", function(Given, When, Then) { let rangeLimit = 200; // Default range limit @@ -354,7 +342,7 @@ sap.ui.define([ Then.iSeeCellFocused({ rowIndex: 1, colIndex: 1 }); When.Keyboard.iSelectColumns(); - Then.iSeeCellsSelected({ rowIndex: 0, colIndex: 1 }, { rowIndex: rangeLimit - 1, colIndex: 1 }); + Then.iSeeCellsSelected({ rowIndex: 0, colIndex: 1 }, { rowIndex: rangeLimit, colIndex: 1 }); When.Keyboard.iRemoveSelection(); Then.iSeeCellsSelected(); @@ -370,7 +358,7 @@ sap.ui.define([ Then.iSeeCellFocused({ rowIndex: 1, colIndex: 1 }); When.Keyboard.iSelectColumns(); - Then.iSeeCellsSelected({ rowIndex: 0, colIndex: 1 }, { rowIndex: rangeLimit - 1, colIndex: 1 }); + Then.iSeeCellsSelected({ rowIndex: 0, colIndex: 1 }, { rowIndex: rangeLimit, colIndex: 1 }); When.Keyboard.iRemoveSelection(); Then.iSeeCellsSelected(); @@ -388,17 +376,25 @@ sap.ui.define([ Then.iSeeCellFocused({ rowIndex: 1, colIndex: 3 }); When.Keyboard.iSelectColumns(); - Then.iSeeCellsSelected({ rowIndex: 0, colIndex: 1 }, { rowIndex: rangeLimit - 1, colIndex: 3 }); + Then.iSeeCellsSelected({ rowIndex: 0, colIndex: 1 }, { rowIndex: rangeLimit, colIndex: 3 }); Then.iSeeCellFocused({ rowIndex: 1, colIndex: 3}); // Extend Column Selection When.Keyboard.iSelectNextCell(true, !oConfig.forward); - Then.iSeeCellsSelected({ rowIndex: 0, colIndex: 1 }, { rowIndex: rangeLimit - 1, colIndex: 2 }); + Then.iSeeCellsSelected({ rowIndex: 0, colIndex: 1 }, { rowIndex: rangeLimit, colIndex: 2 }); Then.iSeeCellFocused({ rowIndex: 1, colIndex: 2}); When.Keyboard.iRemoveSelection(); Then.iSeeCellsSelected(); + // Select cell outside of range limit + When.iFocusCell(7, 1); + When.Keyboard.iSelectDeselectCell(); + Then.iSeeCellsSelected({ rowIndex: 7, colIndex: 1 }, { rowIndex: 7, colIndex: 1 }); + + When.Keyboard.iRemoveSelection(); + Then.iSeeCellsSelected(); + // Select column, then focus cell that is not part of selection -> columns of focused cell should be selected When.iFocusCell(1, 1); @@ -412,18 +408,19 @@ sap.ui.define([ Then.iSeeCellFocused({ rowIndex: 1, colIndex: 3 }); When.Keyboard.iSelectColumns(); - Then.iSeeCellsSelected({ rowIndex: 0, colIndex: 1 }, { rowIndex: rangeLimit - 1, colIndex: 3 }); + Then.iSeeCellsSelected({ rowIndex: 0, colIndex: 1 }, { rowIndex: rangeLimit, colIndex: 3 }); Then.iSeeCellFocused({ rowIndex: 1, colIndex: 3}); When.iFocusCell(4, 4); When.Keyboard.iSelectColumns(); - Then.iSeeCellsSelected({ rowIndex: 0, colIndex: 4 }, { rowIndex: rangeLimit - 1, colIndex: 4 }); + Then.iSeeCellsSelected({ rowIndex: 0, colIndex: 4 }, { rowIndex: rangeLimit, colIndex: 4 }); When.Keyboard.iRemoveSelection(); Then.iSeeCellsSelected(); Given.iChangeRangeLimit(200); }); + */ opaTest("Row- and Cell Selection interaction", function(Given, When, Then) { // No selection at all => cells should be selected