Make MD Settings side bar keyboard accessible.

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

 <link rel="import" href="chrome://resources/cr_elements/icons.html">
+<link rel="import" href="chrome://resources/html/action_link.html">

[Dan Beam, 2017/02/06 17:28:12]

you haven't imported action_link_css.html

but we probably just switch to native <a href>

[hcarmona, 2017/02/10 19:27:34]

Done.

 <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">
 <link rel="import" href="chrome://resources/polymer/v1_0/paper-menu/paper-menu.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/paper-menu/paper-submenu.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/paper-ripple/paper-ripple.html">
 <link rel="import" href="../icons.html">
 <link rel="import" href="../route.html">
 <link rel="import" href="../settings_shared_css.html">
 
 <dom-module id="settings-menu">
   <template>
     <style include="settings-shared">
       #about-menu[about-selected] {
         color: var(--google-blue-700);
       }
 
       iron-icon {
         --iron-icon-fill-color: var(--settings-nav-grey);
         -webkit-margin-end: 24px;
+        padding-top: 10px;  /* Center the 20px icon in a 40px space. */
       }
 
       .iron-selected:not(.menu-trigger) > iron-icon {
         fill: var(--google-blue-700);
       }
 
-      .menu-trigger span {
+      .menu-trigger iron-icon {
+        -webkit-margin-end: 22px;  /* 24px - 2px from margin for outline. */
+      }
+
+      a[is='action-link'] span {
         -webkit-margin-end: 16px;
         flex: 1;
       }
 
       paper-menu {
-        --paper-menu-selected-item: {
-          font-weight: 500;
+        padding: 0;
+      }
+
+      a[is='action-link'].no-outline {

[Dan Beam, 2017/02/06 17:28:12]

you shouldn't need this now that scottchen@ fixed this

[hcarmona, 2017/02/10 19:27:34]

Done.

+        --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;
         };
+        -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);
+        position: relative;  /* Needed for paper-ripple. */
       }
 
       #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 {
-        position: relative;  /* Needed for paper-ripple. */
-      }
-
       paper-ripple {
         color: var(--google-blue-700);
         opacity: .5;
       }
 
       .page-menu paper-menu {
         --paper-menu-selected-item: {
           color: var(--google-blue-700);
           font-weight: 500;
         };
       }
 
-      .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;

[dschuyler, 2017/02/02 20:46:53]

Will this scale well with super large fonts? 

[hcarmona, 2017/02/10 19:27:34]

Updated, no more line-height.

+        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="href" 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>
-          <div data-path="/bluetooth" on-tap="openPage_">
-            <iron-icon icon="settings:bluetooth"></iron-icon>
-            $i18n{bluetoothPageTitle}
-          </div>
+        <a is="action-link" href="/internet" on-tap="openPage_">

[Dan Beam, 2017/02/06 17:28:12]

wait why do we need both href AND is="action-link"?  <a href> has the a11y
benefits of being in the tab order already

[hcarmona, 2017/02/10 19:27:34]

We don't. Updated.

+          <iron-icon icon="settings:network-wifi"></iron-icon>
+          <span>$i18n{internetPageTitle}</span>

[dschuyler, 2017/02/02 20:46:53]

Can these spans be removed (here and below in this file)?

[Dan Beam, 2017/02/06 17:28:12]

the span is targeted by css above

hcarmona: why do we need to make the action links display: flex;?

[hcarmona, 2017/02/10 19:27:34]

Spans are not necessary, they were left overs from a previous version. Removed.

Flex on #advancedButton > span so that the expanded/collapsed icon is to the
right.

+        </a>
+        <a is="action-link" href="/bluetooth" on-tap="openPage_">
+          <iron-icon icon="settings:bluetooth"></iron-icon>
+          <span>$i18n{bluetoothPageTitle}</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" href="/people" on-tap="openPage_"
+            hidden="[[!pageVisibility.people]]">
+          <iron-icon icon="settings:people"></iron-icon>
+          <span>$i18n{peoplePageTitle}</span>
+        </a>
+        <a is="action-link" href="/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" href="/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" href="/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" href="/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" href="/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" href="/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>
-        <paper-menu attr-for-selected="data-path" class="menu-content"
+        </a>
+        <paper-menu attr-for-selected="href" class="menu-content"
             id="advancedMenu" selected="[[currentRoute.path]]">
 <if expr="chromeos">
-          <div data-path="/dateTime" on-tap="openPage_">
+          <a is="action-link" href="/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" href="/privacy" on-tap="openPage_">
             <iron-icon icon="settings:security"></iron-icon>
-            $i18n{privacyPageTitle}
-          </div>
-          <div data-path="/passwordsAndForms" on-tap="openPage_"
+            <span>$i18n{privacyPageTitle}</span>
+          </a>
+          <a is="action-link" href="/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" href="/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" href="/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" href="/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" href="/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" href="/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" href="/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_" href="/help">
+        <span>$i18n{aboutPageTitle}</span>
+      </a>
     </paper-menu>
   </template>
   <script src="settings_menu.js"></script>
 </dom-module>