Skip to content

Commit

Permalink
fix #948 Memory leak in menu and Popover.
Browse files Browse the repository at this point in the history
  • Loading branch information
vegegoku committed Jul 25, 2024
1 parent 57e42b0 commit 28250c8
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 25 deletions.
49 changes: 33 additions & 16 deletions domino-ui/src/main/java/org/dominokit/domino/ui/menu/Menu.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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(
() -> {
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -1378,7 +1398,6 @@ private void doOpen(boolean focus) {
menuHeader.get().insertFirst(backArrowContainer);
}
show();
DomGlobal.document.body.addEventListener("blur", lostFocusListener, true);
}
}

Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -47,21 +49,26 @@
*/
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(
EventType.click.getName(),
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.
Expand Down Expand Up @@ -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));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
*
Expand Down

0 comments on commit 28250c8

Please sign in to comment.