Basic keyboard access for recently_closed menu on NTP. Allows using up and down arrows to navigate,…

chrome/browser/resources/shared/js/cr/ui/menu_button.js

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 cr.define('cr.ui', function() {
   /** @const */
   var Menu = cr.ui.Menu;
   /** @const */
   var positionPopupAroundElement = cr.ui.positionPopupAroundElement;
 
@@ skipping 82 matching lines @@
           this.handleKeyDown(e);
           // If the menu is visible we let it handle all the keyboard events.
           if (this.isMenuShown() && e.currentTarget == this.ownerDocument) {
             this.menu.handleKeyDown(e);
             e.preventDefault();
             e.stopPropagation();
           }
           break;
 
         case 'activate':
-        case 'blur':
         case 'resize':
           this.hideMenu();
           break;
       }
     },
 
     /**
      * Shows the menu.
      */
     showMenu: function() {
       this.hideMenu();
 
       var event = document.createEvent('UIEvents');
       event.initUIEvent('menushow', true, true, window, null);
 
       if (this.dispatchEvent(event)) {
         this.menu.hidden = false;
 
         this.setAttribute('menu-shown', '');
+        this.menu.selectedIndex = 0;

[Dan Beam, 2012/09/21 03:44:50]

should the first item always be selected when shown?

[aboxhall, 2012/09/21 06:46:00]

The selectedIndex is set back to -1 below, so it seems reasonable to just go to
the first item each time (unless we want the menu to also keep track of the
previously selected item when it's closed?)

[Dan Beam, 2012/09/21 10:23:48]

Where is it set to -1? Either way I guess this method isn't actually toggling
the visibility of the list or anything (like "showMenu" would you'd think), so I
guess this is fine. You'd think this would start unselected, when their
container was focused the first item (I guess) would be selected, and until
something changed the selected or multiselected + lead would be the same, yes.

[aboxhall, 2012/09/24 00:48:52]

It's set to -1 in the hideMenu() method below.

I agree it would be better to go back to the last selected item; I'll do that.

 
         // when the menu is shown we steal all keyboard events.
         var doc = this.ownerDocument;
         var win = doc.defaultView;
         this.showingEvents_.add(doc, 'keydown', this, true);
         this.showingEvents_.add(doc, 'mousedown', this, true);
         this.showingEvents_.add(doc, 'blur', this, true);
         this.showingEvents_.add(win, 'resize', this);
         this.showingEvents_.add(this.menu, 'activate', this);
         this.positionMenu_();
       }
     },
 
     /**
      * Hides the menu. If your menu can go out of scope, make sure to call this
      * first.
      */
     hideMenu: function() {
       if (!this.isMenuShown())
         return;
 
       this.removeAttribute('menu-shown');
       this.menu.hidden = true;
 
       this.showingEvents_.removeAll();
       this.menu.selectedIndex = -1;
+      this.focus();

[Dan Beam, 2012/09/21 03:44:50]

I'm confused by this ^

[aboxhall, 2012/09/21 06:46:00]

It puts the focus back on the menu button when the menu closes, so you can keep
tabbing through the page as before.

[Dan Beam, 2012/09/21 10:23:48]

How does focus()ing a hidden element put the focus back where it was before? I
thought focus()ing a hidden element did nothing in WebKit.

[aboxhall, 2012/09/24 00:48:52]

It's not focusing a hidden element, it's focusing the menu button. |this.menu|
is hidden at this point, but not |this|.

     },
 
     /**
      * Whether the menu is shown.
      */
     isMenuShown: function() {
       return this.hasAttribute('menu-shown');
     },
 
     /**
@@ skipping 13 matching lines @@
       switch (e.keyIdentifier) {
         case 'Down':
         case 'Up':
         case 'Enter':
         case 'U+0020': // Space
           if (!this.isMenuShown())
             this.showMenu();
           e.preventDefault();
           break;
         case 'Esc':
+        case 'U+0009':

[Dan Beam, 2012/09/21 03:44:50]

case 'U+0009': // Whatever the heck this key is.

[Dan Beam, 2012/09/21 10:23:48]

Ping

[aboxhall, 2012/09/24 00:48:52]

Oopsies. It's 'Tab'. Fixed.

         case 'U+001B': // Maybe this is remote desktop playing a prank?
           this.hideMenu();
           break;
       }
     }
   };
 
   /**
    * Helper for styling a menu button with a drop-down arrow indicator.
    * Creates a new 2D canvas context and draws a downward-facing arrow into it.
@@ skipping 38 matching lines @@
         'drop-down-arrow-hover', ARROW_WIDTH, ARROW_HEIGHT, hoverColor);
     createDropDownArrowCanvas(
         'drop-down-arrow-active', ARROW_WIDTH, ARROW_HEIGHT, activeColor);
   };
 
   // Export
   return {
     MenuButton: MenuButton
   };
 });