Fix issues found in Copilot review iteration 4

Author: wenzhixin

src/utils/events.js

@@ -5,7 +5,7 @@
 
 import DOMHelper from '../helpers/dom.js'
 
-class EventHelper {
+export class EventHelper {
   /**
    * Add event listener with namespace support
    * @param {Element|string} element - DOM element or selector
@@ -270,36 +270,73 @@ class EventHelper {
    */
   static offAll (element, namespace) {
     if (typeof element === 'string') element = DOMHelper.$(element)
-    if (!element || !element._eventHandlers) return element
+    if (!element) return element
+
+    // Clean up non-delegated event handlers first
+    if (element._eventHandlers) {
+      // Iterate through all event types
+      Object.keys(element._eventHandlers).forEach(eventType => {
+        if (namespace) {
+          // Remove only handlers matching the namespace
+          element._eventHandlers[eventType] = element._eventHandlers[eventType].filter(info => {
+            if (info && info.namespace === namespace) {
+              element.removeEventListener(eventType, info.wrappedHandler, info.options)
+              return false
+            }
+            return true
+          })
+        } else {
+          // Remove all handlers for this event type
+          element._eventHandlers[eventType].forEach(info => {
+            if (info) {
+              element.removeEventListener(eventType, info.wrappedHandler, info.options)
+            }
+          })
+          element._eventHandlers[eventType] = []
+        }
 
-    // Iterate through all event types
-    Object.keys(element._eventHandlers).forEach(eventType => {
+        // Clean up empty event type arrays
+        if (!element._eventHandlers[eventType] || element._eventHandlers[eventType].length === 0) {
+          delete element._eventHandlers[eventType]
+        }
+      })
+
+      // Clean up if no handlers left at all
+      if (Object.keys(element._eventHandlers).length === 0) {
+        delete element._eventHandlers
+      }
+    }
+
+    // Also clean up any delegated handlers tied to this element
+    if (element._delegates) {
       if (namespace) {
-        // Remove only handlers matching the namespace
-        element._eventHandlers[eventType] = element._eventHandlers[eventType].filter(info => {
-          if (info.namespace === namespace) {
-            element.removeEventListener(eventType, info.wrappedHandler, info.options)
+        // Remove only delegates matching the namespace
+        element._delegates = element._delegates.filter(delegate => {
+          if (delegate.namespace === namespace) {
+            // Remove the underlying event listener
+            const eventWithNamespace = delegate.namespace
+              ? `${delegate.eventType}.${delegate.namespace}`
+              : delegate.eventType
+            this.off(element, eventWithNamespace, delegate.handler)
             return false
           }
           return true
         })
       } else {
-        // Remove all handlers for this event type
-        element._eventHandlers[eventType].forEach(info => {
-          element.removeEventListener(eventType, info.wrappedHandler, info.options)
+        // Remove all delegates
+        element._delegates.forEach(delegate => {
+          const eventWithNamespace = delegate.namespace
+            ? `${delegate.eventType}.${delegate.namespace}`
+            : delegate.eventType
+          this.off(element, eventWithNamespace, delegate.handler)
         })
-        element._eventHandlers[eventType] = []
+        element._delegates = []
       }
 
-      // Clean up empty event type arrays
-      if (element._eventHandlers[eventType].length === 0) {
-        delete element._eventHandlers[eventType]
+      // Clean up delegates storage if no delegates remain
+      if (element._delegates.length === 0) {
+        delete element._delegates
       }
-    })
-
-    // Clean up if no handlers left at all
-    if (Object.keys(element._eventHandlers).length === 0) {
-      delete element._eventHandlers
     }
 
     return element
@@ -308,7 +345,9 @@ class EventHelper {
   /**
    * Native DOM helper methods
    */
-  static $ = DOMHelper.$.bind(DOMHelper)
+  static $ (...args) {
+    return DOMHelper.$.apply(DOMHelper, args)
+  }
 
   /**
    * Find parent element matching selector
@@ -344,7 +383,7 @@ class EventHelper {
    * Get data attribute value
    * @param {Element|string} element - DOM element or selector
    * @param {string} key - Data key
-   * @returns {string|null} The data attribute value or null
+   * @returns {string|undefined|null} The data attribute value, undefined if the key is missing, or null if the element is not found
    */
   static getData (element, key) {
     if (typeof element === 'string') element = DOMHelper.$(element)
@@ -368,5 +407,3 @@ class EventHelper {
     return element
   }
 }
-
-export default EventHelper

tests/utils/events.test.js

@@ -3,7 +3,7 @@
  */
 
 import { beforeEach, describe, expect, it } from 'vitest'
-import EventHelper from '@/utils/events.js'
+import { EventHelper } from '@/utils/events.js'
 import DOMHelper from '@/helpers/dom.js'
 
 // Setup test environment
@@ -319,7 +319,7 @@ describe('EventHelper', () => {
       expect(EventHelper.trigger(null, 'click')).toBeNull()
     })
 
-    it('should handle invalid selectors gracefully', () => {
+    it('should handle non-existent selectors gracefully', () => {
       expect(EventHelper.on('#non-existent', 'click', () => {})).toBeNull()
     })