Skip to content

Commit 124b10a

Browse files
DiegoCardosoDiegoCardoso
committed
fix: make Icon work when used in Action (#8303)
* fix: make `Icon` work when used in `Action` This change fixes the issue when adding an icon to an action in the context menu in Spreadsheet. To make it work, the following modifications were made: - Add the `Icon` instance passed to the `Action(String, Icon)` as a virtual child to `Spreadsheet` - Passes the node id of the element to the client side to be fetched - Modifies the `SpreadsheetAction` class to overwrite the `getHTML` method in order to add a container for the icon, in case the action has one - Attaches the icon instance to the container - Creates an event to be dispached when the context menu is closed - Listens to the event to make sure that any `Icon` instance is safely removed from the client to avoid memory leaks Fixes #8288
1 parent 59e823e commit 124b10a

File tree

16 files changed

+302
-2
lines changed

16 files changed

+302
-2
lines changed

‎vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow-client/src/main/java/com/vaadin/addon/spreadsheet/client/SpreadsheetAction.java‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ public class SpreadsheetAction extends Action {
2222

2323
private SpreadsheetWidget widget;
2424

25+
private String iconContainerId;
26+
2527
public SpreadsheetAction(ActionOwner owner) {
2628
super(owner);
2729
}
@@ -35,6 +37,10 @@ public SpreadsheetAction(ActionOwner owner, SpreadsheetServerRpc rpc,
3537
actionKey = key;
3638
}
3739

40+
public void setIconContainerId(String iconContainerId) {
41+
this.iconContainerId = iconContainerId;
42+
}
43+
3844
@Override
3945
public void execute() {
4046
if (type == 0) {
@@ -48,4 +54,22 @@ public void execute() {
4854
widget.focusSheet();
4955
}
5056

57+
/**
58+
* Overriding the {@link Action#getHTML()} to add a container for the icon
59+
* if present.
60+
*/
61+
@Override
62+
public String getHTML() {
63+
StringBuilder sb = new StringBuilder();
64+
sb.append(
65+
"<div style=\"display:flex; align-items: baseline; gap: 5px;\">");
66+
if (iconContainerId != null && !iconContainerId.isEmpty()) {
67+
sb.append("<div id=\"").append(iconContainerId)
68+
.append("\" style=\"display:contents\"></div>");
69+
}
70+
sb.append(getCaption());
71+
sb.append("</div>");
72+
return sb.toString();
73+
}
74+
5175
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,4 +19,6 @@ public class SpreadsheetActionDetails implements Serializable {
1919
public String key;
2020
/** 0 = cell, 1 = row, 2 = column */
2121
public int type;
22+
/** Node id of the icon virtual child, 0 if no icon provided */
23+
public String iconNodeId;
2224
}

‎vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow-client/src/main/java/com/vaadin/addon/spreadsheet/client/SpreadsheetConnector.java‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.util.Map;
2020
import java.util.Set;
2121

22+
import com.google.gwt.animation.client.AnimationScheduler;
2223
import com.google.gwt.core.client.GWT;
2324
import com.google.gwt.core.client.Scheduler;
2425
import com.google.gwt.core.client.Scheduler.ScheduledCommand;
@@ -27,6 +28,7 @@
2728
import com.google.gwt.event.dom.client.ContextMenuEvent;
2829
import com.google.gwt.event.dom.client.ContextMenuHandler;
2930
import com.google.gwt.event.shared.HandlerRegistration;
31+
import com.google.gwt.user.client.DOM;
3032
import com.google.gwt.user.client.ui.Image;
3133
import com.google.gwt.user.client.ui.Widget;
3234
import com.vaadin.addon.spreadsheet.client.SpreadsheetWidget.SheetContextMenuHandler;
@@ -105,17 +107,46 @@ public Action[] getActions() {
105107
SpreadsheetWidget widget = getWidget();
106108
SpreadsheetServerRpc rpcProxy = getRpcProxy(
107109
SpreadsheetServerRpc.class);
110+
final String APP_ID = host.getPropertyString("appId");
111+
final HashMap<String, Element> iconsToAppend = new HashMap<>();
112+
108113
for (SpreadsheetActionDetails actionDetail : actionDetails) {
109114
SpreadsheetAction spreadsheetAction = new SpreadsheetAction(
110115
this, rpcProxy, actionDetail.key,
111116
actionDetail.type, widget);
112117
spreadsheetAction.setCaption(actionDetail.caption);
113-
spreadsheetAction
114-
.setIconUrl(getResourceUrl(actionDetail.key));
118+
119+
if (actionDetail.iconNodeId != null) {
120+
var iconContainerId = "spreadsheet-icon-container-"
121+
+ actionDetail.iconNodeId;
122+
iconsToAppend.put(iconContainerId,
123+
SheetJsniUtil.getVirtualChild(
124+
actionDetail.iconNodeId, APP_ID));
125+
spreadsheetAction
126+
.setIconContainerId(iconContainerId);
127+
}
128+
115129
actions.add(spreadsheetAction);
116130
}
131+
132+
if (!iconsToAppend.isEmpty()) {
133+
// Need to wait for the actions to be rendered to the
134+
// context menu to then attach the icons to their
135+
// containers
136+
AnimationScheduler.get()
137+
.requestAnimationFrame((timestamp) -> {
138+
for (String nodeId : iconsToAppend
139+
.keySet()) {
140+
var container = DOM
141+
.getElementById(nodeId);
142+
container.appendChild(
143+
iconsToAppend.get(nodeId));
144+
}
145+
});
146+
}
117147
return actions.toArray(new Action[actions.size()]);
118148
}
149+
119150
}, left, top);
120151
}
121152

@@ -254,6 +285,10 @@ public void onContextMenu(ContextMenuEvent event) {
254285
}
255286
}, ContextMenuEvent.getType());
256287

