Merge pull request #52 from github/close-popup-on-checked-clicked

Close popup when checked item is clicked.

Author: koddsson

src/index.ts

@@ -170,8 +170,16 @@ function shouldCommit(details: Element, event: Event) {
 
   if (event.type === 'click') {
     const menuitem = target.closest('[role="menuitem"], [role="menuitemradio"]')
-    const onlyCommitOnChangeEvent = menuitem && menuitem.tagName === 'LABEL' && menuitem.querySelector('input')
-    if (menuitem && !onlyCommitOnChangeEvent) {
+    if (!menuitem) return
+
+    // Ignore double event caused by inputs nested in labels
+    if (menuitem.tagName === 'LABEL' && target !== menuitem) return
+
+    const input = menuitem.querySelector('input')
+    // An input inside a label will be committed as a change event (we assume it's a radio input),
+    // unless the input is already checked, so we need to commit on click (to close the popup)
+    const onlyCommitOnChangeEvent = menuitem.tagName === 'LABEL' && input && !input.checked
+    if (!onlyCommitOnChangeEvent) {
       commit(menuitem, details)
     }
   } else if (event.type === 'change') {

test/test.js

@@ -279,6 +279,11 @@ describe('details-menu element', function () {
       assert.equal(item.getAttribute('aria-checked'), 'true')
       assert.equal(details.querySelectorAll('[aria-checked="true"]').length, 1)
       assert.equal(eventCounter, 1, 'selected event is fired once')
+
+      item.dispatchEvent(new MouseEvent('click', {bubbles: true}))
+      assert.equal(item.getAttribute('aria-checked'), 'true')
+      assert.equal(details.querySelectorAll('[aria-checked="true"]').length, 1)
+      assert.equal(eventCounter, 2, 'selected event is fired again')
     })
   })
 
@@ -313,6 +318,11 @@ describe('details-menu element', function () {
       assert.equal(item.getAttribute('aria-checked'), 'true')
       assert.equal(details.querySelectorAll('[aria-checked="true"]').length, 1)
       assert.equal(eventCounter, 1, 'selected event is fired once')
+
+      item.dispatchEvent(new MouseEvent('click', {bubbles: true}))
+      assert.equal(item.getAttribute('aria-checked'), 'true')
+      assert.equal(details.querySelectorAll('[aria-checked="true"]').length, 1)
+      assert.equal(eventCounter, 2, 'selected event is fired again')
     })
   })