Make MD Settings side bar keyboard accessible.

chrome/browser/resources/settings/settings_menu/settings_menu.html

Index: chrome/browser/resources/settings/settings_menu/settings_menu.html
diff --git a/chrome/browser/resources/settings/settings_menu/settings_menu.html b/chrome/browser/resources/settings/settings_menu/settings_menu.html
index d9458b9450d2a9ecf30b637b80e9ed532afabe5c..5df39c8ad2b7709e13ccaeb7a0c8a608f3386279 100644
--- a/chrome/browser/resources/settings/settings_menu/settings_menu.html
+++ b/chrome/browser/resources/settings/settings_menu/settings_menu.html
@@ -1,4 +1,5 @@
 <link rel="import" href="chrome://resources/cr_elements/icons.html">
+<link rel="import" href="chrome://resources/html/action_link.html">
 <link rel="import" href="chrome://resources/html/assert.html">
 <link rel="import" href="chrome://resources/html/polymer.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/iron-icon/iron-icon.html">
@@ -19,48 +20,61 @@
       iron-icon {
         --iron-icon-fill-color: var(--settings-nav-grey);
         -webkit-margin-end: 24px;
+        padding-top: 10px;  /* Center the 20px tall icon in a 40px space. */
       }
 
-      .iron-selected:not(.menu-trigger) > iron-icon {
-        fill: var(--google-blue-700);
+      .menu-trigger > iron-icon {
+        -webkit-margin-end: 22px;  /* 24px - 2px from margin for outline. */
       }
 
-      .menu-trigger span {
+      a[is='action-link'] span {
         -webkit-margin-end: 16px;
         flex: 1;
       }
 
+      .iron-selected:not(.menu-trigger) > iron-icon {
+        fill: var(--google-blue-700);
+      }
+
       paper-menu {
-        --paper-menu-selected-item: {
-          font-weight: 500;
+        padding: 0;
+      }
+
+      a[is='action-link'].no-outline {
+        --paper-menu-focused-item: {
+          outline: none;
+        };
+      }
+
+      a[is='action-link'] {
+        @apply(--settings-actionable);
+        --paper-menu-focused-item: {
+          outline: auto 5px -webkit-focus-ring-color;
         };
         --paper-menu-focused-item-after: {
           background: none;
         };
+        --paper-menu-selected-item: {
+          color: var(--settings-nav-grey);
+          font-weight: 500;
+        };
+        -webkit-margin-end: 2px;  /* Margin so selected outline is visible. */
+        -webkit-margin-start: 1px;
         color: var(--settings-nav-grey);
-        font-size: 100%;
+        display: flex;
         padding: 0;
       }
 
-      div[data-path] {
-        @apply(--settings-actionable);
-      }
-
       #basicPage {
         margin-top: 8px;
       }
 
-      .page-menu > div {
+      .page-menu > a[is='action-link'] {
+        -webkit-padding-end: 0;
         margin-top: 8px;
       }
 
-      .page-menu {
-        --paper-menu-selected-item: {
-          font-weight: 500;
-        };
-      }
-
-      paper-menu div {
+      .menu-trigger {
         position: relative;  /* Needed for paper-ripple. */
       }
 
@@ -76,144 +90,137 @@
         };
       }
 
-      .page-menu div {
-        -webkit-padding-start: 24px;
-        align-items: center;
-        display: flex;
+      .page-menu a[is='action-link'] {
+        -webkit-padding-start: 23px;  /* 24px - 1px from margin for outline. */
         font-size: 100%;
         font-weight: 500;
-        min-height: 40px;
+        line-height: 40px;
+        text-transform: none;
         word-break: break-word;
       }
 
-      .page-menu div iron-icon {
-        flex-shrink: 0;
-      }
-
       .separator {
         /* Per Alan@, this line is different from the other separator lines. */
         border-bottom: 1px solid rgba(0, 0, 0, 0.08);
         margin-top: 8px;
       }
     </style>
-    <paper-menu name="root-menu">
-      <div class="page-menu" id="basicPage">
-        <paper-menu attr-for-selected="data-path" class="menu-content"
-            id="basicMenu" selected="[[currentRoute.path]]">
+    <div class="page-menu" id="basicPage">
+      <paper-menu attr-for-selected="data-path" class="menu-content"
+          id="basicMenu" selected="[[currentRoute.path]]">
 <if expr="chromeos">
-          <div data-path="/internet" on-tap="openPage_">
-            <iron-icon icon="settings:network-wifi"></iron-icon>
-            $i18n{internetPageTitle}
-          </div>
+        <a is="action-link" data-path="/internet" on-tap="openPage_">

[Dan Beam, 2017/02/01 00:47:50]

if you do something like:

<button is="paper-icon-button-light" data-path="/internet" on-tap="openPage_">
  <iron-icon icon="settings:network-wifi"></iron-icon>
  <span>$i18n{internetPageTitle}</span>
</button>

(which is dynamically adding ripples and pretty light) what's the effect on load
time?

[hcarmona, 2017/02/01 18:50:05]

IIUC paper-icon-button-light is just for an icon, no text.

When Dave and I spoke he mentioned that history's side bar was always visible
but since settings side bar slides away we only needed a ripple on the advanced
button. However, now I see that history's side bar also slides away if your
window is small enough and has ripples.

Adding ripples to all these action-links increases stamp time to ~20ms

+          <iron-icon icon="settings:network-wifi"></iron-icon>
+          <span>$i18n{internetPageTitle}</span>
+        </a>
 </if>
-          <div id="people" data-path="/people" on-tap="openPage_"
-              hidden="[[!pageVisibility.people]]">
-            <iron-icon icon="settings:people"></iron-icon>
-            $i18n{peoplePageTitle}
-          </div>
-          <div data-path="/appearance" on-tap="openPage_"
-              hidden="[[!pageVisibility.appearance]]">
-            <iron-icon icon="settings:palette"></iron-icon>
-            $i18n{appearancePageTitle}
-          </div>
+        <a is="action-link" id="people" data-path="/people" on-tap="openPage_"
+            hidden="[[!pageVisibility.people]]">
+          <iron-icon icon="settings:people"></iron-icon>
+          <span>$i18n{peoplePageTitle}</span>
+        </a>
+        <a is="action-link" data-path="/appearance" on-tap="openPage_"
+            hidden="[[!pageVisibility.appearance]]">
+          <iron-icon icon="settings:palette"></iron-icon>
+          <span>$i18n{appearancePageTitle}</span>
+        </a>
 <if expr="chromeos">
-          <div data-path="/device" on-tap="openPage_">
-            <iron-icon icon="settings:laptop-chromebook"></iron-icon>
-            $i18n{devicePageTitle}
-          </div>
+        <a is="action-link" data-path="/device" on-tap="openPage_">
+          <iron-icon icon="settings:laptop-chromebook"></iron-icon>
+          <span>$i18n{devicePageTitle}</span>
+        </a>
 </if>
-          <div data-path="/search" on-tap="openPage_">
-            <iron-icon icon="cr:search"></iron-icon>
-            $i18n{searchPageTitle}
-          </div>
+        <a is="action-link" data-path="/search" on-tap="openPage_">
+          <iron-icon icon="cr:search"></iron-icon>
+          <span>$i18n{searchPageTitle}</span>
+        </a>
 <if expr="chromeos">
-           <div data-path="/androidApps" on-tap="openPage_"
-               hidden="[[!showAndroidApps]]">
-             <iron-icon icon="settings:android"></iron-icon>
-             $i18n{androidAppsPageTitle}
-           </div>
+         <a is="action-link" data-path="/androidApps" on-tap="openPage_"
+             hidden="[[!showAndroidApps]]">
+           <iron-icon icon="settings:android"></iron-icon>
+           <span>$i18n{androidAppsPageTitle}</span>
+         </a>
 </if>
 <if expr="not chromeos">
-          <div data-path="/defaultBrowser" on-tap="openPage_"
-              hidden="[[!pageVisibility.defaultBrowser]]">
-            <iron-icon icon="settings:web"></iron-icon>
-            $i18n{defaultBrowser}
-          </div>
+        <a is="action-link" data-path="/defaultBrowser" on-tap="openPage_"
+            hidden="[[!pageVisibility.defaultBrowser]]">
+          <iron-icon icon="settings:web"></iron-icon>
+          <span>$i18n{defaultBrowser}</span>
+        </a>
 </if>
-          <div data-path="/onStartup" on-tap="openPage_"
-              hidden="[[!pageVisibility.onStartup]]">
-            <iron-icon icon="settings:power-settings-new"></iron-icon>
-            $i18n{onStartup}
-          </div>
-        </paper-menu>
-      </div>
+        <a is="action-link" data-path="/onStartup" on-tap="openPage_"
+            hidden="[[!pageVisibility.onStartup]]">
+          <iron-icon icon="settings:power-settings-new"></iron-icon>
+          <span>$i18n{onStartup}</span>
+        </a>
+      </paper-menu>
       <paper-submenu class="page-menu" id="advancedSubmenu" actionable
           opened="{{advancedOpened}}"
           hidden="[[!pageVisibility.advancedSettings]]">
-        <div class="menu-trigger">
+        <a is="action-link" class="menu-trigger" tabindex="0">
           <span>$i18n{advancedPageTitle}</span>
           <iron-icon icon="[[arrowState_(advancedOpened)]]"></iron-icon>
           <paper-ripple></paper-ripple>
-        </div>
+        </a>
         <paper-menu attr-for-selected="data-path" class="menu-content"
             id="advancedMenu" selected="[[currentRoute.path]]">
 <if expr="chromeos">
-          <div data-path="/dateTime" on-tap="openPage_">
+          <a is="action-link" data-path="/dateTime" on-tap="openPage_">
             <iron-icon icon="settings:access-time"></iron-icon>
-            $i18n{dateTimePageTitle}
-          </div>
+            <span>$i18n{dateTimePageTitle}</span>
+          </a>
 </if>
-          <div data-path="/privacy" on-tap="openPage_">
+          <a is="action-link" data-path="/privacy" on-tap="openPage_">
             <iron-icon icon="settings:security"></iron-icon>
-            $i18n{privacyPageTitle}
-          </div>
+            <span>$i18n{privacyPageTitle}</span>
+          </a>
 <if expr="chromeos">
-          <div data-path="/bluetooth" on-tap="openPage_">
+          <a is="action-link" data-path="/bluetooth" on-tap="openPage_">
             <iron-icon icon="settings:bluetooth"></iron-icon>
-            $i18n{bluetoothPageTitle}
-          </div>
+            <span>$i18n{bluetoothPageTitle}</span>
+          </a>
 </if>
-          <div data-path="/passwordsAndForms" on-tap="openPage_"
+          <a is="action-link" data-path="/passwordsAndForms" on-tap="openPage_"
               hidden="[[!pageVisibility.passwordsAndForms]]">
             <iron-icon icon="settings:assignment"></iron-icon>
-            $i18n{passwordsAndAutofillPageTitle}
-          </div>
-          <div data-path="/languages" on-tap="openPage_">
+            <span>$i18n{passwordsAndAutofillPageTitle}</span>
+          </a>
+          <a is="action-link" data-path="/languages" on-tap="openPage_">
             <iron-icon icon="settings:language"></iron-icon>
-            $i18n{languagesPageTitle}
-          </div>
-          <div data-path="/downloads" on-tap="openPage_">
+            <span>$i18n{languagesPageTitle}</span>
+          </a>
+          <a is="action-link" data-path="/downloads" on-tap="openPage_">
             <iron-icon icon="cr:file-download"></iron-icon>
-            $i18n{downloadsPageTitle}
-          </div>
-          <div data-path="/printing" on-tap="openPage_">
+            <span>$i18n{downloadsPageTitle}</span>
+          </a>
+          <a is="action-link" data-path="/printing" on-tap="openPage_">
             <iron-icon icon="cr:print"></iron-icon>
-            $i18n{printingPageTitle}
-          </div>
-          <div data-path="/accessibility" on-tap="openPage_">
+            <span>$i18n{printingPageTitle}</span>
+          </a>
+          <a is="action-link" data-path="/accessibility" on-tap="openPage_">
             <iron-icon icon="settings:accessibility"></iron-icon>
-            $i18n{a11yPageTitle}
-          </div>
+            <span>$i18n{a11yPageTitle}</span>
+          </a>
 <if expr="not chromeos">
-          <div data-path="/system" on-tap="openPage_">
+          <a is="action-link" data-path="/system" on-tap="openPage_">
             <iron-icon icon="settings:build"></iron-icon>
-            $i18n{systemPageTitle}
-          </div>
+            <span>$i18n{systemPageTitle}</span>
+          </a>
 </if>
-          <div data-path="/reset" on-tap="openPage_"
+          <a is="action-link" data-path="/reset" on-tap="openPage_"
               hidden="[[!pageVisibility.reset]]">
             <iron-icon icon="settings:restore"></iron-icon>
-            $i18n{resetPageTitle}
-          </div>
+            <span>$i18n{resetPageTitle}</span>
+          </a>
         </paper-menu>
       </paper-submenu>
-      <div class="separator"></div>
-      <paper-submenu class="page-menu">
-        <div about-selected$="[[aboutSelected_]]" class="menu-trigger"
-            id="about-menu" on-tap="openPage_" data-path="/help">
-          $i18n{aboutPageTitle}
-        </div>
-      </paper-submenu>
+    </div>
+    <div class="separator"></div>
+    <paper-menu class="page-menu">
+      <a is="action-link" about-selected$="[[aboutSelected_]]"
+          id="about-menu" on-tap="openPage_" data-path="/help">
+        <span>$i18n{aboutPageTitle}</span>
+      </a>
     </paper-menu>
   </template>
   <script src="settings_menu.js"></script>