288+
getConnection().getContextMenu().addCloseHandler(handler -> {
289+
getRpcProxy(SpreadsheetServerRpc.class).contextMenuClosed();
290+
});
291+
257292
getRpcProxy(SpreadsheetServerRpc.class).onConnectorInit();
258293
}
259294

‎vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow-client/src/main/java/com/vaadin/addon/spreadsheet/client/SpreadsheetServerRpc.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,4 +72,8 @@ public interface SpreadsheetServerRpc extends ServerRpc, SpreadsheetHandler {
7272
*/
7373
void actionOnColumnHeader(String actionKey);
7474

75+
/**
76+
* Context menu was closed.
77+
*/
78+
void contextMenuClosed();
7579
}

‎vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow-client/src/main/java/com/vaadin/component/spreadsheet/client/js/SpreadsheetJsApi.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -683,6 +683,10 @@ public void setCellsAddedToRangeSelectionCallback(
683683
getServerRpcInstance().setCellsAddedToRangeSelectionCallback(callback);
684684
}
685685

686+
public void setContextMenuClosedCallback(JsConsumer<Void> callback) {
687+
getServerRpcInstance().setContextMenuClosedCallback(callback);
688+
}
689+
686690
public void setRowSelectedCallback(JsConsumer<String> callback) {
687691
getServerRpcInstance().setRowSelectedCallback(callback);
688692
}

