From 28250c8b6d9759de5be660721a89577b22138a37 Mon Sep 17 00:00:00 2001
From: "Ahmad K. Bawaneh" <ahmad.bawaneh@dominokit.com>
Date: Thu, 25 Jul 2024 20:44:19 +0300
Subject: [PATCH] fix #948 Memory leak in menu and Popover.
---
.../org/dominokit/domino/ui/menu/Menu.java | 49 +++++++++++++------
.../dominokit/domino/ui/popover/Popover.java | 17 +++----
.../domino/ui/utils/BaseDominoElement.java | 35 +++++++++++++
3 files changed, 76 insertions(+), 25 deletions(-)
diff --git a/domino-ui/src/main/java/org/dominokit/domino/ui/menu/Menu.java b/domino-ui/src/main/java/org/dominokit/domino/ui/menu/Menu.java
index 1bb8ed0c7..c4ec45df1 100644
--- a/domino-ui/src/main/java/org/dominokit/domino/ui/menu/Menu.java
+++ b/domino-ui/src/main/java/org/dominokit/domino/ui/menu/Menu.java
@@ -156,6 +156,7 @@ public class Menu<V> extends BaseDominoElement<HTMLDivElement, Menu<V>>
private EventListener lostFocusListener;
private boolean closeOnBlur = DominoUIConfig.CONFIG.isClosePopupOnBlur();
private OpenMenuCondition<V> openMenuCondition = (menu) -> true;
+ private List<MediaQuery.MediaQueryListenerRecord> mediaQueryRecords = new ArrayList<>();
/**
* Factory method to create a new Menu instance.
@@ -355,19 +356,6 @@ public Menu() {
element.addEventListener("keydown", keyboardNavigation);
- MediaQuery.addOnSmallAndDownListener(
- () -> {
- if (centerOnSmallScreens) {
- this.smallScreen = true;
- }
- });
- MediaQuery.addOnMediumAndUpListener(
- () -> {
- if (centerOnSmallScreens) {
- this.smallScreen = false;
- backArrowContainer.remove();
- }
- });
backIcon = LazyChild.of(Icons.keyboard_backspace().addCss(dui_menu_back_icon), menuHeader);
backIcon.whenInitialized(
() -> {
@@ -403,6 +391,38 @@ public Menu() {
}
};
+ nowAndWhenAttached(
+ () -> {
+ mediaQueryRecords.add(
+ MediaQuery.addOnSmallAndDownListener(
+ () -> {
+ if (centerOnSmallScreens) {
+ this.smallScreen = true;
+ }
+ }));
+
+ mediaQueryRecords.add(
+ MediaQuery.addOnMediumAndUpListener(
+ () -> {
+ if (centerOnSmallScreens) {
+ this.smallScreen = false;
+ backArrowContainer.remove();
+ }
+ }));
+
+ DomGlobal.document.body.addEventListener("blur", lostFocusListener, true);
+ if (this.dropDown) {
+ document.addEventListener("scroll", repositionListener, true);
+ }
+ });
+
+ nowAndWhenDetached(
+ () -> {
+ DomGlobal.document.body.removeEventListener("blur", lostFocusListener, true);
+ document.removeEventListener("scroll", repositionListener, true);
+ mediaQueryRecords.forEach(MediaQuery.MediaQueryListenerRecord::remove);
+ });
+
this.addEventListener(EventType.touchstart.getName(), Event::stopPropagation);
this.addEventListener(EventType.touchend.getName(), Event::stopPropagation);
}
@@ -1378,7 +1398,6 @@ private void doOpen(boolean focus) {
menuHeader.get().insertFirst(backArrowContainer);
}
show();
- DomGlobal.document.body.addEventListener("blur", lostFocusListener, true);
}
}
@@ -1570,7 +1589,6 @@ public Menu<V> close() {
menuTarget -> {
menuTarget.getTargetElement().element().focus();
});
- DomGlobal.document.body.removeEventListener("blur", lostFocusListener, true);
if (isSearchable()) {
searchBox.get().clearSearch();
}
@@ -1785,7 +1803,6 @@ private void setDropDown(boolean dropdown) {
if (dropdown) {
this.setAttribute("domino-ui-root-menu", true).setAttribute(DOMINO_UI_AUTO_CLOSABLE, true);
menuElement.elevate(Elevation.LEVEL_1);
- document.addEventListener("scroll", repositionListener, true);
} else {
this.removeAttribute("domino-ui-root-menu").removeAttribute(DOMINO_UI_AUTO_CLOSABLE);
menuElement.elevate(Elevation.NONE);
diff --git a/domino-ui/src/main/java/org/dominokit/domino/ui/popover/Popover.java b/domino-ui/src/main/java/org/dominokit/domino/ui/popover/Popover.java
index d4caa916e..b664fd086 100644
--- a/domino-ui/src/main/java/org/dominokit/domino/ui/popover/Popover.java
+++ b/domino-ui/src/main/java/org/dominokit/domino/ui/popover/Popover.java
@@ -21,6 +21,8 @@
import elemental2.dom.EventListener;
import elemental2.dom.HTMLElement;
+import java.util.ArrayList;
+import java.util.List;
import org.dominokit.domino.ui.IsElement;
import org.dominokit.domino.ui.animations.Transition;
import org.dominokit.domino.ui.collapsible.AnimationCollapseStrategy;
@@ -47,6 +49,7 @@
*/
public class Popover extends BasePopover<Popover> {
+ private static boolean asDialog = false;
/** Static initialization block to add a global click event listener for closing popovers. */
static {
document.body.addEventListener(
@@ -54,14 +57,18 @@ public class Popover extends BasePopover<Popover> {
element -> {
ModalBackDrop.INSTANCE.closePopovers("");
});
+
+ MediaQuery.addOnSmallAndDownListener(() -> asDialog = true);
+
+ MediaQuery.addOnMediumAndUpListener(() -> asDialog = false);
}
private final EventListener showListener;
private boolean openOnClick = true;
private boolean closeOnEscape = true;
- private boolean asDialog = false;
private final DropDirection dialog = DropDirection.MIDDLE_SCREEN;
private boolean modal = false;
+ private final List<MediaQuery.MediaQueryListenerRecord> mediaListenersRecords = new ArrayList<>();
/**
* Creates a new `Popover` instance for the specified HTML element target.
@@ -102,14 +109,6 @@ public Popover(HTMLElement target) {
new AnimationCollapseStrategy(
Transition.FADE_IN, Transition.FADE_OUT, CollapsibleDuration._300ms));
- MediaQuery.addOnSmallAndDownListener(
- () -> {
- this.asDialog = true;
- });
- MediaQuery.addOnMediumAndUpListener(
- () -> {
- this.asDialog = false;
- });
addCollapseListener(() -> removeEventListener(DUI_REMOVE_POPOVERS, closeAllListener));
}
diff --git a/domino-ui/src/main/java/org/dominokit/domino/ui/utils/BaseDominoElement.java b/domino-ui/src/main/java/org/dominokit/domino/ui/utils/BaseDominoElement.java
index d3ead7fe6..fa22d7d7b 100644
--- a/domino-ui/src/main/java/org/dominokit/domino/ui/utils/BaseDominoElement.java
+++ b/domino-ui/src/main/java/org/dominokit/domino/ui/utils/BaseDominoElement.java
@@ -694,6 +694,24 @@ public T nowOrWhenAttached(Runnable handler) {
return (T) this;
}
+ /**
+ * Executes a given handler either immediately if the element is already detached from the DOM or
+ * when it gets detached.
+ *
+ * @param handler The handler to execute.
+ * @return The modified DOM element.
+ */
+ @Editor.Ignore
+ public T nowOrWhenDetached(Runnable handler) {
+ if (isAttached()) {
+ onDetached(mutationRecord -> handler.run());
+ } else {
+ handler.run();
+ }
+ dominoUuidInitializer.apply();
+ return (T) this;
+ }
+
/**
* Executes a given handler when the element is attached to the DOM. If the element is already
* attached, the handler is executed immediately.
@@ -711,6 +729,23 @@ public T nowAndWhenAttached(Runnable handler) {
return (T) this;
}
+ /**
+ * Executes a given handler when the element is detached from the DOM. If the element is already
+ * detached, the handler is executed immediately.
+ *
+ * @param handler The handler to execute.
+ * @return The modified DOM element.
+ */
+ @Editor.Ignore
+ public T nowAndWhenDetached(Runnable handler) {
+ if (!isAttached()) {
+ handler.run();
+ }
+ onDetached(mutationRecord -> handler.run());
+ dominoUuidInitializer.apply();
+ return (T) this;
+ }
+
/**
* Registers a resize handler to be notified when the size of this element changes.
*