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

Unified Diff: chrome/browser/resources/settings/settings_menu/settings_menu.js

Issue 2651293003: Make MD Settings side bar keyboard accessible. (Closed)
Patch Set: nits + rebase Created 3 years, 10 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/settings/settings_menu/settings_menu.js
diff --git a/chrome/browser/resources/settings/settings_menu/settings_menu.js b/chrome/browser/resources/settings/settings_menu/settings_menu.js
index 590b33167d96fedbed6013032674a177c7b1d9bc..306f691c8bf585129c31090e4a80aeb44df67728 100644
--- a/chrome/browser/resources/settings/settings_menu/settings_menu.js
+++ b/chrome/browser/resources/settings/settings_menu/settings_menu.js
@@ -9,17 +9,12 @@
Polymer({
is: 'settings-menu',
- behaviors: [settings.RouteObserverBehavior],
-
properties: {
advancedOpened: {
type: Boolean,
notify: true,
},
- /** @private */
- aboutSelected_: Boolean,
-
/**
* Dictionary defining page visibility.
* @type {!GuestModePageVisibility}
@@ -29,52 +24,55 @@ Polymer({
},
},
- ready: function() {
- // When a <paper-submenu> is created with the [opened] attribute as true,
- // its _active member isn't correctly initialized. See this bug for more
- // info: https://github.com/PolymerElements/paper-menu/issues/88. This means
- // the first tap to close an opened Advanced section does nothing (because
- // it calls .open() on an opened menu instead of .close(). This is a fix for
- // that bug without changing that code through its public API.
- //
- // TODO(dbeam): we're currently deciding whether <paper-{,sub}menu> are
- // right for our needs (there have been minor a11y problems). If we decide
- // to keep <paper-{,sub}menu>, fix this bug with a local Chrome CL (ex:
- // https://codereview.chromium.org/2412343004) or a Polymer PR (ex:
- // https://github.com/PolymerElements/paper-menu/pull/107).
- if (this.advancedOpened)
- this.$.advancedSubmenu.open();
+ listeners: {
+ 'topMenu.tap': 'onLinkTap_',
+ 'subMenu.tap': 'onLinkTap_',
+ },
+
+ /** @override */
+ attached: function() {
+ var currentPath = settings.getCurrentRoute().path;
+
+ // Focus the initially selected path.
+ var anchors = this.root.querySelectorAll('a');
+ for (var i = 0; i < anchors.length; ++i) {
+ if (anchors[i].getAttribute('href') == currentPath) {
+ this.setSelectedUrl_(anchors[i].href);
+ break;
+ }
+ }
+ },
+
+ /**
+ * Prevent clicks on sidebar items from navigating. These are only links for
+ * accessibility purposes, taps are handled separately by <iron-selector>.
+ * @param {!Event} event
+ * @private
+ */
+ onLinkTap_: function(event) {
+ if (event.target.hasAttribute('href'))
+ event.preventDefault();
},
/**
- * @param {!settings.Route} newRoute
+ * Keeps both menus in sync. |url| needs to come from |element.href| because
+ * |iron-list| uses the entire url. Using |getAttribute| will not work.
+ * @param {string} url
*/
- currentRouteChanged: function(newRoute) {
- // Make the three menus mutually exclusive.
- if (settings.Route.ABOUT.contains(newRoute)) {
- this.aboutSelected_ = true;
- this.$.advancedMenu.selected = null;
- this.$.basicMenu.selected = null;
- } else if (settings.Route.ADVANCED.contains(newRoute)) {
- this.aboutSelected_ = false;
- // For routes from URL entry, we need to set selected.
- this.$.advancedMenu.selected = newRoute.path;
- this.$.basicMenu.selected = null;
- } else if (settings.Route.BASIC.contains(newRoute)) {
- this.aboutSelected_ = false;
- this.$.advancedMenu.selected = null;
- // For routes from URL entry, we need to set selected.
- this.$.basicMenu.selected = newRoute.path;
- }
+ setSelectedUrl_: function(url) {
+ this.$.topMenu.selected = this.$.subMenu.selected = url;
},
/**
* @param {!Event} event
* @private
*/
- openPage_: function(event) {
- var route = settings.getRouteForPath(event.currentTarget.dataset.path);
- assert(route, 'settings-menu has an an entry with an invalid path');
+ onSelectorActivate_: function(event) {
+ this.setSelectedUrl_(event.detail.selected);
+
+ var path = new URL(event.detail.selected).pathname;
+ var route = settings.getRouteForPath(path);
+ assert(route, 'settings-menu has an entry with an invalid route.');
settings.navigateTo(
route, /* dynamicParams */ null, /* removeSearch */ true);
},

Powered by Google App Engine
This is Rietveld 408576698