From 245d39bf3448f126855eda64e58222a1504445b5 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Thu, 2 Feb 2023 10:24:39 +0200 Subject: [PATCH 01/10] fix: use the same cell reference constructor in order to ensure consistency --- .../tests/fixtures/PopupButtonFixture.java | 5 +- .../spreadsheet/CellSelectionManager.java | 59 ++++++++++++------- .../spreadsheet/CellSelectionShifter.java | 8 ++- .../spreadsheet/CellValueManager.java | 4 +- .../component/spreadsheet/Spreadsheet.java | 16 +++-- .../spreadsheet/SpreadsheetHandlerImpl.java | 4 +- .../command/CellShiftValuesCommand.java | 6 +- .../spreadsheet/command/CellValueCommand.java | 11 ++-- .../command/RowInsertOrDeleteCommand.java | 3 +- 9 files changed, 77 insertions(+), 39 deletions(-) diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow-integration-tests/src/main/java/com/vaadin/flow/component/spreadsheet/tests/fixtures/PopupButtonFixture.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow-integration-tests/src/main/java/com/vaadin/flow/component/spreadsheet/tests/fixtures/PopupButtonFixture.java index a78afcbecec..a3985d65a56 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow-integration-tests/src/main/java/com/vaadin/flow/component/spreadsheet/tests/fixtures/PopupButtonFixture.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow-integration-tests/src/main/java/com/vaadin/flow/component/spreadsheet/tests/fixtures/PopupButtonFixture.java @@ -26,8 +26,9 @@ public void loadFixture(final Spreadsheet spreadsheet) { } List values = new ArrayList<>(VALUES); CellReference ref = event.getSelectedCellReference(); - CellReference newRef = new CellReference(ref.getRow(), - ref.getCol()); + CellReference newRef = new CellReference( + spreadsheet.getActiveSheet().getSheetName(), ref.getRow(), + ref.getCol(), false, false); DataValidationButton popupButton = new DataValidationButton( spreadsheet, values); popupButton.setUp(); diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellSelectionManager.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellSelectionManager.java index 1a62df278d2..269e2c9ce3a 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellSelectionManager.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellSelectionManager.java @@ -116,7 +116,9 @@ public SelectionChangeEvent getLatestSelectionEvent() { } boolean isCellInsideSelection(int row, int column) { - CellReference cellReference = new CellReference(row - 1, column - 1); + CellReference cellReference = new CellReference( + spreadsheet.getActiveSheet().getSheetName(), row - 1, + column - 1, false, false); boolean inside = cellReference.equals(selectedCellReference) || individualSelectedCells.contains(cellReference); if (!inside) { @@ -173,7 +175,9 @@ protected void reloadCurrentSelection() { */ protected void onCellSelected(int row, int column, boolean discardOldRangeSelection) { - CellReference cellReference = new CellReference(row - 1, column - 1); + CellReference cellReference = new CellReference( + spreadsheet.getActiveSheet().getSheetName(), row - 1, + column - 1, false, false); CellReference previousCellReference = selectedCellReference; if (!cellReference.equals(previousCellReference) || discardOldRangeSelection && (!cellRangeAddresses.isEmpty() @@ -217,8 +221,9 @@ protected void onSheetAddressChanged(String value, region.col1 - 1, region.col2 - 1); } handleCellRangeSelection(cra); - selectedCellReference = new CellReference(cra.getFirstRow(), - cra.getFirstColumn()); + selectedCellReference = new CellReference( + spreadsheet.getActiveSheet().getSheetName(), + cra.getFirstRow(), cra.getFirstColumn(), false, false); paintedCellRange = cra; cellRangeAddresses.clear(); cellRangeAddresses.add(cra); @@ -234,8 +239,10 @@ protected void onSheetAddressChanged(String value, .createCorrectCellRangeAddress(region.row1, region.col1, region.row2, region.col2); handleCellRangeSelection(cra); - selectedCellReference = new CellReference(cra.getFirstRow(), - cra.getFirstColumn()); + selectedCellReference = new CellReference( + spreadsheet.getActiveSheet().getSheetName(), + cra.getFirstRow(), cra.getFirstColumn(), false, + false); paintedCellRange = cra; cellRangeAddresses.clear(); cellRangeAddresses.add(cra); @@ -382,8 +389,9 @@ protected void handleCellRangeSelection(CellRangeAddress cra) { protected void handleCellRangeSelection(String name, CellRangeAddress cra) { - final CellReference firstCell = new CellReference(cra.getFirstRow(), - cra.getFirstColumn()); + final CellReference firstCell = new CellReference( + spreadsheet.getActiveSheet().getSheetName(), cra.getFirstRow(), + cra.getFirstColumn(), false, false); handleCellRangeSelection(name, firstCell, cra, true); } @@ -463,8 +471,9 @@ protected void onCellRangePainted(int selectedCellRow, cellRangeAddresses.clear(); individualSelectedCells.clear(); - selectedCellReference = new CellReference(selectedCellRow - 1, - selectedCellColumn - 1); + selectedCellReference = new CellReference( + spreadsheet.getActiveSheet().getSheetName(), + selectedCellRow - 1, selectedCellColumn - 1, false, false); CellRangeAddress cra = spreadsheet.createCorrectCellRangeAddress(row1, col1, row2, col2); @@ -507,7 +516,9 @@ protected void onCellAddToSelectionAndSelected(int row, int column) { individualSelectedCells.add(selectedCellReference); } handleCellSelection(row, column); - selectedCellReference = new CellReference(row - 1, column - 1); + selectedCellReference = new CellReference( + spreadsheet.getActiveSheet().getSheetName(), row - 1, + column - 1, false, false); spreadsheet.loadCustomEditorOnSelectedCell(); if (individualSelectedCells.contains(selectedCellReference)) { individualSelectedCells.remove( @@ -558,8 +569,9 @@ protected void onCellsAddedToRangeSelection(int row1, int col1, int row2, */ protected void onRowSelected(int row, int firstColumnIndex) { handleCellSelection(row, firstColumnIndex); - selectedCellReference = new CellReference(row - 1, - firstColumnIndex - 1); + selectedCellReference = new CellReference( + spreadsheet.getActiveSheet().getSheetName(), row - 1, + firstColumnIndex - 1, false, false); spreadsheet.loadCustomEditorOnSelectedCell(); cellRangeAddresses.clear(); individualSelectedCells.clear(); @@ -592,8 +604,9 @@ protected void onRowAddedToRangeSelection(int row, int firstColumnIndex) { individualSelectedCells.add(selectedCellReference); } handleCellSelection(row, firstColumnIndex); - selectedCellReference = new CellReference(row - 1, - firstColumnIndex - 1); + selectedCellReference = new CellReference( + spreadsheet.getActiveSheet().getSheetName(), row - 1, + firstColumnIndex - 1, false, false); spreadsheet.loadCustomEditorOnSelectedCell(); cellRangeAddresses.add(spreadsheet.createCorrectCellRangeAddress(row, 1, row, spreadsheet.getColumns())); @@ -612,8 +625,9 @@ protected void onRowAddedToRangeSelection(int row, int firstColumnIndex) { */ protected void onColumnSelected(int firstRowIndex, int column) { handleCellSelection(firstRowIndex, column); - selectedCellReference = new CellReference(firstRowIndex - 1, - column - 1); + selectedCellReference = new CellReference( + spreadsheet.getActiveSheet().getSheetName(), firstRowIndex - 1, + column - 1, false, false); spreadsheet.loadCustomEditorOnSelectedCell(); cellRangeAddresses.clear(); individualSelectedCells.clear(); @@ -646,8 +660,9 @@ protected void onColumnAddedToSelection(int firstRowIndex, int column) { individualSelectedCells.add(selectedCellReference); } handleCellSelection(firstRowIndex, column); - selectedCellReference = new CellReference(firstRowIndex - 1, - column - 1); + selectedCellReference = new CellReference( + spreadsheet.getActiveSheet().getSheetName(), firstRowIndex - 1, + column - 1, false, false); spreadsheet.loadCustomEditorOnSelectedCell(); cellRangeAddresses.add(spreadsheet.createCorrectCellRangeAddress(1, column, spreadsheet.getRows(), column)); @@ -676,8 +691,10 @@ protected void mergedRegionAdded(CellRangeAddress region) { handleCellAddressChange(region.getFirstRow() + 1, region.getFirstColumn() + 1, false); } - selectedCellReference = new CellReference(region.getFirstRow(), - region.getFirstColumn()); + selectedCellReference = new CellReference( + spreadsheet.getActiveSheet().getSheetName(), + region.getFirstRow(), region.getFirstColumn(), false, + false); fire = true; } for (Iterator i = cellRangeAddresses.iterator(); i diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellSelectionShifter.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellSelectionShifter.java index 1da9d750c24..e191c38dfaf 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellSelectionShifter.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellSelectionShifter.java @@ -141,7 +141,9 @@ private void fireCellValueChangeEvent(CellRangeAddress region) { for (int x = region.getFirstColumn(); x <= region .getLastColumn(); x++) { for (int y = region.getFirstRow(); y <= region.getLastRow(); y++) { - cells.add(new CellReference(y, x)); + cells.add(new CellReference( + spreadsheet.getActiveSheet().getSheetName(), y, x, + false, false)); } } spreadsheet.fireEvent(new CellValueChangeEvent(spreadsheet, cells)); @@ -399,8 +401,10 @@ public void onSelectionDecreasePainted(int r, int c) { if (!SpreadsheetUtil.isCellInRange(selectedCellReference, newPaintedCellRange)) { selectedCellReference = new CellReference( + spreadsheet.getActiveSheet().getSheetName(), newPaintedCellRange.getFirstRow(), - newPaintedCellRange.getFirstColumn()); + newPaintedCellRange.getFirstColumn(), false, + false); } getCellSelectionManager().handleCellRangeSelection( selectedCellReference, newPaintedCellRange, false); diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellValueManager.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellValueManager.java index a0824104c57..6f4b98eaf36 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellValueManager.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellValueManager.java @@ -602,7 +602,9 @@ public void onCellValueChange(int col, int row, String value) { // capture cell value to history CellValueCommand command = new CellValueCommand(spreadsheet); - command.captureCellValues(new CellReference(row - 1, col - 1)); + command.captureCellValues( + new CellReference(spreadsheet.getActiveSheet().getSheetName(), + row - 1, col - 1, false, false)); spreadsheet.getSpreadsheetHistoryManager().addCommand(command); boolean updateHyperlinks = false; boolean formulaChanged = false; diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/Spreadsheet.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/Spreadsheet.java index 09782c87760..8065b5473c3 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/Spreadsheet.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/Spreadsheet.java @@ -643,7 +643,8 @@ void setNamedRanges(List namedRanges) { void onPopupButtonClick(int row, int column) { PopupButton popup = sheetPopupButtons .get(SpreadsheetUtil.relativeToAbsolute(this, - new CellReference(row - 1, column - 1))); + new CellReference(getActiveSheet().getSheetName(), + row - 1, column - 1, false, false))); if (popup != null) { popup.openPopup(); } @@ -652,7 +653,8 @@ void onPopupButtonClick(int row, int column) { void onPopupClose(int row, int column) { PopupButton popup = sheetPopupButtons .get(SpreadsheetUtil.relativeToAbsolute(this, - new CellReference(row - 1, column - 1))); + new CellReference(getActiveSheet().getSheetName(), + row - 1, column - 1, false, false))); if (popup != null) { popup.closePopup(); @@ -2922,7 +2924,8 @@ private void rowsMoved(int first, int last, int n) { } else if (numberOfRowsAboveWasChanged(row, last, first)) { int newRow = cell.getRow() + n; int col = cell.getCol(); - CellReference newCell = new CellReference(newRow, col, true, + CellReference newCell = new CellReference( + getActiveSheet().getSheetName(), newRow, col, true, true); pbutton.setCellReference(newCell); updated.put(newCell, pbutton); @@ -4760,7 +4763,8 @@ public void setPopup(String cellAddress, PopupButton popupButton) { * removes the pop-up button for the target cell. */ public void setPopup(int row, int col, PopupButton popupButton) { - setPopup(new CellReference(row, col), popupButton); + setPopup(new CellReference(getActiveSheet().getSheetName(), row, col, + false, false), popupButton); } /** @@ -5184,7 +5188,9 @@ private static Set getAllSelectedCells( for (int x = a.getFirstColumn(); x <= a.getLastColumn(); x++) { for (int y = a.getFirstRow(); y <= a.getLastRow(); y++) { - cells.add(new CellReference(y, x)); + cells.add(new CellReference( + selectedCellReference.getSheetName(), y, x, + false, false)); } } } diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/SpreadsheetHandlerImpl.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/SpreadsheetHandlerImpl.java index 91e77ba7ca5..3062ccf7eef 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/SpreadsheetHandlerImpl.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/SpreadsheetHandlerImpl.java @@ -334,7 +334,9 @@ private void fireCellValueChangeEvent(CellRangeAddress region) { for (int x = region.getFirstColumn(); x <= region .getLastColumn(); x++) { for (int y = region.getFirstRow(); y <= region.getLastRow(); y++) { - cells.add(new CellReference(y, x)); + cells.add(new CellReference( + spreadsheet.getActiveSheet().getSheetName(), y, x, + false, false)); } } fireCellValueChangeEvent(cells); diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/CellShiftValuesCommand.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/CellShiftValuesCommand.java index 2fac89c4192..7296db6f849 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/CellShiftValuesCommand.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/CellShiftValuesCommand.java @@ -52,8 +52,10 @@ public CellReference getSelectedCellReference() { .isCellInRange(selectedCellReference, paintedCellRange)) { return selectedCellReference; } else { - return new CellReference(paintedCellRange.getFirstRow(), - paintedCellRange.getFirstColumn()); + return new CellReference( + spreadsheet.getActiveSheet().getSheetName(), + paintedCellRange.getFirstRow(), + paintedCellRange.getFirstColumn(), false, false); } } diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/CellValueCommand.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/CellValueCommand.java index 95fa46e858d..949b875ecc9 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/CellValueCommand.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/CellValueCommand.java @@ -148,7 +148,8 @@ public void captureCellRangeValues(CellRangeAddress... cellRanges) { @Override public CellReference getSelectedCellReference() { - return new CellReference(selectedCellRow, selectedcellCol); + return new CellReference(spreadsheet.getActiveSheet().getSheetName(), + selectedCellRow, selectedcellCol, false, false); } @Override @@ -339,13 +340,15 @@ public Set getChangedCells() { for (Object o : values) { if (o instanceof CellValue) { CellValue cellValue = (CellValue) o; - changedCells - .add(new CellReference(cellValue.row, cellValue.col)); + changedCells.add(new CellReference( + spreadsheet.getActiveSheet().getSheetName(), + cellValue.row, cellValue.col, false, false)); } else { CellRangeValue cellRangeValue = (CellRangeValue) o; for (int r = cellRangeValue.row1; r <= cellRangeValue.row2; r++) { for (int c = cellRangeValue.col1; c <= cellRangeValue.col2; c++) { - changedCells.add(new CellReference(r, c)); + changedCells.add(new CellReference( + getSheet().getSheetName(), r, c, false, false)); } } } diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/RowInsertOrDeleteCommand.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/RowInsertOrDeleteCommand.java index 07af0d092ac..fa85b2c1c53 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/RowInsertOrDeleteCommand.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/RowInsertOrDeleteCommand.java @@ -44,7 +44,8 @@ public void execute() { @Override public CellReference getSelectedCellReference() { - return new CellReference(row, 0); + return new CellReference(spreadsheet.getActiveSheet().getSheetName(), + row, 0, false, false); } @Override From fe708df73138476aab14f4d733c5f3658da698d9 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Tue, 14 Feb 2023 08:45:53 +0200 Subject: [PATCH 02/10] fix: use getSheet from SpreadsheetCommand to get the sheet --- .../component/spreadsheet/command/CellShiftValuesCommand.java | 2 +- .../flow/component/spreadsheet/command/CellValueCommand.java | 4 ++-- .../spreadsheet/command/RowInsertOrDeleteCommand.java | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/CellShiftValuesCommand.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/CellShiftValuesCommand.java index 7296db6f849..5c8d03365f9 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/CellShiftValuesCommand.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/CellShiftValuesCommand.java @@ -53,7 +53,7 @@ public CellReference getSelectedCellReference() { return selectedCellReference; } else { return new CellReference( - spreadsheet.getActiveSheet().getSheetName(), + getSheet().getSheetName(), paintedCellRange.getFirstRow(), paintedCellRange.getFirstColumn(), false, false); } diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/CellValueCommand.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/CellValueCommand.java index 949b875ecc9..a095a43945f 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/CellValueCommand.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/CellValueCommand.java @@ -148,7 +148,7 @@ public void captureCellRangeValues(CellRangeAddress... cellRanges) { @Override public CellReference getSelectedCellReference() { - return new CellReference(spreadsheet.getActiveSheet().getSheetName(), + return new CellReference(getSheet().getSheetName(), selectedCellRow, selectedcellCol, false, false); } @@ -341,7 +341,7 @@ public Set getChangedCells() { if (o instanceof CellValue) { CellValue cellValue = (CellValue) o; changedCells.add(new CellReference( - spreadsheet.getActiveSheet().getSheetName(), + getSheet().getSheetName(), cellValue.row, cellValue.col, false, false)); } else { CellRangeValue cellRangeValue = (CellRangeValue) o; diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/RowInsertOrDeleteCommand.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/RowInsertOrDeleteCommand.java index fa85b2c1c53..6bebf55d1f6 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/RowInsertOrDeleteCommand.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/RowInsertOrDeleteCommand.java @@ -44,7 +44,7 @@ public void execute() { @Override public CellReference getSelectedCellReference() { - return new CellReference(spreadsheet.getActiveSheet().getSheetName(), + return new CellReference(getSheet().getSheetName(), row, 0, false, false); } From 6227ec5d6d4b878cd7bf28bf60e72b65747ed2b6 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Tue, 14 Feb 2023 08:47:08 +0200 Subject: [PATCH 03/10] fix: apply formatter --- .../spreadsheet/command/CellShiftValuesCommand.java | 3 +-- .../component/spreadsheet/command/CellValueCommand.java | 7 +++---- .../spreadsheet/command/RowInsertOrDeleteCommand.java | 4 ++-- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/CellShiftValuesCommand.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/CellShiftValuesCommand.java index 5c8d03365f9..85f9efa6bd6 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/CellShiftValuesCommand.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/CellShiftValuesCommand.java @@ -52,8 +52,7 @@ public CellReference getSelectedCellReference() { .isCellInRange(selectedCellReference, paintedCellRange)) { return selectedCellReference; } else { - return new CellReference( - getSheet().getSheetName(), + return new CellReference(getSheet().getSheetName(), paintedCellRange.getFirstRow(), paintedCellRange.getFirstColumn(), false, false); } diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/CellValueCommand.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/CellValueCommand.java index a095a43945f..c9c03e3ff5c 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/CellValueCommand.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/CellValueCommand.java @@ -148,8 +148,8 @@ public void captureCellRangeValues(CellRangeAddress... cellRanges) { @Override public CellReference getSelectedCellReference() { - return new CellReference(getSheet().getSheetName(), - selectedCellRow, selectedcellCol, false, false); + return new CellReference(getSheet().getSheetName(), selectedCellRow, + selectedcellCol, false, false); } @Override @@ -340,8 +340,7 @@ public Set getChangedCells() { for (Object o : values) { if (o instanceof CellValue) { CellValue cellValue = (CellValue) o; - changedCells.add(new CellReference( - getSheet().getSheetName(), + changedCells.add(new CellReference(getSheet().getSheetName(), cellValue.row, cellValue.col, false, false)); } else { CellRangeValue cellRangeValue = (CellRangeValue) o; diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/RowInsertOrDeleteCommand.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/RowInsertOrDeleteCommand.java index 6bebf55d1f6..55534d53238 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/RowInsertOrDeleteCommand.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/command/RowInsertOrDeleteCommand.java @@ -44,8 +44,8 @@ public void execute() { @Override public CellReference getSelectedCellReference() { - return new CellReference(getSheet().getSheetName(), - row, 0, false, false); + return new CellReference(getSheet().getSheetName(), row, 0, false, + false); } @Override From 268ff656b90e3d4d3ae7d19f76b327edea360fe7 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Tue, 14 Feb 2023 08:48:08 +0200 Subject: [PATCH 04/10] fix: add cell reference without sheet name to cell value change event --- .../flow/component/spreadsheet/CellValueManager.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellValueManager.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellValueManager.java index 6f4b98eaf36..a3fa8d2261f 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellValueManager.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellValueManager.java @@ -794,6 +794,13 @@ private void fireFormulaValueChangeEvent(Set changedCells) { } private void fireCellValueChangeEvent(Set changedCells) { + List cellRefsWithSheetName = changedCells.stream() + .filter(ref -> ref.getSheetName() != null).toList(); + cellRefsWithSheetName.forEach(ref -> { + CellReference refWithoutSheetName = new CellReference(ref.getRow(), + ref.getCol()); + changedCells.add(refWithoutSheetName); + }); spreadsheet .fireEvent(new CellValueChangeEvent(spreadsheet, changedCells)); } From 71053759804ca715fa433333f739877910364673 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Wed, 15 Feb 2023 09:30:48 +0200 Subject: [PATCH 05/10] fix: add missing cell reference without sheet name to event --- .../flow/component/spreadsheet/CellValueManager.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellValueManager.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellValueManager.java index a3fa8d2261f..b59d38eaf99 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellValueManager.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellValueManager.java @@ -784,7 +784,13 @@ private String getFormattedCellValue(Cell cell) { private void fireCellValueChangeEvent(Cell cell) { Set cells = new HashSet(); - cells.add(new CellReference(cell)); + CellReference ref = new CellReference(cell); + cells.add(ref); + if (ref.getSheetName() != null) { + CellReference refWithoutSheetName = new CellReference(ref.getRow(), + ref.getCol()); + cells.add(refWithoutSheetName); + } spreadsheet.fireEvent(new CellValueChangeEvent(spreadsheet, cells)); } From 31fa8aca805a2e393e39f0aef7c61ba419c9addd Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Wed, 15 Feb 2023 09:49:18 +0200 Subject: [PATCH 06/10] fix: update cell value change unit test --- .../tests/CellValueChangeEventOnFormulaChangeTest.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/test/java/com/vaadin/flow/component/spreadsheet/tests/CellValueChangeEventOnFormulaChangeTest.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/test/java/com/vaadin/flow/component/spreadsheet/tests/CellValueChangeEventOnFormulaChangeTest.java index c17fd85adb0..f67db454dd4 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/test/java/com/vaadin/flow/component/spreadsheet/tests/CellValueChangeEventOnFormulaChangeTest.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/test/java/com/vaadin/flow/component/spreadsheet/tests/CellValueChangeEventOnFormulaChangeTest.java @@ -1,6 +1,7 @@ package com.vaadin.flow.component.spreadsheet.tests; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import java.util.LinkedList; import java.util.List; @@ -51,9 +52,11 @@ public void formulaChangeResultingInSameValue() { // B1 is 0, so the result doesn't change spreadsheet.getCellValueManager().onCellValueChange(3, 1, "=A1+2*B1"); - assertEquals("There should be 1 changed cell", 1, changedCells.size()); - assertEquals("The changed cell should be C1", - new CellReference("Sheet0!C1"), changedCells.get(0)); + assertEquals("There should be 2 changed cells", 2, changedCells.size()); + assertTrue("The changed cells should include C1 with sheet name", + changedCells.contains(new CellReference("Sheet0!C1"))); + assertTrue("The changed cells should include C1 without sheet name", + changedCells.contains(new CellReference("C1"))); } } From 5476fdd1e9b39c11254320fe1283100037ad350d Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Mon, 27 Feb 2023 20:14:26 +0200 Subject: [PATCH 07/10] fix: add sheet name to more internal cell references --- .../component/spreadsheet/CellSelectionManager.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellSelectionManager.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellSelectionManager.java index 269e2c9ce3a..f2ba4fff3d1 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellSelectionManager.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellSelectionManager.java @@ -255,7 +255,11 @@ protected void onSheetAddressChanged(String value, cellReference.getCol() + 1, cellReference.getRow() + 1, cellReference.getCol() + 1); - selectedCellReference = cellReference; + final CellReference cellReferenceWithSheetName = new CellReference( + spreadsheet.getActiveSheet().getSheetName(), + cellReference.getRow(), cellReference.getCol(), + false, false); + selectedCellReference = cellReferenceWithSheetName; cellRangeAddresses.clear(); } } @@ -399,7 +403,12 @@ protected void handleCellRangeSelection(String name, CellRangeAddress cra) { protected void handleCellRangeSelection(CellReference startingPoint, CellRangeAddress cellsToSelect, boolean scroll) { - handleCellRangeSelection(null, startingPoint, cellsToSelect, scroll); + final CellReference startingPointWithSheetName = new CellReference( + spreadsheet.getActiveSheet().getSheetName(), + startingPoint.getRow(), startingPoint.getCol(), false, false); + + handleCellRangeSelection(null, startingPointWithSheetName, + cellsToSelect, scroll); } private void handleCellRangeSelection(String name, From 530e9e7953a7802bbb8a5d8494869403fa0f291f Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Mon, 3 Jul 2023 20:06:04 +0300 Subject: [PATCH 08/10] fix: introduce cellset to avoid selected cell duplication in event --- .../tests/fixtures/PopupButtonFixture.java | 2 +- .../flow/component/spreadsheet/CellSet.java | 36 +++++++++++++++++++ .../spreadsheet/CellValueManager.java | 14 +------- .../component/spreadsheet/Spreadsheet.java | 30 +++++++--------- .../xssfreader/AbstractSeriesReader.java | 2 +- ...llValueChangeEventOnFormulaChangeTest.java | 16 +++++---- .../spreadsheet/tests/FormulasTest.java | 6 ++-- 7 files changed, 63 insertions(+), 43 deletions(-) create mode 100644 vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellSet.java diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow-integration-tests/src/main/java/com/vaadin/flow/component/spreadsheet/tests/fixtures/PopupButtonFixture.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow-integration-tests/src/main/java/com/vaadin/flow/component/spreadsheet/tests/fixtures/PopupButtonFixture.java index a3985d65a56..38ca446f4c4 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow-integration-tests/src/main/java/com/vaadin/flow/component/spreadsheet/tests/fixtures/PopupButtonFixture.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow-integration-tests/src/main/java/com/vaadin/flow/component/spreadsheet/tests/fixtures/PopupButtonFixture.java @@ -21,7 +21,7 @@ public class PopupButtonFixture implements SpreadsheetFixture { @Override public void loadFixture(final Spreadsheet spreadsheet) { spreadsheet.addSelectionChangeListener(event -> { - if (event.getAllSelectedCells().size() != 1) { + if (event.getAllSelectedCells().getCellCount() != 1) { return; } List values = new ArrayList<>(VALUES); diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellSet.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellSet.java new file mode 100644 index 00000000000..7df6fae9fc8 --- /dev/null +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellSet.java @@ -0,0 +1,36 @@ +package com.vaadin.flow.component.spreadsheet; + +import org.apache.poi.ss.util.CellReference; + +import java.util.Collections; +import java.util.Set; + +public class CellSet { + + private final Set cells; + + public CellSet(Set cells) { + this.cells = cells; + } + + public Set getCells() { + return Collections.unmodifiableSet(cells); + } + + public int getCellCount() { + return cells.size(); + } + + public boolean containsCell(CellReference cell) { + if (cells.isEmpty()) { + return false; + } + if (cell.getSheetName() == null) { + CellReference cellWithSheetName = new CellReference( + cells.iterator().next().getSheetName(), cell.getRow(), + cell.getCol(), cell.isRowAbsolute(), cell.isColAbsolute()); + return cells.contains(cellWithSheetName); + } + return cells.contains(cell); + } +} diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellValueManager.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellValueManager.java index b59d38eaf99..d49b9e125df 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellValueManager.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellValueManager.java @@ -783,14 +783,9 @@ private String getFormattedCellValue(Cell cell) { } private void fireCellValueChangeEvent(Cell cell) { - Set cells = new HashSet(); + Set cells = new HashSet<>(); CellReference ref = new CellReference(cell); cells.add(ref); - if (ref.getSheetName() != null) { - CellReference refWithoutSheetName = new CellReference(ref.getRow(), - ref.getCol()); - cells.add(refWithoutSheetName); - } spreadsheet.fireEvent(new CellValueChangeEvent(spreadsheet, cells)); } @@ -800,13 +795,6 @@ private void fireFormulaValueChangeEvent(Set changedCells) { } private void fireCellValueChangeEvent(Set changedCells) { - List cellRefsWithSheetName = changedCells.stream() - .filter(ref -> ref.getSheetName() != null).toList(); - cellRefsWithSheetName.forEach(ref -> { - CellReference refWithoutSheetName = new CellReference(ref.getRow(), - ref.getCol()); - changedCells.add(refWithoutSheetName); - }); spreadsheet .fireEvent(new CellValueChangeEvent(spreadsheet, changedCells)); } diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/Spreadsheet.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/Spreadsheet.java index 3e6c2df04ed..c980ad7c75f 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/Spreadsheet.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/Spreadsheet.java @@ -2801,16 +2801,14 @@ public void shiftRows(int startRow, int endRow, int n, getHiddenRowIndexes()); if (row == null) { valueManager.updateDeletedRowsInClientCache(rowIndex, rowIndex); - if (_hiddenRowIndexes.contains(rowIndex)) { - _hiddenRowIndexes.remove(rowIndex); - } + _hiddenRowIndexes.remove(rowIndex); for (int c = 0; c < getCols(); c++) { styler.clearCellStyle(r, c); } } else { if (row.getZeroHeight()) { _hiddenRowIndexes.add(rowIndex); - } else if (_hiddenRowIndexes.contains(rowIndex)) { + } else { _hiddenRowIndexes.remove(rowIndex); } for (int c = 0; c < getCols(); c++) { @@ -5039,15 +5037,15 @@ public void setRowColHeadingsVisible(boolean visible) { */ public abstract static class ValueChangeEvent extends ComponentEvent { - private final Set changedCells; + private final CellSet changedCells; public ValueChangeEvent(Component source, Set changedCells) { super(source, false); - this.changedCells = changedCells; + this.changedCells = new CellSet(changedCells); } - public Set getChangedCells() { + public CellSet getChangedCells() { return changedCells; } } @@ -5171,10 +5169,10 @@ public List getCellRangeAddresses() { * @return A combination of all selected cells, regardless of selection * mode. Doesn't contain duplicates. */ - public Set getAllSelectedCells() { - return Spreadsheet.getAllSelectedCells(selectedCellReference, - individualSelectedCells, cellRangeAddresses); - + public CellSet getAllSelectedCells() { + return new CellSet( + Spreadsheet.getAllSelectedCells(selectedCellReference, + individualSelectedCells, cellRangeAddresses)); } } @@ -5182,10 +5180,7 @@ private static Set getAllSelectedCells( CellReference selectedCellReference, List individualSelectedCells, List cellRangeAddresses) { - Set cells = new HashSet(); - for (CellReference r : individualSelectedCells) { - cells.add(r); - } + Set cells = new HashSet<>(individualSelectedCells); cells.add(selectedCellReference); if (cellRangeAddresses != null) { @@ -5391,10 +5386,9 @@ public CellReference getSelectedCellReference() { public Set getSelectedCellReferences() { SelectionChangeEvent event = selectionManager.getLatestSelectionEvent(); if (event == null) { - return new HashSet(); - } else { - return event.getAllSelectedCells(); + return new HashSet<>(); } + return event.getAllSelectedCells().getCells(); } /** diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/charts/converter/xssfreader/AbstractSeriesReader.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/charts/converter/xssfreader/AbstractSeriesReader.java index 430b31b0e26..248a2b0024a 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/charts/converter/xssfreader/AbstractSeriesReader.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/charts/converter/xssfreader/AbstractSeriesReader.java @@ -293,7 +293,7 @@ void onValueChange(final List referencedCells, return; } - for (CellReference changedCell : event.getChangedCells()) { + for (CellReference changedCell : event.getChangedCells().getCells()) { // getChangedCell erroneously provides relative cell refs // if this gets fixed, this conversion method should be // removed diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/test/java/com/vaadin/flow/component/spreadsheet/tests/CellValueChangeEventOnFormulaChangeTest.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/test/java/com/vaadin/flow/component/spreadsheet/tests/CellValueChangeEventOnFormulaChangeTest.java index f67db454dd4..95f3b702943 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/test/java/com/vaadin/flow/component/spreadsheet/tests/CellValueChangeEventOnFormulaChangeTest.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/test/java/com/vaadin/flow/component/spreadsheet/tests/CellValueChangeEventOnFormulaChangeTest.java @@ -3,9 +3,9 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; -import java.util.LinkedList; -import java.util.List; +import java.util.concurrent.atomic.AtomicReference; +import com.vaadin.flow.component.spreadsheet.CellSet; import org.apache.poi.ss.usermodel.Row; import org.apache.poi.ss.usermodel.Sheet; import org.apache.poi.ss.usermodel.Workbook; @@ -43,20 +43,22 @@ public void setup() { */ @Test public void formulaChangeResultingInSameValue() { - List changedCells = new LinkedList<>(); + AtomicReference changedCells = new AtomicReference<>(); spreadsheet.addCellValueChangeListener( - event -> changedCells.addAll(event.getChangedCells())); + event -> changedCells.set(event.getChangedCells())); spreadsheet.setSelection("C1"); // B1 is 0, so the result doesn't change spreadsheet.getCellValueManager().onCellValueChange(3, 1, "=A1+2*B1"); - assertEquals("There should be 2 changed cells", 2, changedCells.size()); + assertEquals("There should be one changed cell", 1, + changedCells.get().getCellCount()); assertTrue("The changed cells should include C1 with sheet name", - changedCells.contains(new CellReference("Sheet0!C1"))); + changedCells.get() + .containsCell(new CellReference("Sheet0!C1"))); assertTrue("The changed cells should include C1 without sheet name", - changedCells.contains(new CellReference("C1"))); + changedCells.get().containsCell(new CellReference("C1"))); } } diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/test/java/com/vaadin/flow/component/spreadsheet/tests/FormulasTest.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/test/java/com/vaadin/flow/component/spreadsheet/tests/FormulasTest.java index c8f999bb2a9..16a9e1bba49 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/test/java/com/vaadin/flow/component/spreadsheet/tests/FormulasTest.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/test/java/com/vaadin/flow/component/spreadsheet/tests/FormulasTest.java @@ -64,9 +64,9 @@ public void formulaValueChangeListener_invokedOnFormulaValueChange() { spreadsheet.refreshCells(A1, A2); // Check that the event was fired with the correct values - Assert.assertEquals(event.get().getChangedCells().size(), 1); - Assert.assertEquals(event.get().getChangedCells().iterator().next() - .formatAsString(), "Sheet1!A1"); + Assert.assertEquals(event.get().getChangedCells().getCellCount(), 1); + Assert.assertEquals(event.get().getChangedCells().getCells().iterator() + .next().formatAsString(), "Sheet1!A1"); // Sanity check for the forumula cell effective value Assert.assertEquals(2.0, A1.getNumericCellValue(), 0.0); } From f58821b6498c053c8fc0b70f31e619fd84c23139 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Mon, 10 Jul 2023 17:42:29 +0300 Subject: [PATCH 09/10] refactor: add contains checks for indexes and add javadocs --- .../flow/component/spreadsheet/CellSet.java | 92 ++++++++++++++++++- ...llValueChangeEventOnFormulaChangeTest.java | 15 +++ 2 files changed, 102 insertions(+), 5 deletions(-) diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellSet.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellSet.java index 7df6fae9fc8..dc218eecdeb 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellSet.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellSet.java @@ -1,36 +1,118 @@ +/* + * Copyright 2023 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ package com.vaadin.flow.component.spreadsheet; import org.apache.poi.ss.util.CellReference; import java.util.Collections; +import java.util.Objects; import java.util.Set; +/** + * CellSet is a set of cells that also provides utilities regarding the contents + * of the set. + */ public class CellSet { private final Set cells; + /** + * Creates a set with the specified cells + * + * @param cells + * cells to construct the set with, not {@code null} + */ public CellSet(Set cells) { + Objects.requireNonNull(cells, "Cells cannot be null"); this.cells = cells; } + /** + * Gets an unmodifiable set of the cells + * + * @return an unmodifiable set of the cells + */ public Set getCells() { return Collections.unmodifiableSet(cells); } + /** + * Gets the number of the cells + * + * @return number of cells + */ public int getCellCount() { return cells.size(); } - public boolean containsCell(CellReference cell) { + /** + * Whether the set contains the specified cell. Does not take the sheet + * names of the cells in set into account if the sheet name of the cell + * reference is {@code null}. + * + * @param cellReference + * cell to be checked whether it exists in the set + * @return {@code true} if set contains the specified cell, {@code false} + * otherwise + */ + public boolean containsCell(CellReference cellReference) { if (cells.isEmpty()) { return false; } - if (cell.getSheetName() == null) { + if (cellReference.getSheetName() == null) { CellReference cellWithSheetName = new CellReference( - cells.iterator().next().getSheetName(), cell.getRow(), - cell.getCol(), cell.isRowAbsolute(), cell.isColAbsolute()); + cells.iterator().next().getSheetName(), + cellReference.getRow(), cellReference.getCol(), + cellReference.isRowAbsolute(), + cellReference.isColAbsolute()); return cells.contains(cellWithSheetName); } - return cells.contains(cell); + return cells.contains(cellReference); + } + + /** + * Whether the set contains the specified cell. Does not take the sheet + * names of the cells in set into account. + * + * @param row + * row index of the cell, 0-based + * @param col + * col index of the cell, 0-based + * @return {@code true} if set contains the specified cell, {@code false} + * otherwise + */ + public boolean containsCell(int row, int col) { + return containsCell(new CellReference(row, col)); + } + + /** + * Whether the set contains the specified cell + * + * @param row + * row index of the cell, 0-based + * @param col + * col index of the cell, 0-based + * @param sheetName + * sheet name of the cell, not {@code null} + * @return {@code true} if set contains the specified cell, {@code false} + * otherwise + */ + public boolean containsCell(int row, int col, String sheetName) { + Objects.requireNonNull(sheetName, "The sheet name cannot be null"); + return containsCell( + new CellReference(sheetName, row, col, false, false)); } } diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/test/java/com/vaadin/flow/component/spreadsheet/tests/CellValueChangeEventOnFormulaChangeTest.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/test/java/com/vaadin/flow/component/spreadsheet/tests/CellValueChangeEventOnFormulaChangeTest.java index 95f3b702943..904b15d621a 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/test/java/com/vaadin/flow/component/spreadsheet/tests/CellValueChangeEventOnFormulaChangeTest.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/test/java/com/vaadin/flow/component/spreadsheet/tests/CellValueChangeEventOnFormulaChangeTest.java @@ -1,6 +1,7 @@ package com.vaadin.flow.component.spreadsheet.tests; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import java.util.concurrent.atomic.AtomicReference; @@ -57,8 +58,22 @@ public void formulaChangeResultingInSameValue() { assertTrue("The changed cells should include C1 with sheet name", changedCells.get() .containsCell(new CellReference("Sheet0!C1"))); + assertFalse( + "The changed cells should not include C1 with a wrong sheet name", + changedCells.get() + .containsCell(new CellReference("Sheet1!C1"))); assertTrue("The changed cells should include C1 without sheet name", changedCells.get().containsCell(new CellReference("C1"))); + assertTrue( + "The changed cells should include a cell with correct indexes without a sheet name", + changedCells.get().containsCell(0, 2)); + assertTrue( + "The changed cells should include a cell with correct indexes and sheet name", + changedCells.get().containsCell(0, 2, "Sheet0")); + assertFalse( + "The changed cells should not include a cell with correct indexes and a wrong sheet name", + changedCells.get().containsCell(0, 2, "Sheet1")); + } } From e20b0fe378cf2db8637cbed6ea730cb8fb85ed44 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Tue, 18 Jul 2023 19:47:09 +0300 Subject: [PATCH 10/10] refactor: rename api to minimize the need for refactoring --- .../tests/fixtures/PopupButtonFixture.java | 2 +- .../flow/component/spreadsheet/CellSet.java | 13 ++++++------- .../CellValueChangeEventOnFormulaChangeTest.java | 16 +++++++--------- .../spreadsheet/tests/FormulasTest.java | 2 +- 4 files changed, 15 insertions(+), 18 deletions(-) diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow-integration-tests/src/main/java/com/vaadin/flow/component/spreadsheet/tests/fixtures/PopupButtonFixture.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow-integration-tests/src/main/java/com/vaadin/flow/component/spreadsheet/tests/fixtures/PopupButtonFixture.java index 38ca446f4c4..a3985d65a56 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow-integration-tests/src/main/java/com/vaadin/flow/component/spreadsheet/tests/fixtures/PopupButtonFixture.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow-integration-tests/src/main/java/com/vaadin/flow/component/spreadsheet/tests/fixtures/PopupButtonFixture.java @@ -21,7 +21,7 @@ public class PopupButtonFixture implements SpreadsheetFixture { @Override public void loadFixture(final Spreadsheet spreadsheet) { spreadsheet.addSelectionChangeListener(event -> { - if (event.getAllSelectedCells().getCellCount() != 1) { + if (event.getAllSelectedCells().size() != 1) { return; } List values = new ArrayList<>(VALUES); diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellSet.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellSet.java index dc218eecdeb..baf1474eb53 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellSet.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/CellSet.java @@ -54,7 +54,7 @@ public Set getCells() { * * @return number of cells */ - public int getCellCount() { + public int size() { return cells.size(); } @@ -68,7 +68,7 @@ public int getCellCount() { * @return {@code true} if set contains the specified cell, {@code false} * otherwise */ - public boolean containsCell(CellReference cellReference) { + public boolean contains(CellReference cellReference) { if (cells.isEmpty()) { return false; } @@ -94,8 +94,8 @@ public boolean containsCell(CellReference cellReference) { * @return {@code true} if set contains the specified cell, {@code false} * otherwise */ - public boolean containsCell(int row, int col) { - return containsCell(new CellReference(row, col)); + public boolean contains(int row, int col) { + return contains(new CellReference(row, col)); } /** @@ -110,9 +110,8 @@ public boolean containsCell(int row, int col) { * @return {@code true} if set contains the specified cell, {@code false} * otherwise */ - public boolean containsCell(int row, int col, String sheetName) { + public boolean contains(int row, int col, String sheetName) { Objects.requireNonNull(sheetName, "The sheet name cannot be null"); - return containsCell( - new CellReference(sheetName, row, col, false, false)); + return contains(new CellReference(sheetName, row, col, false, false)); } } diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/test/java/com/vaadin/flow/component/spreadsheet/tests/CellValueChangeEventOnFormulaChangeTest.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/test/java/com/vaadin/flow/component/spreadsheet/tests/CellValueChangeEventOnFormulaChangeTest.java index 904b15d621a..b7fa2aeed10 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/test/java/com/vaadin/flow/component/spreadsheet/tests/CellValueChangeEventOnFormulaChangeTest.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/test/java/com/vaadin/flow/component/spreadsheet/tests/CellValueChangeEventOnFormulaChangeTest.java @@ -54,25 +54,23 @@ public void formulaChangeResultingInSameValue() { spreadsheet.getCellValueManager().onCellValueChange(3, 1, "=A1+2*B1"); assertEquals("There should be one changed cell", 1, - changedCells.get().getCellCount()); + changedCells.get().size()); assertTrue("The changed cells should include C1 with sheet name", - changedCells.get() - .containsCell(new CellReference("Sheet0!C1"))); + changedCells.get().contains(new CellReference("Sheet0!C1"))); assertFalse( "The changed cells should not include C1 with a wrong sheet name", - changedCells.get() - .containsCell(new CellReference("Sheet1!C1"))); + changedCells.get().contains(new CellReference("Sheet1!C1"))); assertTrue("The changed cells should include C1 without sheet name", - changedCells.get().containsCell(new CellReference("C1"))); + changedCells.get().contains(new CellReference("C1"))); assertTrue( "The changed cells should include a cell with correct indexes without a sheet name", - changedCells.get().containsCell(0, 2)); + changedCells.get().contains(0, 2)); assertTrue( "The changed cells should include a cell with correct indexes and sheet name", - changedCells.get().containsCell(0, 2, "Sheet0")); + changedCells.get().contains(0, 2, "Sheet0")); assertFalse( "The changed cells should not include a cell with correct indexes and a wrong sheet name", - changedCells.get().containsCell(0, 2, "Sheet1")); + changedCells.get().contains(0, 2, "Sheet1")); } diff --git a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/test/java/com/vaadin/flow/component/spreadsheet/tests/FormulasTest.java b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/test/java/com/vaadin/flow/component/spreadsheet/tests/FormulasTest.java index 16a9e1bba49..29d9c5b2d7e 100644 --- a/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/test/java/com/vaadin/flow/component/spreadsheet/tests/FormulasTest.java +++ b/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/test/java/com/vaadin/flow/component/spreadsheet/tests/FormulasTest.java @@ -64,7 +64,7 @@ public void formulaValueChangeListener_invokedOnFormulaValueChange() { spreadsheet.refreshCells(A1, A2); // Check that the event was fired with the correct values - Assert.assertEquals(event.get().getChangedCells().getCellCount(), 1); + Assert.assertEquals(event.get().getChangedCells().size(), 1); Assert.assertEquals(event.get().getChangedCells().getCells().iterator() .next().formatAsString(), "Sheet1!A1"); // Sanity check for the forumula cell effective value