Skip to content

Commit ad9acfb

Browse files
committed
refactor: remove duplication code in ContextMenuManager (#8285)
Move the logic to create `SpreadsheetActionDetails` from `Action` to a helper method. The only difference was the `type` value assigned to each of them, so it makes sense to pass this value as parameter to the helper method.
1 parent 07b8c23 commit ad9acfb

File tree

3 files changed

+461
-26
lines changed

3 files changed

+461
-26
lines changed

vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/ContextMenuManager.java

Lines changed: 55 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
import java.util.LinkedList;
1414

1515
import org.apache.poi.ss.util.CellRangeAddress;
16+
import org.jsoup.Jsoup;
17+
import org.jsoup.safety.Safelist;
1618
import org.slf4j.Logger;
1719
import org.slf4j.LoggerFactory;
1820

@@ -42,6 +44,23 @@ public class ContextMenuManager implements Serializable {
4244

4345
private int contextMenuHeaderIndex = -1;
4446

47+
/**
48+
* Enum for spreadsheet action types.
49+
*/
50+
enum ActionType {
51+
CELL(0), ROW(1), COLUMN(2);
52+
53+
private final int value;
54+
55+
ActionType(int value) {
56+
this.value = value;
57+
}
58+
59+
public int getValue() {
60+
return value;
61+
}
62+
}
63+
4564
/**
4665
* Constructs a new ContextMenuManager and ties it to the given Spreadsheet.
4766
*
@@ -216,19 +235,11 @@ public void onActionOnColumnHeader(String actionKey) {
216235
protected ArrayList<SpreadsheetActionDetails> createActionsListForSelection() {
217236
ArrayList<SpreadsheetActionDetails> actions = new ArrayList<SpreadsheetActionDetails>();
218237
for (Handler handler : actionHandlers) {
219-
Action[] actions2 = handler.getActions(spreadsheet
238+
Action[] handlerActions = handler.getActions(spreadsheet
220239
.getCellSelectionManager().getLatestSelectionEvent(),
221240
spreadsheet);
222-
if (actions2 != null) {
223-
for (Action action : actions2) {
224-
String key = actionMapper.key(action);
225-
SpreadsheetActionDetails spreadsheetActionDetails = new SpreadsheetActionDetails();
226-
spreadsheetActionDetails.caption = action.getCaption();
227-
spreadsheetActionDetails.key = key;
228-
spreadsheetActionDetails.type = 0;
229-
actions.add(spreadsheetActionDetails);
230-
}
231-
}
241+
actions.addAll(
242+
createActionDetailsList(handlerActions, ActionType.CELL));
232243
}
233244
return actions;
234245
}
@@ -246,14 +257,9 @@ protected ArrayList<SpreadsheetActionDetails> createActionsListForColumn(
246257
final CellRangeAddress column = new CellRangeAddress(-1, -1,
247258
columnIndex - 1, columnIndex - 1);
248259
for (Handler handler : actionHandlers) {
249-
for (Action action : handler.getActions(column, spreadsheet)) {
250-
String key = actionMapper.key(action);
251-
SpreadsheetActionDetails spreadsheetActionDetails = new SpreadsheetActionDetails();
252-
spreadsheetActionDetails.caption = action.getCaption();
253-
spreadsheetActionDetails.key = key;
254-
spreadsheetActionDetails.type = 2;
255-
actions.add(spreadsheetActionDetails);
256-
}
260+
Action[] handlerActions = handler.getActions(column, spreadsheet);
261+
actions.addAll(
262+
createActionDetailsList(handlerActions, ActionType.COLUMN));
257263
}
258264
return actions;
259265
}
@@ -271,16 +277,40 @@ protected ArrayList<SpreadsheetActionDetails> createActionsListForRow(
271277
final CellRangeAddress row = new CellRangeAddress(rowIndex - 1,
272278
rowIndex - 1, -1, -1);
273279
for (Handler handler : actionHandlers) {
274-
for (Action action : handler.getActions(row, spreadsheet)) {
280+
Action[] handlerActions = handler.getActions(row, spreadsheet);
281+
actions.addAll(
282+
createActionDetailsList(handlerActions, ActionType.ROW));
283+
}
284+
return actions;
285+
}
286+
287+
/**
288+
* Helper method to create SpreadsheetActionDetails from actions.
289+
*
290+
* @param actions
291+
* Array of actions to convert
292+
* @param actionType
293+
* Type of the action (cell, row, or column)
294+
* @return List of SpreadsheetActionDetails
295+
*/
296+
private ArrayList<SpreadsheetActionDetails> createActionDetailsList(
297+
Action[] actions, ActionType actionType) {
298+
ArrayList<SpreadsheetActionDetails> actionDetailsList = new ArrayList<SpreadsheetActionDetails>();
299+
if (actions != null) {
300+
for (Action action : actions) {
275301
String key = actionMapper.key(action);
276302
SpreadsheetActionDetails spreadsheetActionDetails = new SpreadsheetActionDetails();
277-
spreadsheetActionDetails.caption = action.getCaption();
303+
String caption = action.getCaption();
304+
if (caption == null) {
305+
caption = "";
306+
}
307+
spreadsheetActionDetails.caption = Jsoup.clean(caption,
308+
Safelist.relaxed());
278309
spreadsheetActionDetails.key = key;
279-
spreadsheetActionDetails.type = 1;
280-
actions.add(spreadsheetActionDetails);
310+
spreadsheetActionDetails.type = actionType.getValue();
311+
actionDetailsList.add(spreadsheetActionDetails);
281312
}
282313
}
283-
return actions;
314+
return actionDetailsList;
284315
}
285-
286316
}

vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/client/SpreadsheetActionDetails.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@
1414
public class SpreadsheetActionDetails implements Serializable {
1515
public String caption;
1616
public String key;
17-
/** 0 = cell, 1 = row, 2 = column TODO replace with enum type */
17+
/**
18+
* 0 = cell, 1 = row, 2 = column - kept as int for client-server
19+
* compatibility
20+
*/
1821
public int type;
1922
}

0 commit comments

Comments
 (0)