‎vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow-client/src/main/java/com/vaadin/component/spreadsheet/client/js/SpreadsheetServerRpcImpl.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ public interface JsConsumer<T> {
4141
private JsConsumer<String> rowAddedToRangeSelectionCallback;
4242
private JsConsumer<String> columnSelectedCallback;
4343
private JsConsumer<String> columnAddedToSelectionCallback;
44+
private JsConsumer<Void> contextMenuClosedCallback;
4445
private JsConsumer<String> selectionIncreasePaintedCallback;
4546
private JsConsumer<String> selectionDecreasePaintedCallback;
4647
private JsConsumer<String> cellValueEditedCallback;
@@ -107,6 +108,10 @@ public void setCellsAddedToRangeSelectionCallback(
107108
cellsAddedToRangeSelectionCallback = callback;
108109
}
109110

111+
public void setContextMenuClosedCallback(JsConsumer<Void> callback) {
112+
contextMenuClosedCallback = callback;
113+
}
114+
110115
public void setRowSelectedCallback(JsConsumer<String> callback) {
111116
rowSelectedCallback = callback;
112117
}
@@ -480,6 +485,10 @@ public void columnHeaderContextMenuOpen(int columnIndex) {
480485
call(columnHeaderContextMenuOpenCallback, columnIndex);
481486
}
482487

488+
public void contextMenuClosed() {
489+
call(contextMenuClosedCallback);
490+
}
491+
483492
@Override
484493
public void actionOnColumnHeader(String actionKey) {
485494
call(actionOnColumnHeaderCallback, actionKey);

‎vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow-integration-tests/src/main/java/com/vaadin/flow/component/spreadsheet/tests/fixtures/IconActionFixture.java‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
/**
2+
* Copyright 2000-2025 Vaadin Ltd.
3+
*
4+
* This program is available under Vaadin Commercial License and Service Terms.
5+
*
6+
* See {@literal <https://vaadin.com/commercial-license-and-service-terms>} for the full
7+
* license.
8+
*/
9+
package com.vaadin.flow.component.spreadsheet.tests.fixtures;
10+
11+
import org.apache.poi.ss.util.CellRangeAddress;
12+
13+
import com.vaadin.flow.component.icon.VaadinIcon;
14+
import com.vaadin.flow.component.spreadsheet.Spreadsheet;
15+
import com.vaadin.flow.component.spreadsheet.framework.Action;
16+
import com.vaadin.flow.component.spreadsheet.tests.SpreadsheetActionHandler;
17+
import com.vaadin.flow.theme.lumo.LumoIcon;
18+
19+
/**
20+
* Fixture that verifies Actions created with Icon constructors render icons in
21+
* the context menu.
22+
*/
23+
public class IconActionFixture implements SpreadsheetFixture {
24+
25+
@Override
26+
public void loadFixture(Spreadsheet spreadsheet) {
27+
28+
SpreadsheetActionHandler handler = new SpreadsheetActionHandler();
29+
30+
// Row action with a Lumo icon
31+
handler.addRowHandler(new SpreadsheetActionHandler.Row() {
32+
@Override
33+
public Action[] getActions(CellRangeAddress target,
34+
Spreadsheet sender) {
35+
return new Action[] {
36+
new Action("Row action", LumoIcon.CALENDAR.create()) };
37+
}
38+
39+
@Override
40+
public void handleAction(Action action, CellRangeAddress sender,
41+
Spreadsheet target) {
42+
// no-op
43+
}
44+
});
45+
46+
// Cell action with Lumo icon
47+
handler.addCellHandler(new SpreadsheetActionHandler.Cell() {
48+
@Override
49+
public void handleAction(Action action,
50+
Spreadsheet.SelectionChangeEvent sender,
51+
Spreadsheet target) {
52+
// no-op
53+
}
54+
55+
@Override
56+
public Action[] getActions(
57+
Spreadsheet.SelectionChangeEvent selection,
58+
Spreadsheet sender) {
59+
return new Action[] {
60+
new Action("Lumo icon", LumoIcon.ANGLE_UP.create()) };
61+
}
62+
});
63+
64+
// Cell action with Vaadin icon
65+
handler.addCellHandler(new SpreadsheetActionHandler.Cell() {
66+
@Override
67+
public void handleAction(Action action,
68+
Spreadsheet.SelectionChangeEvent sender,
69+
Spreadsheet target) {
70+
// no-op
71+
}
72+
73+
@Override
74+
public Action[] getActions(Spreadsheet.SelectionChangeEvent target,
75+
Spreadsheet sender) {
76+
return new Action[] { new Action("Vaadin icon",
77+
VaadinIcon.ACADEMY_CAP.create()) };
78+
}
79+
});
80+
81+
// Column action without icon to ensure mixed items render fine
82+
handler.addColumnHandler(new SpreadsheetActionHandler.Column() {
83+
@Override
84+
public void handleAction(Action action, CellRangeAddress sender,
85+
Spreadsheet target) {
86+
// no-op
87+
}
88+
89+
@Override
90+
public Action[] getActions(CellRangeAddress target,
91+
Spreadsheet sender) {
92+
return new Action[] {
93+
new Action("Column action", LumoIcon.COG.create()) };
94+
}
95+
});
96+
97+
spreadsheet.removeDefaultActionHandler();
98+
spreadsheet.addActionHandler(handler);
99+
}
100+
101+
}

‎vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow-integration-tests/src/main/java/com/vaadin/flow/component/spreadsheet/tests/fixtures/TestFixtures.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ public enum TestFixtures {
4242
LockSheet(LockSheetFixture.class),
4343
CustomComponent(CustomComponentFixture.class),
4444
Action(ActionFixture.class),
45+
IconAction(IconActionFixture.class),
4546
InsertRow(InsertRowFixture.class),
4647
DeleteRow(DeleteRowFixture.class),
4748
RowHeaderDoubleClick(RowHeaderDoubleClickFixture.class);

‎vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow-integration-tests/src/test/java/com/vaadin/flow/component/spreadsheet/test/ContextMenuIT.java‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import org.junit.Test;
1414
import org.openqa.selenium.By;
1515
import org.openqa.selenium.Keys;
16+
import org.openqa.selenium.WebElement;
1617
import org.openqa.selenium.interactions.Actions;
1718

1819
import com.vaadin.flow.component.spreadsheet.testbench.SheetCellElement;
@@ -99,4 +100,48 @@ public void testHeaders() throws InterruptedException {
99100
Assert.assertEquals("first column", getCellContent("C3"));
100101
Assert.assertEquals("last column", getCellContent("C4"));
101102
}
103+
104+
@Test
105+
public void contextMenu_itemsContainExpectedIcons() {
106+
loadTestFixture(TestFixtures.IconAction);
107+
108+
var cell = getSpreadsheet().getCellAt("B2");
109+
cell.contextClick();
110+
111+
var vaadinIcon = getIconFromAction("Vaadin icon");
112+
Assert.assertEquals("vaadin:academy-cap",
113+
vaadinIcon.getAttribute("icon"));
114+
115+
var lumoIcon = getIconFromAction("Lumo icon");
116+
String lumoAttr = lumoIcon.getAttribute("icon");
117+
Assert.assertEquals("lumo:angle-up", lumoAttr);
118+
}
119+
120+
private WebElement getIconFromAction(String actionText) {
121+
return findElement(By.xpath("//div[@class='popupContent']//*[text()='"
122+
+ actionText
123+
+ "']/ancestor::*[contains(@class,'gwt-MenuItem')]//*[@icon]"));
124+
}
125+
126+
@Test
127+
public void contextMenu_rowHeader_itemsContainExpectedIcons() {
128+
loadTestFixture(TestFixtures.IconAction);
129+
130+
// Open context menu for row header
131+
getSpreadsheet().getRowHeader(3).contextClick();
132+
133+
var rowIcon = getIconFromAction("Row action");
134+
Assert.assertEquals("lumo:calendar", rowIcon.getAttribute("icon"));
135+
}
136+
137+
@Test
138+
public void contextMenu_columnHeader_itemsContainExpectedIcons() {
139+
loadTestFixture(TestFixtures.IconAction);
140+
141+
// Open context menu for column header
142+
getSpreadsheet().getColumnHeader(3).contextClick();
143+
144+
var colIcon = getIconFromAction("Column action");
145+
Assert.assertEquals("lumo:cog", colIcon.getAttribute("icon"));
146+
}
102147
}

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.vaadin.flow.component.spreadsheet.framework.Action;
2323
import com.vaadin.flow.component.spreadsheet.framework.Action.Handler;
2424
import com.vaadin.flow.data.provider.KeyMapper;
25+
import com.vaadin.flow.dom.Element;
2526

2627
/**
2728
* ContextMenuManager is an utility class for the Spreadsheet component. This
@@ -42,6 +43,8 @@ public class ContextMenuManager implements Serializable {
4243

4344
private final Spreadsheet spreadsheet;
4445

46+
private final ArrayList<Element> iconsInContextMenu = new ArrayList<>();
47+
4548
private int contextMenuHeaderIndex = -1;
4649

4750
/**
@@ -284,6 +287,10 @@ protected ArrayList<SpreadsheetActionDetails> createActionsListForRow(
284287
return actions;
285288
}
286289

290+
public void onContextMenuClosed() {
291+
removeActionIcons();
292+
}
293+
287294
/**
288295
* Helper method to create SpreadsheetActionDetails from actions.
289296
*
@@ -306,11 +313,27 @@ private ArrayList<SpreadsheetActionDetails> createActionDetailsList(
306313
}
307314
spreadsheetActionDetails.caption = Jsoup.clean(caption,
308315
Safelist.relaxed());
316+
if (action.getIcon() != null) {
317+
var icon = action.getIcon().getElement();
318+
// Attach the icon to the spreadsheet as a virtual child so
319+
// that it can be fetched by the client-side context menu.
320+
spreadsheet.getElement().appendVirtualChild(icon);
321+
iconsInContextMenu.add(icon);
322+
spreadsheetActionDetails.iconNodeId = icon.getNode()
323+
.getId();
324+
}
309325
spreadsheetActionDetails.key = key;
310326
spreadsheetActionDetails.type = actionType.getValue();
311327
actionDetailsList.add(spreadsheetActionDetails);
312328
}
313329
}
314330
return actionDetailsList;
315331
}
332+
333+
private void removeActionIcons() {
334+
for (var icon : iconsInContextMenu) {
335+
spreadsheet.getElement().removeVirtualChild(icon);
336+
}
337+
iconsInContextMenu.clear();
338+
}
316339
}

0 commit comments

Comments
 (0)