Commit fc896c04 authored by Caroline Rising's avatar Caroline Rising Committed by Commit Bot

Read later: Add keyboard navigation and enable selecting items with Enter/Space.

Allow tab/shift+tab to navigate through all elements in the UI. Let up/down arrows move focus between read later list items. Let right/left arrows cycle focus between a list item and its buttons. This behavior comes from discussion during a11y office hours.

Still TODO: add dark mode support for buttons and hover/focus states (crbug/1147457).

Bug: 1140166
Change-Id: I3a119e8d940a4307e746c10f3ded7541963f46f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2527345
Commit-Queue: Caroline Rising <corising@chromium.org>
Reviewed-by: default avatardpapad <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825964}
parent ad6362a1
...@@ -96,8 +96,10 @@ js_library("app") { ...@@ -96,8 +96,10 @@ js_library("app") {
deps = [ deps = [
":read_later_api_proxy", ":read_later_api_proxy",
":read_later_item", ":read_later_item",
"//third_party/polymer/v3_0/components-chromium/iron-selector:iron-selector",
"//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled", "//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled",
"//ui/webui/resources/cr_elements/cr_icon_button:cr_icon_button.m", "//ui/webui/resources/cr_elements/cr_icon_button:cr_icon_button.m",
"//ui/webui/resources/js:assert.m",
] ]
} }
...@@ -113,6 +115,7 @@ js_library("read_later_item") { ...@@ -113,6 +115,7 @@ js_library("read_later_item") {
":read_later_api_proxy", ":read_later_api_proxy",
"//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled", "//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled",
"//ui/webui/resources/cr_elements/cr_icon_button:cr_icon_button.m", "//ui/webui/resources/cr_elements/cr_icon_button:cr_icon_button.m",
"//ui/webui/resources/js:assert.m",
] ]
} }
......
...@@ -32,6 +32,14 @@ ...@@ -32,6 +32,14 @@
overflow-y: auto; overflow-y: auto;
} }
.mwb-list-item:focus-within {
background-color: var(--mwb-list-item-hover-background-color);
}
.mwb-list-item:active {
background-color: var(--mwb-list-item-selected-background-color);
}
.sub-heading { .sub-heading {
align-items: center; align-items: center;
border-bottom: 1px solid #dbdbdb; border-bottom: 1px solid #dbdbdb;
...@@ -52,14 +60,17 @@ ...@@ -52,14 +60,17 @@
</cr-icon-button> </cr-icon-button>
</div> </div>
<div id="read-later-list"> <div id="read-later-list">
<div class="sub-heading">$i18n{unreadHeader}</div> <iron-selector id="selector" on-keydown="onItemKeyDown_"
<template id="ureadItemsList" is="dom-repeat" items="[[unreadItems_]]"> attr-for-selected="data-url" selectable="read-later-item">
<read-later-item data-url$="[[item.url]]" class="mwb-list-item" <div class="sub-heading">$i18n{unreadHeader}</div>
data="[[item]]"></read-later-item> <template id="ureadItemsList" is="dom-repeat" items="[[unreadItems_]]">
</template> <read-later-item data-url$="[[item.url.url]]" on-focus="onItemFocus_"
<div class="sub-heading">$i18n{readHeader}</div> class="mwb-list-item" data="[[item]]" tabindex="0"></read-later-item>
<template id="readItemsList" is="dom-repeat" items="[[readItems_]]"> </template>
<read-later-item data-url$="[[item.url]]" class="mwb-list-item" <div class="sub-heading">$i18n{readHeader}</div>
data="[[item]]"></read-later-item> <template id="readItemsList" is="dom-repeat" items="[[readItems_]]">
</template> <read-later-item data-url$="[[item.url.url]]" on-focus="onItemFocus_"
class="mwb-list-item" data="[[item]]" tabindex="0"></read-later-item>
</template>
</iron-selector>
</div> </div>
...@@ -6,12 +6,17 @@ import 'chrome://resources/cr_elements/cr_icon_button/cr_icon_button.m.js'; ...@@ -6,12 +6,17 @@ import 'chrome://resources/cr_elements/cr_icon_button/cr_icon_button.m.js';
import 'chrome://resources/cr_elements/shared_vars_css.m.js'; import 'chrome://resources/cr_elements/shared_vars_css.m.js';
import 'chrome://resources/cr_elements/mwb_shared_style.js'; import 'chrome://resources/cr_elements/mwb_shared_style.js';
import 'chrome://resources/cr_elements/mwb_shared_vars.js'; import 'chrome://resources/cr_elements/mwb_shared_vars.js';
import 'chrome://resources/polymer/v3_0/iron-selector/iron-selector.js';
import {assertNotReached} from 'chrome://resources/js/assert.m.js';
import {html, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js'; import {html, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
import {ReadLaterApiProxy, ReadLaterApiProxyImpl} from './read_later_api_proxy.js'; import {ReadLaterApiProxy, ReadLaterApiProxyImpl} from './read_later_api_proxy.js';
import {ReadLaterItemElement} from './read_later_item.js'; import {ReadLaterItemElement} from './read_later_item.js';
/** @type {!Set<string>} */
const navigationKeys = new Set(['ArrowDown', 'ArrowUp']);
export class ReadLaterAppElement extends PolymerElement { export class ReadLaterAppElement extends PolymerElement {
static get is() { static get is() {
return 'read-later-app'; return 'read-later-app';
...@@ -76,6 +81,41 @@ export class ReadLaterAppElement extends PolymerElement { ...@@ -76,6 +81,41 @@ export class ReadLaterAppElement extends PolymerElement {
}); });
} }
/**
* @param {!Event} e
* @private
*/
onItemKeyDown_(e) {
if (e.shiftKey || !navigationKeys.has(e.key)) {
return;
}
const selector = /** @type {!IronSelectorElement} */ (this.$.selector);
switch (e.key) {
case 'ArrowDown':
selector.selectNext();
/** @type {!ReadLaterItemElement} */ (selector.selectedItem).focus();
break;
case 'ArrowUp':
selector.selectPrevious();
/** @type {!ReadLaterItemElement} */ (selector.selectedItem).focus();
break;
default:
assertNotReached();
return;
}
e.preventDefault();
e.stopPropagation();
}
/**
* @param {!Event} e
* @private
*/
onItemFocus_(e) {
this.$.selector.selected =
/** @type {!ReadLaterItemElement} */ (e.currentTarget).dataset.url;
}
/** /**
* @param {!Event} e * @param {!Event} e
* @private * @private
......
<style> <style>
/* TODO(corising): Add dark mode support for buttons and hover/focus states
* (crbug/1147457).*/
:host(:focus) {
outline: none;
}
.button-container { .button-container {
display: flex; display: flex;
margin-inline-start: calc(var(--mwb-icon-size) / 2); margin-inline-start: calc(var(--mwb-icon-size) / 2);
...@@ -6,7 +12,8 @@ ...@@ -6,7 +12,8 @@
width: 0; width: 0;
} }
:host(:hover) .button-container { :host(:hover) .button-container,
:host(:focus-within) .button-container {
overflow: visible; overflow: visible;
width: auto; width: auto;
} }
...@@ -44,7 +51,8 @@ ...@@ -44,7 +51,8 @@
--cr-icon-button-margin-end: 0; --cr-icon-button-margin-end: 0;
} }
:host(:hover) cr-icon-button { :host(:hover) cr-icon-button,
:host(:focus-within) cr-icon-button {
--cr-icon-button-fill-color: var(--google-grey-refresh-700); --cr-icon-button-fill-color: var(--google-grey-refresh-700);
} }
...@@ -52,6 +60,10 @@ ...@@ -52,6 +60,10 @@
background-color: rgba(var(--google-grey-900-rgb), .1); background-color: rgba(var(--google-grey-900-rgb), .1);
} }
cr-icon-button:focus {
box-shadow: 0 0 0 2px rgba(var(--google-blue-600-rgb), 0.4);
}
#deleteButton { #deleteButton {
margin-inline-start: calc(var(--cr-icon-button-size) / 2); margin-inline-start: calc(var(--cr-icon-button-size) / 2);
} }
......
...@@ -9,10 +9,14 @@ import 'chrome://resources/cr_elements/mwb_shared_vars.js'; ...@@ -9,10 +9,14 @@ import 'chrome://resources/cr_elements/mwb_shared_vars.js';
import 'chrome://resources/cr_elements/shared_vars_css.m.js'; import 'chrome://resources/cr_elements/shared_vars_css.m.js';
import './icons.js'; import './icons.js';
import {assertNotReached} from 'chrome://resources/js/assert.m.js';
import {html, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js'; import {html, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
import {ReadLaterApiProxy, ReadLaterApiProxyImpl} from './read_later_api_proxy.js'; import {ReadLaterApiProxy, ReadLaterApiProxyImpl} from './read_later_api_proxy.js';
/** @type {!Set<string>} */
const navigationKeys = new Set([' ', 'Enter', 'ArrowRight', 'ArrowLeft']);
export class ReadLaterItemElement extends PolymerElement { export class ReadLaterItemElement extends PolymerElement {
static get is() { static get is() {
return 'read-later-item'; return 'read-later-item';
...@@ -39,6 +43,7 @@ export class ReadLaterItemElement extends PolymerElement { ...@@ -39,6 +43,7 @@ export class ReadLaterItemElement extends PolymerElement {
ready() { ready() {
super.ready(); super.ready();
this.addEventListener('click', this.onClick_); this.addEventListener('click', this.onClick_);
this.addEventListener('keydown', this.onKeyDown_.bind(this));
} }
/** @private */ /** @private */
...@@ -46,6 +51,45 @@ export class ReadLaterItemElement extends PolymerElement { ...@@ -46,6 +51,45 @@ export class ReadLaterItemElement extends PolymerElement {
this.apiProxy_.openSavedEntry(this.data.url); this.apiProxy_.openSavedEntry(this.data.url);
} }
/**
* @param {!Event} e
* @private
*/
onKeyDown_(e) {
if (e.shiftKey || !navigationKeys.has(e.key)) {
return;
}
switch (e.key) {
case ' ':
case 'Enter':
this.onClick_();
break;
case 'ArrowRight':
if (!this.shadowRoot.activeElement) {
this.shadowRoot.getElementById('updateStatusButton').focus();
} else if (this.shadowRoot.activeElement.nextElementSibling) {
this.shadowRoot.activeElement.nextElementSibling.focus();
} else {
this.focus();
}
break;
case 'ArrowLeft':
if (!this.shadowRoot.activeElement) {
this.shadowRoot.getElementById('deleteButton').focus();
} else if (this.shadowRoot.activeElement.previousElementSibling) {
this.shadowRoot.activeElement.previousElementSibling.focus();
} else {
this.focus();
}
break;
default:
assertNotReached();
return;
}
e.preventDefault();
e.stopPropagation();
}
/** /**
* @param {!Event} e * @param {!Event} e
* @private * @private
......
...@@ -22,6 +22,7 @@ js_library("read_later_app_test") { ...@@ -22,6 +22,7 @@ js_library("read_later_app_test") {
":test_read_later_api_proxy", ":test_read_later_api_proxy",
"..:chai_assert", "..:chai_assert",
"//chrome/browser/resources/read_later:app", "//chrome/browser/resources/read_later:app",
"//third_party/polymer/v3_0/components-chromium/iron-test-helpers:mock-interactions",
] ]
externs_list = [ "$externs_path/mocha-2.5.js" ] externs_list = [ "$externs_path/mocha-2.5.js" ]
} }
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
import {ReadLaterAppElement} from 'chrome://read-later/app.js'; import {ReadLaterAppElement} from 'chrome://read-later/app.js';
import {ReadLaterApiProxy, ReadLaterApiProxyImpl} from 'chrome://read-later/read_later_api_proxy.js'; import {ReadLaterApiProxy, ReadLaterApiProxyImpl} from 'chrome://read-later/read_later_api_proxy.js';
import {keyDownOn} from 'chrome://resources/polymer/v3_0/iron-test-helpers/mock-interactions.js';
import {assertEquals, assertFalse, assertTrue} from '../chai_assert.js'; import {assertEquals, assertFalse, assertTrue} from '../chai_assert.js';
import {flushTasks} from '../test_util.m.js'; import {flushTasks} from '../test_util.m.js';
...@@ -23,7 +24,7 @@ suite('ReadLaterAppTest', () => { ...@@ -23,7 +24,7 @@ suite('ReadLaterAppTest', () => {
function assertEntryURLs(items, urls) { function assertEntryURLs(items, urls) {
assertEquals(urls.length, items.length); assertEquals(urls.length, items.length);
items.forEach((item, index) => { items.forEach((item, index) => {
assertEquals(urls[index], item.data.url); assertEquals(urls[index], item.dataset.url);
}); });
} }
...@@ -43,7 +44,7 @@ suite('ReadLaterAppTest', () => { ...@@ -43,7 +44,7 @@ suite('ReadLaterAppTest', () => {
unreadEntries: [ unreadEntries: [
{ {
title: 'Google', title: 'Google',
url: 'https://www.google.com', url: {url: 'https://www.google.com'},
displayUrl: 'google.com', displayUrl: 'google.com',
updateTime: 0, updateTime: 0,
read: false, read: false,
...@@ -51,7 +52,7 @@ suite('ReadLaterAppTest', () => { ...@@ -51,7 +52,7 @@ suite('ReadLaterAppTest', () => {
}, },
{ {
title: 'Apple', title: 'Apple',
url: 'https://www.apple.com', url: {url: 'https://www.apple.com'},
displayUrl: 'apple.com', displayUrl: 'apple.com',
updateTime: 0, updateTime: 0,
read: false, read: false,
...@@ -61,7 +62,7 @@ suite('ReadLaterAppTest', () => { ...@@ -61,7 +62,7 @@ suite('ReadLaterAppTest', () => {
readEntries: [ readEntries: [
{ {
title: 'Bing', title: 'Bing',
url: 'https://www.bing.com', url: {url: 'https://www.bing.com'},
displayUrl: 'bing.com', displayUrl: 'bing.com',
updateTime: 0, updateTime: 0,
read: true, read: true,
...@@ -69,7 +70,7 @@ suite('ReadLaterAppTest', () => { ...@@ -69,7 +70,7 @@ suite('ReadLaterAppTest', () => {
}, },
{ {
title: 'Yahoo', title: 'Yahoo',
url: 'https://www.yahoo.com', url: {url: 'https://www.yahoo.com'},
displayUrl: 'yahoo.com', displayUrl: 'yahoo.com',
updateTime: 0, updateTime: 0,
read: true, read: true,
...@@ -105,7 +106,7 @@ suite('ReadLaterAppTest', () => { ...@@ -105,7 +106,7 @@ suite('ReadLaterAppTest', () => {
const expectedUrl = 'https://www.apple.com'; const expectedUrl = 'https://www.apple.com';
clickItem(expectedUrl); clickItem(expectedUrl);
const url = await testProxy.whenCalled('openSavedEntry'); const url = await testProxy.whenCalled('openSavedEntry');
assertEquals(url, expectedUrl); assertEquals(url.url, expectedUrl);
}); });
test('Click on item mark as read button triggers actions', async () => { test('Click on item mark as read button triggers actions', async () => {
...@@ -117,7 +118,7 @@ suite('ReadLaterAppTest', () => { ...@@ -117,7 +118,7 @@ suite('ReadLaterAppTest', () => {
readLaterItem.shadowRoot.querySelector('#updateStatusButton'); readLaterItem.shadowRoot.querySelector('#updateStatusButton');
readLaterItemUpdateStatusButton.click(); readLaterItemUpdateStatusButton.click();
const [url, read] = await testProxy.whenCalled('updateReadStatus'); const [url, read] = await testProxy.whenCalled('updateReadStatus');
assertEquals(expectedUrl, url); assertEquals(expectedUrl, url.url);
assertTrue(read); assertTrue(read);
}); });
...@@ -130,7 +131,7 @@ suite('ReadLaterAppTest', () => { ...@@ -130,7 +131,7 @@ suite('ReadLaterAppTest', () => {
readLaterItem.shadowRoot.querySelector('#updateStatusButton'); readLaterItem.shadowRoot.querySelector('#updateStatusButton');
readLaterItemUpdateStatusButton.click(); readLaterItemUpdateStatusButton.click();
const [url, read] = await testProxy.whenCalled('updateReadStatus'); const [url, read] = await testProxy.whenCalled('updateReadStatus');
assertEquals(expectedUrl, url); assertEquals(expectedUrl, url.url);
assertFalse(read); assertFalse(read);
}); });
...@@ -143,7 +144,7 @@ suite('ReadLaterAppTest', () => { ...@@ -143,7 +144,7 @@ suite('ReadLaterAppTest', () => {
readLaterItem.shadowRoot.querySelector('#deleteButton'); readLaterItem.shadowRoot.querySelector('#deleteButton');
readLaterItemDeleteButton.click(); readLaterItemDeleteButton.click();
const url = await testProxy.whenCalled('removeEntry'); const url = await testProxy.whenCalled('removeEntry');
assertEquals(expectedUrl, url); assertEquals(expectedUrl, url.url);
}); });
test('Click on menu button triggers actions', async () => { test('Click on menu button triggers actions', async () => {
...@@ -152,4 +153,83 @@ suite('ReadLaterAppTest', () => { ...@@ -152,4 +153,83 @@ suite('ReadLaterAppTest', () => {
readLaterCloseButton.click(); readLaterCloseButton.click();
await testProxy.whenCalled('closeUI'); await testProxy.whenCalled('closeUI');
}); });
test('Enter key triggers action and passes correct url', async () => {
const expectedUrl = 'https://www.apple.com';
const readLaterItem = /** @type {!Element} */
(readLaterApp.shadowRoot.querySelector(`[data-url="${expectedUrl}"]`));
keyDownOn(readLaterItem, 0, [], 'Enter');
const url = await testProxy.whenCalled('openSavedEntry');
assertEquals(url.url, expectedUrl);
});
test('Space key triggers action and passes correct url', async () => {
const expectedUrl = 'https://www.apple.com';
const readLaterItem = /** @type {!Element} */
(readLaterApp.shadowRoot.querySelector(`[data-url="${expectedUrl}"]`));
keyDownOn(readLaterItem, 0, [], ' ');
const url = await testProxy.whenCalled('openSavedEntry');
assertEquals(url.url, expectedUrl);
});
test('Keyboard navigation abides by item list range boundaries', async () => {
const urls = [
'https://www.google.com', 'https://www.apple.com', 'https://www.bing.com',
'https://www.yahoo.com'
];
const selector = readLaterApp.shadowRoot.querySelector('iron-selector');
// Select first item.
selector.selected =
readLaterApp.shadowRoot.querySelector('read-later-item').dataset.url;
keyDownOn(selector, 0, [], 'ArrowUp');
assertEquals(urls[3], selector.selected);
keyDownOn(selector, 0, [], 'ArrowDown');
assertEquals(urls[0], selector.selected);
keyDownOn(selector, 0, [], 'ArrowDown');
assertEquals(urls[1], selector.selected);
keyDownOn(selector, 0, [], 'ArrowUp');
assertEquals(urls[0], selector.selected);
});
test(
'Keyboard navigation left/right cycles through list item elements',
async () => {
const firstItem =
readLaterApp.shadowRoot.querySelector('read-later-item');
// Focus first item.
firstItem.focus();
keyDownOn(firstItem, 0, [], 'ArrowRight');
assertEquals(
firstItem.shadowRoot.getElementById('updateStatusButton'),
firstItem.shadowRoot.activeElement);
keyDownOn(firstItem, 0, [], 'ArrowRight');
assertEquals(
firstItem.shadowRoot.getElementById('deleteButton'),
firstItem.shadowRoot.activeElement);
keyDownOn(firstItem, 0, [], 'ArrowRight');
assertEquals(firstItem, readLaterApp.shadowRoot.activeElement);
keyDownOn(firstItem, 0, [], 'ArrowLeft');
assertEquals(
firstItem.shadowRoot.getElementById('deleteButton'),
firstItem.shadowRoot.activeElement);
keyDownOn(firstItem, 0, [], 'ArrowLeft');
assertEquals(
firstItem.shadowRoot.getElementById('updateStatusButton'),
firstItem.shadowRoot.activeElement);
keyDownOn(firstItem, 0, [], 'ArrowLeft');
assertEquals(firstItem, readLaterApp.shadowRoot.activeElement);
});
}); });
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment