Fix a second new on an auto-initialised element keeping old listeners (#3225)

.changeset/replace-existing-instance.md

@@ -0,0 +1,6 @@
+---
+"@tabler/core": patch
+"@tabler/docs": patch
+---
+
+Fixed `new Tooltip()`, `new Popover()` and `new Dropdown()` on auto-initialised elements ignoring options and doubling listeners.

core/js/src/bootstrap/base-component.ts

@@ -25,10 +25,12 @@ class BaseComponent extends Config {
       return
     }
 
+    const ctor = this.constructor as unknown as BaseComponentStatic
+    Data.get<BaseComponent>(resolved, ctor.DATA_KEY)?.dispose()
+
     this._element = resolved
     this._config = this._getConfig(config)
 
-    const ctor = this.constructor as unknown as BaseComponentStatic
     Data.set(this._element, ctor.DATA_KEY, this)
   }
 
@@ -43,7 +45,15 @@ class BaseComponent extends Config {
   }
 
   _queueCallback(callback: () => void, element: HTMLElement, isAnimated = true): void {
-    executeAfterTransition(callback, element, isAnimated)
+    executeAfterTransition(
+      () => {
+        if (this._element) {
+          callback()
+        }
+      },
+      element,
+      isAnimated,
+    )
   }
 
   _getConfig(config?: ComponentConfig): ComponentConfig {

core/js/tests/unit/alert.spec.ts

@@ -18,9 +18,8 @@ describe('Alert', () => {
 
     const alertEl = fixtureEl.querySelector('.alert')!
     const alertBySelector = new Alert('.alert')
-    const alertByElement = new Alert(alertEl)
-
     expect(alertBySelector._element).toBe(alertEl)
+    const alertByElement = new Alert(alertEl)
     expect(alertByElement._element).toBe(alertEl)
   })
 

core/js/tests/unit/base-component.spec.ts

@@ -79,6 +79,16 @@ describe('BaseComponent', () => {
       const inst = new DummyClass('#nonexistent')
       expect(inst._element).toBeUndefined()
     })
+
+    it('should dispose an existing instance on the same element', () => {
+      createInstance()
+      const spy = vi.spyOn(instance, 'dispose')
+
+      const second = new DummyClass(element)
+
+      expect(spy).toHaveBeenCalledOnce()
+      expect(DummyClass.getInstance(element)).toBe(second)
+    })
   })
 
   describe('dispose', () => {
@@ -162,5 +172,14 @@ describe('BaseComponent', () => {
       element.dispatchEvent(new Event('transitionend'))
       expect(callback).toHaveBeenCalledOnce()
     })
+
+    it('should skip the callback when the instance is disposed before the transition ends', () => {
+      createInstance()
+      const callback = vi.fn()
+      instance._queueCallback(callback, element, true)
+      instance.dispose()
+      element.dispatchEvent(new Event('transitionend'))
+      expect(callback).not.toHaveBeenCalled()
+    })
   })
 })

core/js/tests/unit/button.spec.ts

@@ -17,9 +17,8 @@ describe('Button', () => {
     fixtureEl.innerHTML = '<button data-bs-toggle="button">Placeholder</button>'
     const buttonEl = fixtureEl.querySelector('[data-bs-toggle="button"]')!
     const buttonBySelector = new Button('[data-bs-toggle="button"]')
-    const buttonByElement = new Button(buttonEl)
-
     expect(buttonBySelector._element).toBe(buttonEl)
+    const buttonByElement = new Button(buttonEl)
     expect(buttonByElement._element).toBe(buttonEl)
   })
 

core/js/tests/unit/carousel.spec.ts

@@ -39,9 +39,8 @@ describe('Carousel', () => {
 
       const carouselEl = fixtureEl.querySelector('#myCarousel')!
       const carouselBySelector = new Carousel('#myCarousel')
-      const carouselByElement = new Carousel(carouselEl)
-
       expect(carouselBySelector._element).toBe(carouselEl)
+      const carouselByElement = new Carousel(carouselEl)
       expect(carouselByElement._element).toBe(carouselEl)
     })
 

core/js/tests/unit/collapse.spec.ts

@@ -39,9 +39,8 @@ describe('Collapse', () => {
 
       const collapseEl = fixtureEl.querySelector('div.my-collapse')!
       const collapseBySelector = new Collapse('div.my-collapse')
-      const collapseByElement = new Collapse(collapseEl)
-
       expect(collapseBySelector._element).toBe(collapseEl)
+      const collapseByElement = new Collapse(collapseEl)
       expect(collapseByElement._element).toBe(collapseEl)
     })
 

core/js/tests/unit/dropdown.spec.ts

@@ -52,9 +52,8 @@ describe('Dropdown', () => {
 
       const btnDropdown = fixtureEl.querySelector('[data-bs-toggle="dropdown"]')!
       const dropdownBySelector = new Dropdown('[data-bs-toggle="dropdown"]')
-      const dropdownByElement = new Dropdown(btnDropdown)
-
       expect(dropdownBySelector._element).toBe(btnDropdown)
+      const dropdownByElement = new Dropdown(btnDropdown)
       expect(dropdownByElement._element).toBe(btnDropdown)
     })
 

core/js/tests/unit/scrollspy.spec.ts

