fix: sub-menu event listeners leaking when closing sub-menus (#888)

- when a sub-menu is closed, we remove it from the DOM but we also need to remove any click event listeners attached to all list items

src/controls/slick.gridmenu.ts

@@ -380,14 +380,16 @@ export class SlickGridMenu {
 
   /** Close and destroy all previously opened sub-menus */
   destroySubMenus() {
+    this._bindingEventService.unbindAll('sub-menu');
     document.querySelectorAll(`.slick-gridmenu.slick-submenu${this.getGridUidSelector()}`)
       .forEach(subElm => subElm.remove());
   }
 
   /** Construct the custom command menu items. */
   protected populateCommandsMenu(commandItems: Array<GridMenuItem | MenuCommandItem | 'divider'>, commandListElm: HTMLElement, args: { grid: SlickGrid, level: number }) {
     // user could pass a title on top of the custom section
-    const isSubMenu = args.level > 0;
+    const level = args?.level || 0;
+    const isSubMenu = level > 0;
     if (!isSubMenu && (this._gridMenuOptions?.commandTitle || this._gridMenuOptions?.customTitle)) {
       this._commandTitleElm = document.createElement('div');
       this._commandTitleElm.className = 'title';
@@ -470,14 +472,15 @@ export class SlickGridMenu {
       commandListElm.appendChild(liElm);
 
       if (addClickListener) {
-        this._bindingEventService.bind(liElm, 'click', this.handleMenuItemClick.bind(this, item, args.level) as EventListener);
+        const eventGroup = isSubMenu ? 'sub-menu' : 'parent-menu';
+        this._bindingEventService.bind(liElm, 'click', this.handleMenuItemClick.bind(this, item, level) as EventListener, undefined, eventGroup);
       }
 
       // optionally open sub-menu(s) by mouseover
       if (this._gridMenuOptions?.subMenuOpenByEvent === 'mouseover') {
         this._bindingEventService.bind(liElm, 'mouseover', ((e: DOMMouseOrTouchEvent<HTMLDivElement>) => {
           if ((item as GridMenuItem).commandItems || (item as GridMenuItem).customItems) {
-            this.repositionSubMenu(item, args.level, e);
+            this.repositionSubMenu(item, level, e);
           } else if (!isSubMenu) {
             this.destroySubMenus();
           }

src/models/core.interface.ts

@@ -4,6 +4,7 @@ export interface ElementEventListener {
   element: Element | Window;
   eventName: string;
   listener: EventListenerOrEventListenerObject;
+  groupName?: string;
 }
 
 export interface EditController {

src/plugins/slick.cellmenu.ts

@@ -434,12 +434,16 @@ export class SlickCellMenu implements SlickPlugin {
   /** Destroy all parent menus and any sub-menus */
   destroyAllMenus() {
     this.destroySubMenus();
+
+    // remove all parent menu listeners before removing them from the DOM
+    this._bindingEventService.unbindAll('parent-menu');
     document.querySelectorAll(`.slick-cell-menu${this.getGridUidSelector()}`)
       .forEach(subElm => subElm.remove());
   }
 
   /** Close and destroy all previously opened sub-menus */
   destroySubMenus() {
+    this._bindingEventService.unbindAll('sub-menu');
     document.querySelectorAll(`.slick-cell-menu.slick-submenu${this.getGridUidSelector()}`)
       .forEach(subElm => subElm.remove());
   }
@@ -623,7 +627,8 @@ export class SlickCellMenu implements SlickPlugin {
     }
 
     // user could pass a title on top of the Commands/Options section
-    const isSubMenu = args.level > 0;
+    const level = args?.level || 0;
+    const isSubMenu = level > 0;
     if (cellMenu?.[`${itemType}Title`] && !isSubMenu) {
       this[`_${itemType}TitleElm`] = document.createElement('div');
       this[`_${itemType}TitleElm`]!.className = 'slick-menu-title';
@@ -703,14 +708,15 @@ export class SlickCellMenu implements SlickPlugin {
       commandOrOptionMenuElm.appendChild(liElm);
 
       if (addClickListener) {
-        this._bindingEventService.bind(liElm, 'click', this.handleMenuItemClick.bind(this, item, itemType, args.level) as EventListener);
+        const eventGroup = isSubMenu ? 'sub-menu' : 'parent-menu';
+        this._bindingEventService.bind(liElm, 'click', this.handleMenuItemClick.bind(this, item, itemType, level) as EventListener, undefined, eventGroup);
       }
 
       // optionally open sub-menu(s) by mouseover
       if (this._cellMenuProperties.subMenuOpenByEvent === 'mouseover') {
         this._bindingEventService.bind(liElm, 'mouseover', ((e: DOMMouseOrTouchEvent<HTMLDivElement>) => {
           if ((item as MenuCommandItem).commandItems || (item as MenuOptionItem).optionItems) {
-            this.repositionSubMenu(item, itemType, args.level, e);
+            this.repositionSubMenu(item, itemType, level, e);
             this._lastMenuTypeClicked = itemType;
           } else if (!isSubMenu) {
             this.destroySubMenus();

src/plugins/slick.contextmenu.ts

@@ -450,12 +450,16 @@ export class SlickContextMenu implements SlickPlugin {
   /** Destroy all parent menus and any sub-menus */
   destroyAllMenus() {
     this.destroySubMenus();
+
+    // remove all parent menu listeners before removing them from the DOM
+    this._bindingEventService.unbindAll('parent-menu');
     document.querySelectorAll(`.slick-context-menu${this.getGridUidSelector()}`)
       .forEach(subElm => subElm.remove());
   }
 
   /** Close and destroy all previously opened sub-menus */
   destroySubMenus() {
+    this._bindingEventService.unbindAll('sub-menu');
     document.querySelectorAll(`.slick-context-menu.slick-submenu${this.getGridUidSelector()}`)
       .forEach(subElm => subElm.remove());
   }
@@ -551,7 +555,8 @@ export class SlickContextMenu implements SlickPlugin {
     }
 
     // user could pass a title on top of the Commands/Options section
-    const isSubMenu = args.level > 0;
+    const level = args?.level || 0;
+    const isSubMenu = level > 0;
     if (contextMenu?.[`${itemType}Title`] && !isSubMenu) {
       this[`_${itemType}TitleElm`] = document.createElement('div');
       this[`_${itemType}TitleElm`]!.className = 'slick-menu-title';
@@ -631,14 +636,15 @@ export class SlickContextMenu implements SlickPlugin {
       commandOrOptionMenuElm.appendChild(liElm);
 
       if (addClickListener) {
-        this._bindingEventService.bind(liElm, 'click', this.handleMenuItemClick.bind(this, item, itemType, args.level) as EventListener);
+        const eventGroup = isSubMenu ? 'sub-menu' : 'parent-menu';
+        this._bindingEventService.bind(liElm, 'click', this.handleMenuItemClick.bind(this, item, itemType, level) as EventListener, undefined, eventGroup);
       }
 
       // optionally open sub-menu(s) by mouseover
       if (this._contextMenuProperties.subMenuOpenByEvent === 'mouseover') {
         this._bindingEventService.bind(liElm, 'mouseover', ((e: DOMMouseOrTouchEvent<HTMLDivElement>) => {
           if ((item as MenuCommandItem).commandItems || (item as MenuOptionItem).optionItems) {
-            this.repositionSubMenu(item, itemType, args.level, e);
+            this.repositionSubMenu(item, itemType, level, e);
             this._lastMenuTypeClicked = itemType;
           } else if (!isSubMenu) {
             this.destroySubMenus();

src/plugins/slick.headermenu.ts

@@ -171,12 +171,16 @@ export class SlickHeaderMenu implements SlickPlugin {
 
   destroyAllMenus() {
     this.destroySubMenus();
+
+    // remove all parent menu listeners before removing them from the DOM
+    this._bindingEventService.unbindAll('parent-menu');
     document.querySelectorAll(`.slick-header-menu${this.getGridUidSelector()}`)
       .forEach(subElm => subElm.remove());
   }
 
   /** Close and destroy all previously opened sub-menus */
   destroySubMenus() {
+    this._bindingEventService.unbindAll('sub-menu');
     document.querySelectorAll(`.slick-header-menu.slick-submenu${this.getGridUidSelector()}`)
       .forEach(subElm => subElm.remove());
   }
@@ -425,7 +429,8 @@ export class SlickHeaderMenu implements SlickPlugin {
       menuElm.appendChild(menuItemElm);
 
       if (addClickListener) {
-        this._bindingEventService.bind(menuItemElm, 'click', this.handleMenuItemClick.bind(this, item, columnDef, level) as EventListener);
+        const eventGroup = isSubMenu ? 'sub-menu' : 'parent-menu';
+        this._bindingEventService.bind(menuItemElm, 'click', this.handleMenuItemClick.bind(this, item, columnDef, level) as EventListener, undefined, eventGroup);
       }
 
       // optionally open sub-menu(s) by mouseover

src/slick.core.ts

@@ -552,9 +552,9 @@ export class BindingEventService {
   }
 
   /** Bind an event listener to any element */
-  bind(element: Element | Window, eventName: string, listener: EventListenerOrEventListenerObject, options?: boolean | AddEventListenerOptions) {
+  bind(element: Element | Window, eventName: string, listener: EventListenerOrEventListenerObject, options?: boolean | AddEventListenerOptions, groupName = '') {
     element.addEventListener(eventName, listener, options);
-    this._boundedEvents.push({ element, eventName, listener });
+    this._boundedEvents.push({ element, eventName, listener, groupName });
   }
 
   /** Unbind all will remove every every event handlers that were bounded earlier */
@@ -571,11 +571,30 @@ export class BindingEventService {
     }
   }
 
-  /** Unbind all will remove every every event handlers that were bounded earlier */
-  unbindAll() {
-    while (this._boundedEvents.length > 0) {
-      const boundedEvent = this._boundedEvents.pop() as ElementEventListener;
-      this.unbind(boundedEvent.element, boundedEvent.eventName, boundedEvent.listener);
+  /**
+   * Unbind all event listeners that were bounded, optionally provide a group name to unbind all listeners assigned to that specific group only.
+   */
+  unbindAll(groupName?: string | string[]) {
+    if (groupName) {
+      const groupNames = Array.isArray(groupName) ? groupName : [groupName];
+
+      // unbind only the bounded event with a specific group
+      // Note: we need to loop in reverse order to avoid array reindexing (causing index offset) after a splice is called
+      for (let i = this._boundedEvents.length - 1; i >= 0; --i) {
+        const boundedEvent = this._boundedEvents[i];
+        if (groupNames.some(g => g === boundedEvent.groupName)) {
+          const { element, eventName, listener } = boundedEvent;
+          this.unbind(element, eventName, listener);
+          this._boundedEvents.splice(i, 1);
+        }
+      }
+    } else {
+      // unbind everything
+      while (this._boundedEvents.length > 0) {
+        const boundedEvent = this._boundedEvents.pop() as ElementEventListener;
+        const { element, eventName, listener } = boundedEvent;
+        this.unbind(element, eventName, listener);
+      }
     }
   }
 }