From 245d39bf3448f126855eda64e58222a1504445b5 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Thu, 2 Feb 2023 10:24:39 +0200 Subject: [PATCH 01/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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 From e380adcc657eae4a1c740b2ec9a73a324cf68f49 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Thu, 2 Feb 2023 10:24:39 +0200 Subject: [PATCH 11/21] 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 8bceb75af6f..8abf7a354e2 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 @@ -35,8 +35,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 b74b8e4442b..ba81d37020b 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 @@ -117,7 +117,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) { @@ -174,7 +176,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() @@ -218,8 +222,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); @@ -235,8 +240,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); @@ -386,8 +393,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); } @@ -467,8 +475,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); @@ -511,7 +520,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( @@ -562,8 +573,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(); @@ -596,8 +608,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())); @@ -616,8 +629,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(); @@ -650,8 +664,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)); @@ -680,8 +695,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 15778df7079..a467e6468d7 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 2148ebcee44..e2c95b0d6ec 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 46ee1076573..377a75c8276 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 @@ -665,7 +665,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(); } @@ -674,7 +675,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(); @@ -2983,7 +2985,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); @@ -4968,7 +4971,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); } /** @@ -5392,7 +5396,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 61121cac31b..05d61f02e2b 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 @@ -337,7 +337,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 de770683b32..326df3837b5 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 94ff17a9a01..46e691b9579 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 2642b2edf71..684a761daac 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 8760538db859d53018e98782949000d9850ca5e6 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Tue, 14 Feb 2023 08:45:53 +0200 Subject: [PATCH 12/21] 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 326df3837b5..de3814b56b4 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 46e691b9579..eb16d3aae62 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 684a761daac..d73037b0f54 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 b02d46706f9759c19bf079f7288e3d88abfa966a Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Tue, 14 Feb 2023 08:47:08 +0200 Subject: [PATCH 13/21] 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 de3814b56b4..968e8d4119a 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 eb16d3aae62..6122715b77e 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 d73037b0f54..00d82935e6f 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 ce3b0704e08b8f6d5a05f2428766b02cd613809a Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Tue, 14 Feb 2023 08:48:08 +0200 Subject: [PATCH 14/21] 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 e2c95b0d6ec..ec5677d7508 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 ef5e46113f37f3f874792cc628f78d09f11e42d5 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Wed, 15 Feb 2023 09:30:48 +0200 Subject: [PATCH 15/21] 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 ec5677d7508..18143ab8899 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 e874832be0bcd3ce4927c50a4d33c5f1bb98ee8b Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Wed, 15 Feb 2023 09:49:18 +0200 Subject: [PATCH 16/21] 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 5c268c62791..68face6fc9a 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 @@ -9,6 +9,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; @@ -59,9 +60,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 2327868a5cc23140775dee5f23c25fdd1c32f4a6 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Mon, 27 Feb 2023 20:14:26 +0200 Subject: [PATCH 17/21] 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 ba81d37020b..099f4d62d0d 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 @@ -256,7 +256,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(); } } @@ -403,7 +407,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 98e7b9480a68c36def7eead6d385a68cdfd8ed9b Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Mon, 3 Jul 2023 20:06:04 +0300 Subject: [PATCH 18/21] 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 8abf7a354e2..3e771c85591 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 @@ -30,7 +30,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 18143ab8899..988c8e85757 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 377a75c8276..aaeeca70dc5 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 @@ -2857,16 +2857,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++) { @@ -5242,15 +5240,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; } } @@ -5374,10 +5372,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)); } } @@ -5385,10 +5383,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) { @@ -5594,10 +5589,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 06f3fd13b71..93e16c3a8c9 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 68face6fc9a..20ddddd108b 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 @@ -11,9 +11,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; @@ -51,20 +51,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 6ec9b858f21..bb6c5f8f5a4 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 @@ -83,9 +83,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 b23654682c2b01e3a091044c9b3e2eb0c8cfdc30 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Mon, 10 Jul 2023 17:42:29 +0300 Subject: [PATCH 19/21] 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 20ddddd108b..d7baff4e440 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 @@ -9,6 +9,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; @@ -65,8 +66,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 ae9f2d21a5022353f2b8e7c1bfcc09665a67df6d Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Tue, 18 Jul 2023 19:47:09 +0300 Subject: [PATCH 20/21] 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 3e771c85591..8abf7a354e2 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 @@ -30,7 +30,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 d7baff4e440..ebf071539ea 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 @@ -62,25 +62,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 bb6c5f8f5a4..8ccadc26f3e 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 @@ -83,7 +83,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 From 30c0bcbfc5bad1d434c27322afe8bc6c7ab04dd4 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Mon, 15 Sep 2025 09:04:45 +0300 Subject: [PATCH 21/21] chore: run formatter --- .../flow/component/spreadsheet/CellSet.java | 21 +++++++------------ ...llValueChangeEventOnFormulaChangeTest.java | 2 +- 2 files changed, 8 insertions(+), 15 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 baf1474eb53..4a95c1c4666 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,26 +1,19 @@ -/* - * 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 +/** + * Copyright 2000-2023 Vaadin Ltd. * - * http://www.apache.org/licenses/LICENSE-2.0 + * This program is available under Vaadin Commercial License and Service Terms. * - * 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. + * See {@literal } for the full + * 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; +import org.apache.poi.ss.util.CellReference; + /** * CellSet is a set of cells that also provides utilities regarding the contents * of the set. 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 ebf071539ea..bcdb940d415 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 @@ -14,7 +14,6 @@ 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; @@ -23,6 +22,7 @@ import org.junit.Before; import org.junit.Test; +import com.vaadin.flow.component.spreadsheet.CellSet; import com.vaadin.flow.component.spreadsheet.Spreadsheet; /**