Skip to content

Commit 4ae47f8

Browse files
DiegoCardosoTatuLundclaude
authored
fix: make spreadsheet overlays native popover, per instance (#9303)
The spreadsheet attaches a `<div id="spreadsheet-overlays">` to host its overlay widgets (context menu, tooltips, comment overlays, popup buttons). It has historically lived in `<body>` to escape clipping by ancestor `overflow`/`transform` rules and as a singleton shared by all spreadsheet instances on the page. Switching the overlays to native popover changes the picture: every overlay enters the top layer regardless of DOM position, so neither the body-attachment nor the singleton are load-bearing anymore. This PR includes the native popover changes proposed in #9270 and adds the structural changes that the popover approach enables: - the container moves into the `<vaadin-spreadsheet>` light DOM, projected into the shadow root via a new `overlays` slot, with one container per instance; - the reference is threaded from JS into the GWT widget chain instead of being looked up by id; - `SpreadsheetConnector` — not `ApplicationConnection` — now owns the `SpreadsheetContextMenu`. Tooltips and the cell-comment overlay are created in `SheetWidget`'s constructor (before the host is known), so the container is wired in via a setter rather than a constructor argument. A side benefit: spreadsheet overlays inside a modal `Dialog` are interactive again. The modal sets `body { pointer-events: none }` to disable everything outside the dialog overlay; with the container now inside the dialog's slotted content, the overlays inherit `pointer-events: auto` from the dialog's overlay part instead of `pointer-events: none` from `<body>`. Intended to replace #9270. Related to #9270 --- 🤖 Generated with Claude Code --------- Co-authored-by: Tatu Lund <tatu@vaadin.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent c551e99 commit 4ae47f8

14 files changed

Lines changed: 296 additions & 49 deletions

File tree

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import com.google.gwt.user.client.ui.PopupPanel.PositionCallback;
2626
import com.google.gwt.user.client.ui.VerticalPanel;
2727
import com.google.gwt.user.client.ui.Widget;
28-
import com.vaadin.client.ui.VOverlay;
2928

3029
public class PopupButtonWidget extends FocusWidget
3130
implements ClickHandler, HasCloseHandlers<PopupPanel> {
@@ -60,7 +59,7 @@ public void setPosition(int offsetWidth, int offsetHeight) {
6059
}
6160
};
6261

63-
private final VOverlay popup;
62+
private final SpreadsheetOverlay popup;
6463
private final PopupButtonHeader popupHeader;
6564
private final VerticalPanel popupLayout;
6665

@@ -94,6 +93,7 @@ public void setSheetWidget(SheetWidget owner, DivElement sheet) {
9493
this.sheet = sheet;
9594
this.owner = owner;
9695
popup.setOwner(owner);
96+
popup.setOverlayContainer(owner.getOverlayContainer());
9797
popupHeader.setSheet(owner);
9898
}
9999

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
import com.google.gwt.user.client.ui.Widget;
2929
import com.vaadin.client.MeasuredSize;
3030
import com.vaadin.client.WidgetUtil;
31-
import com.vaadin.client.ui.VOverlay;
3231

3332
public class SelectionWidget extends Composite {
3433

@@ -576,7 +575,7 @@ public void setVisible(boolean visible) {
576575
private int selectionStartCol;
577576
private int selectionStartRow;
578577

579-
private VOverlay touchActions;
578+
private SpreadsheetOverlay touchActions;
580579

581580
private boolean dragging;
582581

@@ -859,6 +858,7 @@ private void showTouchActions() {
859858
}
860859

861860
touchActions = new SpreadsheetOverlay(true);
861+
touchActions.setOverlayContainer(sheetWidget.getOverlayContainer());
862862
touchActions.setOwner((Widget) sheetWidget.actionHandler);
863863
touchActions.addStyleName("v-contextmenu");
864864

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

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,6 @@
7474
import com.vaadin.client.WidgetUtil;
7575
import com.vaadin.client.ui.VLabel;
7676
import com.vaadin.client.ui.VLazyExecutor;
77-
import com.vaadin.client.ui.VOverlay;
7877

7978
public class SheetWidget extends Panel {
8079

@@ -117,9 +116,9 @@ public class SheetWidget extends Panel {
117116

118117
private final SelectionWidget selectionWidget;
119118

120-
private final VOverlay hyperlinkTooltip;
119+
private final SpreadsheetOverlay hyperlinkTooltip;
121120

122-
private final VOverlay resizeTooltip;
121+
private final SpreadsheetOverlay resizeTooltip;
123122

124123
private final CellComment cellCommentOverlay;
125124

@@ -385,6 +384,7 @@ public class SheetWidget extends Panel {
385384

386385
private Element host;
387386
private Node renderRoot;
387+
private Element overlayContainer;
388388

389389
static class CellCoord {
390390
private int col;
@@ -4153,6 +4153,7 @@ public void setCellCommentVisible(boolean visible, String key) {
41534153

41544154
final CellComment cellComment = new CellComment(this,
41554155
cell.getElement().getParentElement());
4156+
cellComment.setOverlayContainer(overlayContainer);
41564157
cellComment.setAuthor(cellCommentAuthorsMap.get(key));
41574158
cellComment.setCommentText(cellCommentsMap.get(key));
41584159
String errorMessage = invalidFormulaCells.contains(key)
@@ -6958,8 +6959,19 @@ public void setInvalidFormulaMessage(String invalidFormulaMessage) {
69586959
updateAllVisibleComments();
69596960
}
69606961

6961-
public void setHost(Element host, Node renderRoot) {
6962+
public void setHost(Element host, Node renderRoot,
6963+
Element overlayContainer) {
69626964
this.host = host;
69636965
this.renderRoot = renderRoot;
6966+
this.overlayContainer = overlayContainer;
6967+
// Wire the container into overlays that were eagerly created in the
6968+
// constructor (before the container was known).
6969+
hyperlinkTooltip.setOverlayContainer(overlayContainer);
6970+
resizeTooltip.setOverlayContainer(overlayContainer);
6971+
cellCommentOverlay.setOverlayContainer(overlayContainer);
6972+
}
6973+
6974+
public Element getOverlayContainer() {
6975+
return overlayContainer;
69646976
}
69656977
}

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,7 @@ public void onElementResize(ElementResizeEvent e) {
215215
private SpreadsheetServerRpcImpl serverRPC;
216216

217217
private Element host;
218+
private Element overlayContainer;
218219

219220
private HashMap<String, Slot> customEditors = null;
220221

@@ -231,6 +232,9 @@ protected <T extends ServerRpc> T getRpcProxy(Class<T> rpcInterface) {
231232
@Override
232233
protected void init() {
233234
super.init();
235+
getConnection()
236+
.setContextMenu(new SpreadsheetOverlay.SpreadsheetContextMenu(
237+
overlayContainer));
234238
getWidget().setId(getConnectorId());
235239
registerRpc(SpreadsheetClientRpc.class, clientRPC);
236240
getWidget().setCommsTrigger(new CommsTrigger() {
@@ -610,8 +614,14 @@ public interface CommsTrigger {
610614
void sendUpdates();
611615
}
612616

613-
public void setHost(Element host, Node renderRoot) {
617+
public void setHost(Element host, Node renderRoot,
618+
Element overlayContainer) {
614619
this.host = host;
615-
getWidget().setHost(host, renderRoot);
620+
this.overlayContainer = overlayContainer;
621+
getWidget().setHost(host, renderRoot, overlayContainer);
622+
}
623+
624+
public Element getOverlayContainer() {
625+
return overlayContainer;
616626
}
617627
}

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

Lines changed: 69 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,53 +8,107 @@
88
*/
99
package com.vaadin.addon.spreadsheet.client;
1010

11+
import com.google.gwt.dom.client.Element;
1112
import com.google.gwt.user.client.DOM;
12-
import com.google.gwt.user.client.Element;
13-
import com.google.gwt.user.client.ui.RootPanel;
1413
import com.vaadin.client.ui.VContextMenu;
1514
import com.vaadin.client.ui.VOverlay;
1615

1716
/**
18-
* A VOverlay Implementation that attaches the overlay to the container added by
19-
* vaadin-spreadsheet webcomponent
17+
* A VOverlay implementation that attaches the overlay to the per-instance
18+
* container created by the {@code <vaadin-spreadsheet>} custom element. The
19+
* container lives inside the spreadsheet element (light DOM), so the overlay
20+
* inherits {@code pointer-events: auto} from a containing modal Dialog overlay
21+
* — fixing the regression where context menus were visually shown (via native
22+
* popover) but not clickable.
2023
*/
21-
@SuppressWarnings("deprecation")
24+
@SuppressWarnings({ "deprecation", "java:S1699" })
2225
public class SpreadsheetOverlay extends VOverlay {
2326

27+
private static final String POPOVER_ATTRIBUTE = "popover";
28+
29+
private Element overlayContainer;
30+
2431
/**
25-
* A VContextMenu Implementation that attaches the overlay to the container
26-
* added by vaadin-spreadsheet webcomponent
32+
* A VContextMenu implementation that attaches the overlay to the
33+
* per-instance container.
2734
*/
2835
public static class SpreadsheetContextMenu extends VContextMenu {
29-
public SpreadsheetContextMenu() {
36+
37+
private Element overlayContainer;
38+
39+
public SpreadsheetContextMenu(Element overlayContainer) {
40+
this.overlayContainer = overlayContainer;
3041
DOM.setElementProperty(getElement(), "id", "PID_VAADIN_CM");
42+
setPopover(getElement());
43+
}
44+
45+
@Override
46+
public com.google.gwt.user.client.Element getOverlayContainer() {
47+
return asUserElement(overlayContainer);
48+
}
49+
50+
@Override
51+
public void show() {
52+
super.show();
53+
showPopover(getElement());
3154
}
3255

3356
@Override
34-
public Element getOverlayContainer() {
35-
return getOverlayContainerElement();
57+
public void showAt(int x, int y) {
58+
super.showAt(x, y);
59+
showPopover(getElement());
3660
}
3761
}
3862

3963
public SpreadsheetOverlay() {
4064
super();
65+
setPopover(getElement());
4166
}
4267

4368
public SpreadsheetOverlay(boolean autoHide, boolean modal) {
4469
super(autoHide, modal);
70+
setPopover(getElement());
4571
}
4672

4773
public SpreadsheetOverlay(boolean autoHide) {
4874
super(autoHide);
75+
setPopover(getElement());
76+
}
77+
78+
public void setOverlayContainer(Element overlayContainer) {
79+
this.overlayContainer = overlayContainer;
80+
}
81+
82+
@Override
83+
public com.google.gwt.user.client.Element getOverlayContainer() {
84+
return asUserElement(overlayContainer);
4985
}
5086

5187
@Override
52-
public Element getOverlayContainer() {
53-
return getOverlayContainerElement();
88+
public void show() {
89+
super.show();
90+
showPopover(getElement());
5491
}
5592

56-
private static Element getOverlayContainerElement() {
57-
Element overlays = DOM.getElementById("spreadsheet-overlays");
58-
return overlays == null ? RootPanel.getBodyElement() : overlays;
93+
private static void setPopover(Element el) {
94+
if (el != null) {
95+
el.setAttribute(POPOVER_ATTRIBUTE, "manual");
96+
}
97+
}
98+
99+
private static com.google.gwt.user.client.Element asUserElement(
100+
Element el) {
101+
return el == null ? null
102+
: el.<com.google.gwt.user.client.Element> cast();
59103
}
104+
105+
// @formatter:off
106+
private static native void showPopover(Element el)
107+
/*-{
108+
var fn = el && el.showPopover;
109+
if (typeof fn === "function") {
110+
fn.call(el);
111+
}
112+
}-*/;
113+
// @formatter:on
60114
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2212,8 +2212,9 @@ public void setNamedRanges(List<String> namedRanges) {
22122212
formulaBarWidget.setNamedRanges(namedRanges);
22132213
}
22142214

2215-
public void setHost(Element host, Node renderRoot) {
2216-
sheetWidget.setHost(host, renderRoot);
2215+
public void setHost(Element host, Node renderRoot,
2216+
Element overlayContainer) {
2217+
sheetWidget.setHost(host, renderRoot, overlayContainer);
22172218
}
22182219

22192220
/**

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

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import com.google.gwt.user.client.ui.HasWidgets;
2828
import com.google.gwt.user.client.ui.RootPanel;
2929
import com.google.gwt.user.client.ui.Widget;
30-
import com.vaadin.addon.spreadsheet.client.SpreadsheetOverlay;
3130
import com.vaadin.client.ApplicationConfiguration.ErrorMessage;
3231
import com.vaadin.client.communication.ConnectionStateHandler;
3332
import com.vaadin.client.communication.Heartbeat;
@@ -853,18 +852,28 @@ public void setResource(String name, String resource) {
853852
}
854853

855854
/**
856-
* Singleton method to get instance of app's context menu.
855+
* Returns the context menu associated with this connection, previously set
856+
* via {@link #setContextMenu(VContextMenu)}.
857857
*
858-
* @return VContextMenu object
858+
* @return VContextMenu object, or {@code null} if not yet assigned
859859
*/
860860
public VContextMenu getContextMenu() {
861-
if (contextMenu == null) {
862-
contextMenu = new SpreadsheetOverlay.SpreadsheetContextMenu();
863-
contextMenu.setOwner(uIConnector.getWidget());
864-
}
865861
return contextMenu;
866862
}
867863

864+
/**
865+
* Assigns the context menu instance for this connection and wires its owner
866+
* to the connection's UI widget. Called by the spreadsheet connector once
867+
* the overlay container is known.
868+
*
869+
* @param contextMenu
870+
* the context menu instance, not null
871+
*/
872+
public void setContextMenu(VContextMenu contextMenu) {
873+
this.contextMenu = contextMenu;
874+
contextMenu.setOwner(uIConnector.getWidget());
875+
}
876+
868877
/**
869878
* Gets an {@link Icon} instance corresponding to a URI.
870879
*

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,23 +50,28 @@ public class SpreadsheetJsApi {
5050
private Map<String, PopupButtonState> popupButtonStates = new HashMap<>();
5151

5252
/**
53-
* receives the host element and the render root where the widget must be
54-
* embedded into, and publishes the methods which can be used from JS
53+
* receives the host element, the render root where the widget must be
54+
* embedded into, and the overlay container element where spreadsheet
55+
* overlays (context menu, tooltips, comments, popup buttons) will be
56+
* attached. Publishes the methods which can be used from JS.
5557
*
5658
* @param host
5759
* the host element
5860
* @param renderRoot
5961
* render root of the host
62+
* @param overlayContainer
63+
* element that receives spreadsheet overlays
6064
*/
61-
public SpreadsheetJsApi(Element host, Node renderRoot) {
65+
public SpreadsheetJsApi(Element host, Node renderRoot,
66+
Element overlayContainer) {
6267
if (host != null) {
63-
init(host, renderRoot);
68+
init(host, renderRoot, overlayContainer);
6469
}
6570
}
6671

67-
private void init(Element host, Node renderRoot) {
72+
private void init(Element host, Node renderRoot, Element overlayContainer) {
6873
spreadsheetConnector = new SpreadsheetConnector();
69-
spreadsheetConnector.setHost(host, renderRoot);
74+
spreadsheetConnector.setHost(host, renderRoot, overlayContainer);
7075
spreadsheetConnector.doInit("1", new ApplicationConnection());
7176
spreadsheetWidget = spreadsheetConnector.getWidget();
7277
RootPanel.getForElement((Element) renderRoot).add(spreadsheetWidget);

‎vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow-client/src/test/java/com/vaadin/component/spreadsheet/client/js/SpreadsheetJsApiTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ class SpreadsheetJsApiTest {
4242

4343
class SpreadsheetJsApiHack extends SpreadsheetJsApi {
4444
public SpreadsheetJsApiHack(SpreadsheetConnector connector) {
45-
super(null, null);
45+
super(null, null, null);
4646
spreadsheetConnector = connector;
4747
}
4848

0 commit comments

Comments
 (0)