Skip to content

Commit

Permalink
fix: preserve components added to notification on reload (#3913) (#4199)
Browse files Browse the repository at this point in the history
* fix: preserve components added to notification on reload

* fix not switching to component renderer when already attached

Co-authored-by: Tomi Virkki <[email protected]>

Co-authored-by: Sascha Ißbrücker <[email protected]>
Co-authored-by: Tomi Virkki <[email protected]>
  • Loading branch information
3 people authored Nov 24, 2022
1 parent a16d492 commit ca95032
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.vaadin.flow.component.html.Div;
import com.vaadin.flow.component.html.NativeButton;
import com.vaadin.flow.component.html.Span;
import com.vaadin.flow.component.notification.Notification;
import com.vaadin.flow.router.PreserveOnRefresh;
import com.vaadin.flow.router.Route;
Expand All @@ -26,5 +27,14 @@ public PreserveOnRefreshPage() {
});
closeNotification.setId("close-notification");
add(closeNotification);

NativeButton addComponent = new NativeButton(
"Add component to notification", e -> {
Span span = new Span("Component content");
span.setId("component-content");
notification.add(span);
});
addComponent.setId("add-component");
add(addComponent);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.vaadin.flow.testutil.TestPath;
import com.vaadin.testbench.TestBenchElement;
import com.vaadin.tests.AbstractComponentIT;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;

Expand All @@ -12,12 +13,14 @@ public class PreserveOnRefreshIT extends AbstractComponentIT {

private TestBenchElement openNotification;
private TestBenchElement closeNotification;
private TestBenchElement addComponent;

@Before
public void init() {
open();
openNotification = $("button").id("show-notification");
closeNotification = $("button").id("close-notification");
addComponent = $("button").id("add-component");
}

@Test
Expand Down Expand Up @@ -45,6 +48,21 @@ public void open_reload_close_reload_shouldStayClosed() {
assertNotificationIsClosed();
}

@Test
public void addComponent_open_reload_shouldContainComponent() {
addComponent.click();
openNotification.click();
assertNotificationIsOpen();

getDriver().navigate().refresh();
TestBenchElement notification = $(NOTIFICATION_TAG).first();
boolean containsComponentContent = notification.$("span")
.attribute("id", "component-content").exists();
Assert.assertTrue(
"Notification card does not contain added component anymore",
containsComponentContent);
}

private void assertNotificationIsOpen() {
$(NOTIFICATION_TAG).waitForFirst();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,14 @@ public class Notification extends GeneratedVaadinNotification<Notification>

private Registration afterProgrammaticNavigationListenerRegistration;

private SerializableConsumer<UI> deferredJob = new AttachComponentTemplate();
private SerializableConsumer<UI> configureTemplate = new ConfigureComponentRenderer();

private class AttachComponentTemplate implements SerializableConsumer<UI> {
private class ConfigureComponentRenderer
implements SerializableConsumer<UI> {

@Override
public void accept(UI ui) {
if (this == deferredJob) {
if (this == configureTemplate) {
String appId = ui.getInternals().getAppId();
int nodeId = container.getNode().getId();
String template = String.format(
Expand Down Expand Up @@ -131,8 +132,6 @@ static Position fromClientName(String clientName) {
*/
public Notification() {
initBaseElementsAndListeners();
getElement().getNode().runWhenAttached(ui -> ui
.beforeClientResponse(this, context -> deferredJob.accept(ui)));
setPosition(DEFAULT_POSITION);
setDuration(0);
}
Expand Down Expand Up @@ -270,7 +269,7 @@ public static Notification show(String text) {
*/
public void setText(String text) {
removeAll();
deferredJob = NO_OP;
configureTemplate = NO_OP;
templateElement.setProperty("innerHTML", HtmlUtils.escape(text));
}

Expand Down Expand Up @@ -351,7 +350,7 @@ public void add(Component... components) {
"Component to add cannot be null");
container.appendChild(component.getElement());
}
attachComponentTemplate();
configureComponentRenderer();
}

/**
Expand Down Expand Up @@ -402,7 +401,7 @@ public void addComponentAtIndex(int index, Component component) {
// inside the method below
container.insertChild(index, component.getElement());

attachComponentTemplate();
configureComponentRenderer();
}

/**
Expand Down Expand Up @@ -575,16 +574,17 @@ public void removeThemeVariants(NotificationVariant... variants) {
.collect(Collectors.toList()));
}

private void attachComponentTemplate() {
deferredJob = new AttachComponentTemplate();
getElement().getNode().runWhenAttached(ui -> ui
.beforeClientResponse(this, context -> deferredJob.accept(ui)));
private void configureComponentRenderer() {
configureTemplate = new ConfigureComponentRenderer();
getElement().getNode()
.runWhenAttached(ui -> configureTemplate.accept(ui));
}

@Override
protected void onAttach(AttachEvent attachEvent) {
super.onAttach(attachEvent);
initConnector();
configureTemplate.accept(attachEvent.getUI());
}

@Override
Expand Down

0 comments on commit ca95032

Please sign in to comment.