Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
245d39b
fix: use the same cell reference constructor in order to ensure consi…
ugur-vaadin Feb 2, 2023
2af83b6
Merge branch 'master' into 4588-spreadsheet-does-not-trigger-addcellv…
ugur-vaadin Feb 14, 2023
fe708df
fix: use getSheet from SpreadsheetCommand to get the sheet
ugur-vaadin Feb 14, 2023
6227ec5
fix: apply formatter
ugur-vaadin Feb 14, 2023
268ff65
fix: add cell reference without sheet name to cell value change event
ugur-vaadin Feb 14, 2023
066e1d2
Merge branch 'main' into 4588-spreadsheet-does-not-trigger-addcellval…
ugur-vaadin Feb 15, 2023
7105375
fix: add missing cell reference without sheet name to event
ugur-vaadin Feb 15, 2023
893994a
Merge branch 'main' into 4588-spreadsheet-does-not-trigger-addcellval…
ugur-vaadin Feb 15, 2023
31fa8ac
fix: update cell value change unit test
ugur-vaadin Feb 15, 2023
8f08024
Merge branch '4588-spreadsheet-does-not-trigger-addcellvaluechangelis…
ugur-vaadin Feb 15, 2023
9158159
Merge branch 'main' into 4588-spreadsheet-does-not-trigger-addcellval…
ugur-vaadin Feb 27, 2023
5476fdd
fix: add sheet name to more internal cell references
ugur-vaadin Feb 27, 2023
09928c6
Merge branch 'main' into 4588-spreadsheet-does-not-trigger-addcellval…
ugur-vaadin Jun 30, 2023
530e9e7
fix: introduce cellset to avoid selected cell duplication in event
ugur-vaadin Jul 3, 2023
27ce9d9
Merge branch 'main' into 4588-spreadsheet-does-not-trigger-addcellval…
ugur-vaadin Jul 3, 2023
f58821b
refactor: add contains checks for indexes and add javadocs
ugur-vaadin Jul 10, 2023
60dd913
Merge branch 'main' into 4588-spreadsheet-does-not-trigger-addcellval…
ugur-vaadin Jul 18, 2023
e20b0fe
refactor: rename api to minimize the need for refactoring
ugur-vaadin Jul 18, 2023
e380adc
fix: use the same cell reference constructor in order to ensure consi…
ugur-vaadin Feb 2, 2023
8760538
fix: use getSheet from SpreadsheetCommand to get the sheet
ugur-vaadin Feb 14, 2023
b02d467
fix: apply formatter
ugur-vaadin Feb 14, 2023
ce3b070
fix: add cell reference without sheet name to cell value change event
ugur-vaadin Feb 14, 2023
ef5e461
fix: add missing cell reference without sheet name to event
ugur-vaadin Feb 15, 2023
e874832
fix: update cell value change unit test
ugur-vaadin Feb 15, 2023
2327868
fix: add sheet name to more internal cell references
ugur-vaadin Feb 27, 2023
98e7b94
fix: introduce cellset to avoid selected cell duplication in event
ugur-vaadin Jul 3, 2023
b236546
refactor: add contains checks for indexes and add javadocs
ugur-vaadin Jul 10, 2023
ae9f2d2
refactor: rename api to minimize the need for refactoring
ugur-vaadin Jul 18, 2023
ae25f93
Merge branch '4588-spreadsheet-does-not-trigger-addcellvaluechangelis…
ugur-vaadin Sep 15, 2025
30c0bcb
chore: run formatter
ugur-vaadin Sep 15, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,9 @@ public void loadFixture(final Spreadsheet spreadsheet) {
}
List<String> 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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -249,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();
}
}
Expand Down Expand Up @@ -386,16 +397,22 @@ 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);
}

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,
Expand Down Expand Up @@ -467,8 +484,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);
Expand Down Expand Up @@ -511,7 +529,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(
Expand Down Expand Up @@ -562,8 +582,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();
Expand Down Expand Up @@ -596,8 +617,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()));
Expand All @@ -616,8 +638,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();
Expand Down Expand Up @@ -650,8 +673,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));
Expand Down Expand Up @@ -680,8 +704,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<CellRangeAddress> i = cellRangeAddresses.iterator(); i
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -399,8 +401,10 @@ public void onSelectionDecreasePainted(int r, int c) {
if (!SpreadsheetUtil.isCellInRange(selectedCellReference,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

18% of developers fix this issue

NULL_DEREFERENCE: object selectedCellReference last assigned on line 398 could be null and is dereferenced by call to isCellInRange(...) at line 401.


ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

newPaintedCellRange)) {
selectedCellReference = new CellReference(
spreadsheet.getActiveSheet().getSheetName(),
newPaintedCellRange.getFirstRow(),
newPaintedCellRange.getFirstColumn());
newPaintedCellRange.getFirstColumn(), false,
false);
}
getCellSelectionManager().handleCellRangeSelection(
selectedCellReference, newPaintedCellRange, false);
Expand Down
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to make CellSet extend HashSet and override the contains method to apply the custom logic done here? This would make it a bit less of a breaking change, I suppose, as the returning types could still be left as Set. If CellSet become unused externally, it could even be made package private.

The contains method would become something like this:

    @Override
    public boolean contains(Object object) {
        if (this.isEmpty()) {
            return false;
        }
        if (object instanceof CellReference cellReference) {
            if (cellReference.getSheetName() == null) {
                CellReference cellWithSheetName = new CellReference(
                        this.iterator().next().getSheetName(),
                            cellReference.getRow(), cellReference.getCol(),
                    cellReference.isRowAbsolute(),
                    cellReference.isColAbsolute());
                return super.contains(cellWithSheetName);
            }
        }
        return super.contains(object);

Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/**
* Copyright 2000-2023 Vaadin Ltd.
*
* This program is available under Vaadin Commercial License and Service Terms.
*
* See {@literal <https://vaadin.com/commercial-license-and-service-terms>} for the full
* license.
*/
package com.vaadin.flow.component.spreadsheet;

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.
*/
public class CellSet {

private final Set<CellReference> cells;

/**
* Creates a set with the specified cells
*
* @param cells
* cells to construct the set with, not {@code null}
*/
public CellSet(Set<CellReference> 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<CellReference> getCells() {
return Collections.unmodifiableSet(cells);
}

/**
* Gets the number of the cells
*
* @return number of cells
*/
public int size() {
return cells.size();
}

/**
* 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 contains(CellReference cellReference) {
if (cells.isEmpty()) {
return false;
}
if (cellReference.getSheetName() == null) {
CellReference cellWithSheetName = new CellReference(
cells.iterator().next().getSheetName(),
cellReference.getRow(), cellReference.getCol(),
cellReference.isRowAbsolute(),
cellReference.isColAbsolute());
return cells.contains(cellWithSheetName);
}
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 contains(int row, int col) {
return contains(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 contains(int row, int col, String sheetName) {
Objects.requireNonNull(sheetName, "The sheet name cannot be null");
return contains(new CellReference(sheetName, row, col, false, false));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -781,8 +783,9 @@ private String getFormattedCellValue(Cell cell) {
}

private void fireCellValueChangeEvent(Cell cell) {
Set<CellReference> cells = new HashSet<CellReference>();
cells.add(new CellReference(cell));
Set<CellReference> cells = new HashSet<>();
CellReference ref = new CellReference(cell);
cells.add(ref);
spreadsheet.fireEvent(new CellValueChangeEvent(spreadsheet, cells));
}

Expand Down
Loading