@@ -67,9 +67,8 @@ describe('ScrollSpy', () => {
 
       const sSpyEl = fixtureEl.querySelector('.content')!
       const sSpyBySelector = new ScrollSpy('.content')
-      const sSpyByElement = new ScrollSpy(sSpyEl)
-
       expect(sSpyBySelector._element).toBe(sSpyEl)
+      const sSpyByElement = new ScrollSpy(sSpyEl)
       expect(sSpyByElement._element).toBe(sSpyEl)
     })
 

core/js/tests/unit/tab.spec.ts

@@ -26,9 +26,8 @@ describe('Tab', () => {
 
       const tabEl = fixtureEl.querySelector('[href="#home"]')!
       const tabBySelector = new Tab('[href="#home"]')
-      const tabByElement = new Tab(tabEl)
-
       expect(tabBySelector._element).toBe(tabEl)
+      const tabByElement = new Tab(tabEl)
       expect(tabByElement._element).toBe(tabEl)
     })
 

core/js/tests/unit/toast.spec.ts

@@ -31,9 +31,8 @@ describe('Toast', () => {
 
       const toastEl = fixtureEl.querySelector('.toast')!
       const toastBySelector = new Toast('.toast')
-      const toastByElement = new Toast(toastEl)
-
       expect(toastBySelector._element).toBe(toastEl)
+      const toastByElement = new Toast(toastEl)
       expect(toastByElement._element).toBe(toastEl)
     })
 

core/js/tests/unit/tooltip.spec.ts

@@ -62,6 +62,24 @@ describe('Tooltip', () => {
       expect(Tooltip.getInstance(el)).toBe(tooltip)
     })
 
+    it('should replace an existing instance and use the new config', () => {
+      fixtureEl.innerHTML = '<a href="#" title="Tooltip title">Trigger</a>'
+      const el = fixtureEl.querySelector('a')!
+      const first = Tooltip.getOrCreateInstance(el, { placement: 'auto' })
+      const firstEnter = vi.spyOn(first, '_enter')
+
+      const second = new Tooltip(el, { placement: 'bottom', customClass: 'custom' })
+      const secondEnter = vi.spyOn(second, '_enter')
+
+      el.dispatchEvent(new Event('mouseover'))
+
+      expect(Tooltip.getInstance(el)).toBe(second)
+      expect(second._config.customClass).toBe('custom')
+      expect(el.getAttribute('data-bs-original-title')).toBe('Tooltip title')
+      expect(firstEnter).not.toHaveBeenCalled()
+      expect(secondEnter).toHaveBeenCalledOnce()
+    })
+
     it('should fix title on construction', () => {
       fixtureEl.innerHTML = '<a href="#" title="Tooltip title">Trigger</a>'
       const el = fixtureEl.querySelector('a')!

docs/content/ui/components/dropdown.mdx

@@ -293,6 +293,15 @@ Tabler automatically initializes all elements with `data-bs-toggle="dropdown"` o
 
 <CodeDocs name="dropdown-init" file="core/js/src/dropdown.ts" />
 
+To use your own options, create the instance yourself. It replaces the one Tabler created, so only your options apply:
+
+```js
+const el = document.getElementById('my-dropdown');
+new tabler.Dropdown(el, { autoClose: 'outside' });
+```
+
+To read or control the existing instance without changing its options, use `tabler.Dropdown.getInstance(el)` or `tabler.Dropdown.getOrCreateInstance(el)`.
+
 ## Accessibility
 
 - The trigger must be a `button`, or an `a` with an `href`. Bootstrap toggles `aria-expanded` on it for you, so do not set it by hand.

docs/content/ui/components/popover.mdx

@@ -55,6 +55,15 @@ Tabler automatically initializes all elements with `data-bs-toggle="popover"` on
 
 <CodeDocs name="popover-init" file="core/js/src/popover.ts" />
 
+To use your own options, create the instance yourself. It replaces the one Tabler created, so only your options apply:
+
+```js
+const el = document.getElementById('my-popover');
+new tabler.Popover(el, { customClass: 'popover-wide', trigger: 'focus' });
+```
+
+To read or control the existing instance without changing its options, use `tabler.Popover.getInstance(el)` or `tabler.Popover.getOrCreateInstance(el)`.
+
 ## Accessibility
 
 - The trigger must be a `button` or a link with an `href`, so it can take focus. Bootstrap manages `aria-describedby` while the popover is open.

docs/content/ui/components/tooltip.mdx

@@ -45,6 +45,15 @@ Tabler automatically initializes all elements with `data-bs-toggle="tooltip"` on
 
 <CodeDocs name="tooltip-init" file="core/js/src/tooltip.ts" />
 
+To use your own options, create the instance yourself. It replaces the one Tabler created, so only your options apply:
+
+```js
+const el = document.getElementById('my-tooltip');
+new tabler.Tooltip(el, { customClass: 'tooltip-wide', delay: 300 });
+```
+
+To read or control the existing instance without changing its options, use `tabler.Tooltip.getInstance(el)` or `tabler.Tooltip.getOrCreateInstance(el)`.
+
 ## Accessibility
 
 - A tooltip only describes; it never replaces a name. The trigger still needs its own text or `aria-label`.