Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1443)

Unified Diff: chrome/browser/resources/shared/js/cr/ui/menu_button.js

Issue 10966027: Basic keyboard access for recently_closed menu on NTP. Allows using up and down arrows to navigate,… (Closed) Base URL: http://git.chromium.org/chromium/src.git@master
Patch Set: Don't steal keyboard events that the menu doesn't handle, and detect focusin on unrelated elements … Created 8 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/resources/shared/js/cr/ui/menu_button.js
diff --git a/chrome/browser/resources/shared/js/cr/ui/menu_button.js b/chrome/browser/resources/shared/js/cr/ui/menu_button.js
index 3363babb65b5bd3f2ffa490243c1da3f9aed5566..d31fe584f2f4a98da9431f57d97a0fde7a3a3673 100644
--- a/chrome/browser/resources/shared/js/cr/ui/menu_button.js
+++ b/chrome/browser/resources/shared/js/cr/ui/menu_button.js
@@ -93,14 +93,19 @@ cr.define('cr.ui', function() {
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();
+ if (this.menu.handleKeyDown(e)) {
+ e.preventDefault();
+ e.stopPropagation();
+ }
}
break;
+ case 'focusin':
+ if (!this.contains(e.target) && !this.menu.contains(e.target))
+ this.hideMenu();
+ break;
+
case 'activate':
- case 'blur':
case 'resize':
this.hideMenu();
break;
@@ -120,13 +125,14 @@ cr.define('cr.ui', function() {
this.menu.hidden = false;
this.setAttribute('menu-shown', '');
+ this.menu.focusSelectedItem();
// 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(doc, 'focusin', this, true);
this.showingEvents_.add(win, 'resize', this);
this.showingEvents_.add(this.menu, 'activate', this);
this.positionMenu_();
@@ -145,7 +151,7 @@ cr.define('cr.ui', function() {
this.menu.hidden = true;
this.showingEvents_.removeAll();
- this.menu.selectedIndex = -1;
+ this.focus();
},
/**
@@ -180,6 +186,7 @@ cr.define('cr.ui', function() {
break;
case 'Esc':
case 'U+001B': // Maybe this is remote desktop playing a prank?
+ case 'U+0009': // Tab
this.hideMenu();
break;
}
« no previous file with comments | « chrome/browser/resources/shared/js/cr/ui/menu.js ('k') | chrome/browser/resources/shared/js/cr/ui/menu_item.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698