dropdown show method should do the same as toggle

twbs · Aug 23, 2019 · 3e820d9 · 3e820d9
1 parent 4d10149
commit 3e820d9
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 23 deletions.
diff --git a/js/src/dropdown/dropdown.js b/js/src/dropdown/dropdown.js
@@ -135,7 +135,6 @@ class Dropdown {
       return
     }
 
-    const parent = Dropdown.getParentFromElement(this._element)
     const isActive = this._menu.classList.contains(ClassName.SHOW)
 
     Dropdown.clearMenus()
@@ -144,9 +143,19 @@ class Dropdown {
       return
     }
 
+    this.show()
+  }
+
+  show() {
+    if (this._element.disabled || this._element.classList.contains(ClassName.DISABLED) || this._menu.classList.contains(ClassName.SHOW)) {
+      return
+    }
+
+    const parent = Dropdown.getParentFromElement(this._element)
     const relatedTarget = {
       relatedTarget: this._element
     }
+
     const showEvent = EventHandler.trigger(parent, Event.SHOW, relatedTarget)
 
     if (showEvent.defaultPrevented) {
@@ -200,27 +209,6 @@ class Dropdown {
     EventHandler.trigger(parent, Event.SHOWN, relatedTarget)
   }
 
-  show() {
-    if (this._element.disabled || this._element.classList.contains(ClassName.DISABLED) || this._menu.classList.contains(ClassName.SHOW)) {
-      return
-    }
-
-    const parent = Dropdown.getParentFromElement(this._element)
-    const relatedTarget = {
-      relatedTarget: this._element
-    }
-
-    const showEvent = EventHandler.trigger(parent, Event.SHOW, relatedTarget)
-
-    if (showEvent.defaultPrevented) {
-      return
-    }
-
-    Manipulator.toggleClass(this._menu, ClassName.SHOW)
-    Manipulator.toggleClass(parent, ClassName.SHOW)
-    EventHandler.trigger(parent, Event.SHOWN, relatedTarget)
-  }
-
   hide() {
     if (this._element.disabled || this._element.classList.contains(ClassName.DISABLED) || !this._menu.classList.contains(ClassName.SHOW)) {
       return
@@ -237,6 +225,10 @@ class Dropdown {
       return
     }
 
+    if (this._popper) {
+      this._popper.destroy()
+    }
+
     Manipulator.toggleClass(this._menu, ClassName.SHOW)
     Manipulator.toggleClass(parent, ClassName.SHOW)
     EventHandler.trigger(parent, Event.HIDDEN, relatedTarget)

diff --git a/js/src/dropdown/dropdown.spec.js b/js/src/dropdown/dropdown.spec.js
@@ -174,7 +174,6 @@ describe('Dropdown', () => {
         expect(firstDropdownEl.classList.contains('show')).toEqual(true)
         spyOn(dropdown1._popper, 'destroy')
         dropdown2.toggle()
-        done()
       })
 
       secondDropdownEl.addEventListener('shown.bs.dropdown', () => {
@@ -679,6 +678,33 @@ describe('Dropdown', () => {
       dropdown.hide()
     })
 
+    it('should hide a dropdown and destroy popper', done => {
+      fixtureEl.innerHTML = [
+        '<div class="dropdown">',
+        '  <button href="#" class="btn dropdown-toggle" data-toggle="dropdown">Dropdown</button>',
+        '  <div class="dropdown-menu">',
+        '    <a class="dropdown-item" href="#">Secondary link</a>',
+        '  </div>',
+        '</div>'
+      ].join('')
+
+      const btnDropdown = fixtureEl.querySelector('[data-toggle="dropdown"]')
+      const dropdownEl = fixtureEl.querySelector('.dropdown')
+      const dropdown = new Dropdown(btnDropdown)
+
+      dropdownEl.addEventListener('shown.bs.dropdown', () => {
+        spyOn(dropdown._popper, 'destroy')
+        dropdown.hide()
+      })
+
+      dropdownEl.addEventListener('hidden.bs.dropdown', () => {
+        expect(dropdown._popper.destroy).toHaveBeenCalled()
+        done()
+      })
+
+      dropdown.show()
+    })
+
     it('should not hide a dropdown if the element is disabled', done => {
       fixtureEl.innerHTML = [
         '<div class="dropdown">',