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

Issues Search
    My Issues | Starred     Open | Closed | All

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

Issue 2651293003: Make MD Settings side bar keyboard accessible. (Closed)
Patch Set: href the links Created 3 years, 11 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
« no previous file with comments | « no previous file | chrome/browser/resources/settings/settings_menu/settings_menu.js » ('j') | chrome/browser/resources/settings/settings_menu/settings_menu.js » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 5c42519ce4c700ee4cbf337622e4240dd279a0ef..8d29487a49dd8e0ead2b64c7dd094060c9c5a351 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">
Dan Beam 2017/02/06 17:28:12 you haven't imported action_link_css.html but we
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">
@@ -19,51 +20,57 @@
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
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;
@@ -76,142 +83,135 @@
};
}
- .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"
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
Dan Beam 2017/02/06 17:28:12 the span is targeted by css above hcarmona: why d
hcarmona 2017/02/10 19:27:34 Spans are not necessary, they were left overs from
+ </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>
« no previous file with comments | « no previous file | chrome/browser/resources/settings/settings_menu/settings_menu.js » ('j') | chrome/browser/resources/settings/settings_menu/settings_menu.js » ('J')

Powered by Google App Engine
This is Rietveld 408576698