Skip to content

Commit 8f42c5a

Browse files
TatuLundmcollovati
andauthored
fix: Add popover attribute to system error container (#24093)
E.g. Dialog can obscure the message Fixes #24094 --------- Co-authored-by: Marco Collovati <marco@vaadin.com>
1 parent 3fbc59e commit 8f42c5a

File tree

4 files changed

+83
-5
lines changed

4 files changed

+83
-5
lines changed

flow-client/src/main/java/com/vaadin/client/SystemErrorHandler.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,8 @@ private Element handleError(String caption, String message, String details,
275275
String querySelector) {
276276
Document document = Browser.getDocument();
277277
Element systemErrorContainer = document.createDivElement();
278+
// Set the popover attribute for native popovers.
279+
systemErrorContainer.setAttribute("popover", "manual");
278280
systemErrorContainer.setClassName("v-system-error");
279281

280282
if (caption != null) {
@@ -311,10 +313,21 @@ private Element handleError(String caption, String message, String details,
311313
} else {
312314
document.getBody().appendChild(systemErrorContainer);
313315
}
316+
showPopover(systemErrorContainer);
314317

315318
return systemErrorContainer;
316319
}
317320

321+
// @formatter:off
322+
private native void showPopover(Element el)
323+
/*-{
324+
var fn = el && el.showPopover;
325+
if (typeof fn === "function") {
326+
fn.call(el);
327+
}
328+
}-*/;
329+
// @formatter:on
330+
318331
private static Throwable unwrapUmbrellaException(Throwable e) {
319332
if (e instanceof UmbrellaException) {
320333
Set<Throwable> causes = ((UmbrellaException) e).getCauses();

flow-server/src/main/java/com/vaadin/flow/server/BootstrapHandler.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1565,8 +1565,8 @@ protected static void setupErrorDialogs(Element style) {
15651565
"position: absolute;" +
15661566
"color: black;" +
15671567
"background: white;" +
1568-
"top: 1em;" +
1569-
"right: 1em;" +
1568+
"margin-top: 1em;" +
1569+
"margin-right: 1em;" +
15701570
"border: 1px solid black;" +
15711571
"padding: 1em;" +
15721572
"z-index: 10000;" +

flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/InternalErrorView.java

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818
import java.io.IOException;
1919

2020
import com.vaadin.flow.component.html.Div;
21+
import com.vaadin.flow.component.html.H3;
2122
import com.vaadin.flow.component.html.NativeButton;
23+
import com.vaadin.flow.dom.Style;
2224
import com.vaadin.flow.router.Route;
2325
import com.vaadin.flow.server.CustomizedSystemMessages;
2426
import com.vaadin.flow.server.DefaultSystemMessagesProvider;
@@ -57,9 +59,15 @@ public InternalErrorView() {
5759
"Reset system messages", "reset-system-messages",
5860
event -> resetSystemMessages());
5961

62+
Div popover = createPopover();
63+
NativeButton openPopoverButton = new NativeButton("Open popover");
64+
openPopoverButton.setId("open-popover");
65+
popover.getId().ifPresent(popoverId -> openPopoverButton.getElement()
66+
.setAttribute("popovertarget", popoverId));
67+
6068
add(message, updateMessageButton, closeSessionButton,
6169
enableNotificationButton, causeExceptionButton,
62-
resetSystemMessagesButton);
70+
openPopoverButton, resetSystemMessagesButton, popover);
6371
}
6472

6573
private void showInternalError() {
@@ -99,4 +107,20 @@ private void resetSystemMessages() {
99107
VaadinService.getCurrent()
100108
.setSystemMessagesProvider(DefaultSystemMessagesProvider.get());
101109
}
110+
111+
private Div createPopover() {
112+
Div popover = new Div();
113+
popover.setId("popover-div");
114+
popover.getElement().setAttribute("popover", "manual");
115+
popover.setSizeFull();
116+
popover.getStyle().setPosition(Style.Position.ABSOLUTE);
117+
popover.getStyle().setPadding("10px");
118+
popover.getStyle().setBorder("2px solid black");
119+
popover.add(new H3("Modal dialog"));
120+
NativeButton causeExceptionButton = createButton("Cause exception",
121+
"cause-exception-popover", event -> showInternalError());
122+
popover.add(causeExceptionButton);
123+
return popover;
124+
}
125+
102126
}

flow-tests/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/InternalErrorIT.java

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ public void internalError_showNotification_clickNotification_refresh() {
116116
+ "a server-side exception",
117117
isInternalErrorNotificationPresent());
118118

119-
getErrorNotification().click();
119+
clickErrorNotification();
120120
try {
121121
waitUntil(driver -> !isMessageUpdated());
122122
} catch (TimeoutException e) {
@@ -154,6 +154,33 @@ public void internalError_showNotification_clickEsc_refresh() {
154154
isInternalErrorNotificationPresent());
155155
}
156156

157+
@Test
158+
public void popover_showNotification_notificationInteractable() {
159+
clickButton(UPDATE);
160+
161+
clickButton("open-popover");
162+
waitForElementPresent(By.id("cause-exception-popover"));
163+
clickButton("cause-exception-popover");
164+
165+
Assert.assertTrue("The page should not be immediately refreshed after "
166+
+ "a server-side exception", isMessageUpdated());
167+
Assert.assertTrue(
168+
"'Internal error' notification should be present after "
169+
+ "a server-side exception",
170+
isInternalErrorNotificationPresent());
171+
172+
clickErrorNotification();
173+
try {
174+
waitUntil(driver -> !isMessageUpdated());
175+
} catch (TimeoutException e) {
176+
Assert.fail("After internal error, clicking the notification "
177+
+ "should refresh the page, resetting the state of the UI.");
178+
}
179+
Assert.assertFalse(
180+
"'Internal error' notification should be gone after refreshing",
181+
isInternalErrorNotificationPresent());
182+
}
183+
157184
@After
158185
public void resetSystemMessages() {
159186
waitUntil(ExpectedConditions
@@ -177,7 +204,13 @@ private boolean isErrorNotificationPresent(String text) {
177204
if (!isElementPresent(By.className("v-system-error"))) {
178205
return false;
179206
}
180-
return getErrorNotification().getAttribute("innerHTML").contains(text);
207+
WebElement notification = getErrorNotification();
208+
notification = ExpectedConditions.elementToBeClickable(notification)
209+
.apply(driver);
210+
if (notification == null) {
211+
return false;
212+
}
213+
return notification.getText().contains(text);
181214
}
182215

183216
private WebElement getErrorNotification() {
@@ -187,4 +220,12 @@ private WebElement getErrorNotification() {
187220
private void clickButton(String id) {
188221
findElement(By.id(id)).click();
189222
}
223+
224+
private void clickErrorNotification() {
225+
// Do not use click() because it currently invokes JavaScript function
226+
// instead of clicking on the element
227+
new Actions(getDriver()).click(getErrorNotification()).build()
228+
.perform();
229+
}
230+
190231
}

0 commit comments

Comments
 (0)