Merge pull request #36 from github/keyboard

muan · web-flow · commit 0651aa2dcb69 · 2019-09-10T15:29:59.000-04:00
Fix focus and role problems
diff --git a/examples/index.html b/examples/index.html
@@ -3,49 +3,60 @@
 <head>
   <meta charset="utf-8">
   <title>details-menu demo</title>
+  <style>
+    details-menu {
+      background: white;
+      border: 1px solid;
+      display: block;
+      padding: 4px;
+      width: 200px;
+    }
+    button, label[tabindex] {
+      font-family: inherit;
+      font-size: inherit;
+      display: block;
+      background: none;
+      border: 0;
+      width: 100%;
+      text-align: left;
+      padding: 0;
+    }
+  </style>
 </head>
 <body>
   <details>
     <summary>Best robot: <span data-menu-button>Unknown</span></summary>
     <details-menu>
-      <ul>
-        <li><button type="button" role="menuitem" data-menu-button-text>Hubot</button></li>
-        <li><button type="button" role="menuitem" data-menu-button-text>Bender</button></li>
-        <li><button type="button" role="menuitem" data-menu-button-text>BB-8</button></li>
-      </ul>
+      <button type="button" role="menuitem" data-menu-button-text>Hubot</button>
+      <button type="button" role="menuitem" data-menu-button-text>Bender</button>
+      <button type="button" role="menuitem" data-menu-button-text>BB-8</button>
     </details-menu>
   </details>
 
   <details>
     <summary>Best robot: <span data-menu-button>Unknown</span></summary>
     <details-menu>
-      <ul>
-        <li><label tabindex="0" role="menuitemradio" data-menu-button-text><input type="radio" name="robot"> Hubot</label></li>
-        <li><label tabindex="0" role="menuitemradio" data-menu-button-text><input type="radio" name="robot"> Bender</label></li>
-        <li><label tabindex="0" role="menuitemradio" data-menu-button-text><input type="radio" name="robot"> BB-8</label></li>
-      </ul>
+      <label tabindex="0" role="menuitemradio" data-menu-button-text><input type="radio" name="robot"> Hubot</label>
+      <label tabindex="0" role="menuitemradio" data-menu-button-text><input type="radio" name="robot"> Bender</label>
+      <label tabindex="0" role="menuitemradio" data-menu-button-text><input type="radio" name="robot"> BB-8</label>
     </details-menu>
   </details>
 
   <details>
     <summary>Favorite robots</summary>
     <details-menu>
-      <ul>
-        <li><label tabindex="0" role="menuitemcheckbox"><input type="checkbox" name="robot"> Hubot</label></li>
-        <li><label tabindex="0" role="menuitemcheckbox"><input type="checkbox" name="robot"> Bender</label></li>
-        <li><label tabindex="0" role="menuitemcheckbox"><input type="checkbox" name="robot"> BB-8</label></li>
-      </ul>
+      <label tabindex="0" role="menuitemcheckbox"><input type="checkbox" name="robot"> Hubot</label>
+      <label tabindex="0" role="menuitemcheckbox"><input type="checkbox" name="robot"> Bender</label>
+      <label tabindex="0" role="menuitemcheckbox"><input type="checkbox" name="robot"> BB-8</label>
     </details-menu>
   </details>
 
   <details>
     <summary data-menu-button>Favorite robots</summary>
     <details-menu>
-      <ul>
-        <li><button type="submit" name="robot" value="Hubot" role="menuitemradio" data-menu-button-text>Hubot</button></li>
-        <li><button type="submit" name="robot" value="Bender" role="menuitemradio" data-menu-button-text>Bender</button></li>
-        <li><button type="submit" name="robot" value="BB-8" role="menuitemradio" data-menu-button-text>BB-8</button></li>
-      </ul>
+      <button type="submit" name="robot" value="Hubot" role="menuitemradio" data-menu-button-text>Hubot</button>
+      <button type="submit" name="robot" value="Bender" role="menuitemradio" data-menu-button-text>Bender</button>
+      <button type="submit" name="robot" value="BB-8" role="menuitemradio" data-menu-button-text>BB-8</button>
     </details-menu>
   </details>
 
diff --git a/index.js b/index.js
@@ -26,13 +26,16 @@ class DetailsMenuElement extends HTMLElement {
   }
 
   connectedCallback() {
-    this.setAttribute('role', 'menu')
+    if (!this.hasAttribute('role')) this.setAttribute('role', 'menu')
 
     const details = this.parentElement
     if (!details) return
 
     const summary = details.querySelector('summary')
-    if (summary) summary.setAttribute('aria-haspopup', 'menu')
+    if (summary) {
+      summary.setAttribute('aria-haspopup', 'menu')
+      if (!summary.hasAttribute('role')) summary.setAttribute('role', 'button')
+    }
 
     details.addEventListener('click', shouldCommit)
     details.addEventListener('change', shouldCommit)
@@ -96,10 +99,9 @@ function focusOnOpen(details: Element) {
   const onmousedown = () => (isMouse = true)
   const onkeydown = () => (isMouse = false)
   const ontoggle = () => {
-    autofocus(details)
-    if (details.hasAttribute('open') && !isMouse) {
-      focusFirstItem(details)
-    }
+    if (!details.hasAttribute('open')) return
+    if (autofocus(details)) return
+    if (!isMouse) focusFirstItem(details)
   }
 
   details.addEventListener('mousedown', onmousedown)
@@ -128,12 +130,14 @@ function closeCurrentMenu(event: Event) {
   }
 }
 
-function autofocus(details: Element) {
-  if (!details.hasAttribute('open')) return
-
+function autofocus(details: Element): boolean {
+  if (!details.hasAttribute('open')) return false
   const input = details.querySelector('[autofocus]')
   if (input) {
     input.focus()
+    return true
+  } else {
+    return false
   }
 }
 
diff --git a/test/test.js b/test/test.js
@@ -34,6 +34,14 @@ describe('details-menu element', function() {
       document.body.innerHTML = ''
     })
 
+    it('has default attributes set', function() {
+      const details = document.querySelector('details')
+      const summary = details.querySelector('summary')
+      const menu = details.querySelector('details-menu')
+      assert.equal(summary.getAttribute('role'), 'button')
+      assert.equal(menu.getAttribute('role'), 'menu')
+    })
+
     it('opens and does not focus an item on mouse click', function() {
       const details = document.querySelector('details')
       const summary = details.querySelector('summary')
@@ -546,6 +554,55 @@ describe('details-menu element', function() {
     })
   })
 
+  describe('with input[autofocus]', function() {
+    beforeEach(function() {
+      const container = document.createElement('div')
+      container.innerHTML = `
+        <details>
+          <summary>Menu 1</summary>
+          <details-menu role="none">
+            <input autofocus>
+            <div role="menu">
+              <button role="menuitem">First item</button>
+            </div>
+          </details-menu>
+        </details>
+      `
+      document.body.append(container)
+    })
+
+    afterEach(function() {
+      document.body.innerHTML = ''
+    })
+
+    it('autofocuses on input on mouse click', function() {
+      const details = document.querySelector('details')
+      const summary = details.querySelector('summary')
+      const menu = details.querySelector('details-menu')
+      const input = details.querySelector('input')
+
+      summary.focus()
+      details.open = true
+      summary.dispatchEvent(new MouseEvent('mousedown', {bubbles: true}))
+      details.dispatchEvent(new CustomEvent('toggle'))
+      assert.equal(menu.getAttribute('role'), 'none')
+      assert.equal(input, document.activeElement, 'mouse toggle open leaves summary focused')
+    })
+
+    it('autofocuses on input on keyboard activation', function() {
+      const details = document.querySelector('details')
+      const summary = details.querySelector('summary')
+      const input = details.querySelector('input')
+
+      summary.focus()
+      details.open = true
+      summary.dispatchEvent(new KeyboardEvent('keydown', {key: 'Enter', bubbles: true}))
+      details.dispatchEvent(new CustomEvent('toggle'))
+
+      assert.equal(input, document.activeElement, 'toggle open focuses on [autofocus]')
+    })
+  })
+
   describe('closing the menu', function() {
     beforeEach(function() {
       const container = document.createElement('div')
