Editor Menubar Example: set aria-checked correctly, close with enter,…

… remove redundant code (pull #525) Change the editor menubar example to resolve issues #521 and #524: * ensure that aria-checked is only updated on font-size elements * apply menuitem roles to the triggering element and match menubar-1 semantics * update styles to not rely on span selector * always close the menu on click (keyboard or otherwise) * don't close menu on space
w3c · Nov 17, 2017 · ba84ee6 · ba84ee6
1 parent ae114de
commit ba84ee6
Show file tree

Hide file tree

Showing 6 changed files with 34 additions and 34 deletions.
diff --git a/examples/menubar/menubar-2/css/menubarAction.css b/examples/menubar/menubar-2/css/menubarAction.css
@@ -19,7 +19,7 @@ ul[role="menubar"] > li {
   position: relative;
 }
 
-ul[role="menubar"] > li > span:after {
+ul[role="menubar"] > li > [role="menuitem"]:after {
   content: url('../images/down-arrow-brown.png');
   padding-left: 0.25em;
 }
@@ -35,16 +35,16 @@ ul[role="menubar"] ul[role="menu"] {
 ul[role="menubar"] ul[role="group"] {
   margin: 0;
   padding: 0;
-}  
+}
 
 ul[role="menubar"] li[aria-disabled="true"] {
   color: #666666;
   text-decoration: line-through;
-}  
+}
 
 ul[role="menubar"] [role="menuitem"],
 ul[role="menubar"] [role="menuitemcheckbox"],
-ul[role="menubar"] [role="menuitemradio"], 
+ul[role="menubar"] [role="menuitemradio"],
 ul[role="menubar"] [role="separator"] {
   padding: 0.25em;
   background-color: #EEEEEE;
@@ -54,11 +54,11 @@ ul[role="menubar"] [role="separator"] {
 
 ul[role="menubar"] ul li[role="menuitem"],
 ul[role="menubar"] ul li[role="menuitemcheckbox"],
-ul[role="menubar"] ul li[role="menuitemradio"], 
+ul[role="menubar"] ul li[role="menuitemradio"],
 ul[role="menubar"] ul li[role="separator"] {
   padding-left: 1.5em;
   width: 8em;
-}  
+}
 
 
 ul[role="menubar"] [role="separator"] {
@@ -69,7 +69,7 @@ ul[role="menubar"] [role="separator"] {
 }
 
 ul[role="menubar"] [role="menuitem"]:focus,
-ul[role="menubar"] [role="menuitem"] span:hover,
+ul[role="menubar"] [role="menuitem"] [role="menuitem"]:hover,
 ul[role="menubar"] [role="menu"] [role="menuitem"]:hover,
 ul[role="menubar"] [role="menu"] [role="menuitemcheckbox"]:focus,
 ul[role="menubar"] [role="menu"] [role="menuitemcheckbox"]:hover,

diff --git a/examples/menubar/menubar-2/js/MenubarAction.js b/examples/menubar/menubar-2/js/MenubarAction.js
@@ -31,12 +31,12 @@ var MenubarAction = function (domNode) {
   if (domNode.childElementCount === 0) {
     throw new Error(msgPrefix + 'has no element children.');
   }
-  // Check whether menubarNode has SPAN elements
+  // Check whether menubarNode has A elements
   e = domNode.firstElementChild;
   while (e) {
     var menubarItem = e.firstElementChild;
-    if (e && menubarItem && menubarItem.tagName !== 'SPAN') {
-      throw new Error(msgPrefix + 'has child elements are not SPAN elements.');
+    if (e && menubarItem && menubarItem.tagName !== 'A') {
+      throw new Error(msgPrefix + 'has child elements are not A elements.');
     }
     e = e.nextElementSibling;
   }
@@ -75,7 +75,7 @@ MenubarAction.prototype.init = function (actionManager) {
   while (e) {
     var menuElement = e.firstElementChild;
 
-    if (e && menuElement && menuElement.tagName === 'SPAN') {
+    if (e && menuElement && menuElement.tagName === 'A') {
       menubarItem = new MenubarItemAction(menuElement, this);
       menubarItem.init();
       this.menubarItems.push(menubarItem);
@@ -186,4 +186,3 @@ MenubarAction.prototype.getIndexFirstChars = function (startIndex, char) {
   }
   return -1;
 };
-
diff --git a/examples/menubar/menubar-2/js/MenubarItemAction.js b/examples/menubar/menubar-2/js/MenubarItemAction.js
@@ -59,11 +59,6 @@ var MenubarItemAction = function (domNode, menuObj) {
 MenubarItemAction.prototype.init = function () {
   this.domNode.tabIndex = -1;
 
-  this.domNode.setAttribute('role', 'menuitem');
-  this.domNode.setAttribute('aria-haspopup', 'true');
-  this.domNode.setAttribute('aria-expanded', 'false');
-  this.domNode.tabIndex = -1;
-
   this.domNode.addEventListener('keydown', this.handleKeydown.bind(this));
   this.domNode.addEventListener('click', this.handleClick.bind(this));
   this.domNode.addEventListener('focus', this.handleFocus.bind(this));

diff --git a/examples/menubar/menubar-2/js/PopupMenuAction.js b/examples/menubar/menubar-2/js/PopupMenuAction.js
@@ -143,7 +143,7 @@ PopupMenuAction.prototype.updateMenuStates = function () {
   // Update the radio buttons for font, in case they were updated using the larger 
   // smaller font menu items
 
-  var rbs = this.domNode.querySelectorAll('[role=menuitemradio]');
+  var rbs = this.domNode.querySelectorAll('[rel="font-size"] [role=menuitemradio]');
 
   for (var i = 0; i < rbs.length; i++) {
     var rb = rbs[i];

diff --git a/examples/menubar/menubar-2/js/PopupMenuItemAction.js b/examples/menubar/menubar-2/js/PopupMenuItemAction.js
@@ -125,14 +125,20 @@ MenuItem.prototype.handleKeydown = function (event) {
 
   switch (event.keyCode) {
     case this.keyCode.SPACE:
-    case this.keyCode.RETURN:
       if (this.activateMenuitem(tgt)) {
         this.menu.setFocusToController();
         this.menu.close(true);
       }
       flag = true;
       break;
 
+    case this.keyCode.RETURN:
+      this.activateMenuitem(tgt);
+      this.menu.setFocusToController();
+      this.menu.close(true);
+      flag = true;
+      break;
+
     case this.keyCode.ESC:
       this.menu.setFocusToController();
       this.menu.close(true);

diff --git a/examples/menubar/menubar-2/menubar-2.html b/examples/menubar/menubar-2/menubar-2.html
@@ -44,17 +44,17 @@ <h2 id="ex1_label">Example</h2>
     <div role="separator" id="ex1_start_sep" aria-labelledby="ex1_start_sep ex1_label" aria-label="Start of"></div>
     <div id="ex1">
       <ul id="menubar1" role="menubar" aria-label="Text Formatting">
-        <li role="menuitem" aria-haspopup="true" aria-expanded="false">
-          <span>Font</span>
+        <li role="none">
+          <a role="menuitem" aria-haspopup="true" aria-expanded="false" href="#">Font</a>
           <ul role="menu" rel="font-family" aria-label="Font" >
             <li role="menuitemradio" aria-checked="true">Sans-serif</li>
             <li role="menuitemradio" aria-checked="false">Serif</li>
             <li role="menuitemradio" aria-checked="false">Monospace</li>
             <li role="menuitemradio" aria-checked="false">Fantasy</li>
           </ul>
         </li>
-        <li role="menuitem" aria-haspopup="true" aria-expanded="false">
-          <span>Style/Color</span>
+        <li role="none">
+          <a role="menuitem" aria-haspopup="true" aria-expanded="false" href="#">Style/Color</a>
           <ul role="menu" aria-label="Style/Color">
             <li role="menuitemcheckbox" rel="font-bold" aria-checked="false">Bold</li>
             <li role="menuitemcheckbox" rel="font-italic" aria-checked="false">Italic</li>
@@ -77,8 +77,8 @@ <h2 id="ex1_label">Example</h2>
           </ul>
         </li>
 
-        <li role="menuitem" aria-haspopup="true" aria-expanded="false">
-          <span>Text Align</span>
+        <li role="none">
+          <a role="menuitem" aria-haspopup="true" aria-expanded="false" href="#">Text Align</a>
           <ul role="menu" rel="text-align" aria-label="Text Align">
             <li role="menuitemradio" aria-checked="true">Left</li>
             <li role="menuitemradio" aria-checked="false">Center</li>
@@ -87,8 +87,8 @@ <h2 id="ex1_label">Example</h2>
           </ul>
         </li>
 
-        <li role="menuitem" aria-haspopup="true" aria-expanded="false">
-          <span>Size</span>
+        <li role="none">
+          <a role="menuitem" aria-haspopup="true" aria-expanded="false" href="#">Size</a>
           <ul role="menu" aria-label="Size" >
             <li role="menuitem" rel="font-smaller" aria-disabled="false">Smaller</li>
             <li role="menuitem" rel="font-larger"  aria-disabled="false">Larger</li>
@@ -133,7 +133,7 @@ <h2>Accessibility Features</h2>
       <li>The down arrow and checked icons are made compatible with high contrast mode and hidden from screen readers by using the CSS <code>content</code> property to render images.</li>
     </ol>
   </section>
-  
+
   <section>
     <h2 id="kbd_label">Keyboard Support</h2>
     <h3 id="kbd1_label">Menubar</h3>
@@ -369,7 +369,7 @@ <h3 id="rps1_label">Menubar</h3>
               <ul>
                 <li>Identifies the element as a menu item within the menubar.</li>
                 <li>Accessible name comes from the text content.</li>
-              </ul>  
+              </ul>
             </td>
           </tr>
           <tr>
@@ -554,7 +554,7 @@ <h3 id="rps2_label">Submenu</h3>
             <ul>
               <li>Identifies the element as a <code>menuitemcheckbox</code>.</li>
               <li>Accessible name comes from the text content.</li>
-            </ul>  
+            </ul>
           </td>
         </tr>
         <tr>
@@ -705,9 +705,9 @@ <h3>Note</h3>
     <p>
       Currently, using aria-expanded on elements with role menuitem triggers HTML validation errors because the ARIA specification does not yet support doing so.
       The ARIA working group plans to resolve this issue in the next version of the specification.
-      Until a version of ARIA that resolves 
-      <a href="https://github.com/w3c/aria/issues/454">the issue</a> 
-      becomes a W3C recommendation, it is safe to ignore these validation errors. 
+      Until a version of ARIA that resolves
+      <a href="https://github.com/w3c/aria/issues/454">the issue</a>
+      becomes a W3C recommendation, it is safe to ignore these validation errors.
       Alternatively, since only a few browser and assistive technology combinations exploit this feature of the pattern, it can be omitted from implementations.
     </p>
   </section>
@@ -732,7 +732,7 @@ <h2>Javascript and CSS Source Code</h2>
         Javascript: <a href="js/PopupMenuItemAction.js" type="text/javascript">PopupMenuItemAction.js</a>
       </li>
       <li>
-            Javascript: <a href="js/styleManager.js" type="text/javascript">styleManager.js</a>
+        Javascript: <a href="js/styleManager.js" type="text/javascript">styleManager.js</a>
       </li>
     </ul>
   </section>