[CrOS Settings] Allow arrow navigation through changePicture camera icon
Background
A change from February, 2018 [1] adjusted the changePicture page's
behavior so that when the user navigates to the camera and the camera
is ready to show its video stream (corresponding to the event 'canplay'
on the nested <video> element), the camera's "takePhoto" button becomes
the active element. Before this, the camera would appear active to the
user but it would require an extra click on the cr-picture-list's
camera icon to actually activate it for taking a photo.
However, because the focus went to the "takePhoto" button, it left the
cr-picture-list altogether and therefore it was no longer receiving
keyboard events. As a result, the user would be unable to continue
navigating through the list without clicking on it again.
-----
This CL listens for keyboard events on the cr-camera and passes the
arrow key events to the cr-picture-list (and gives it the focus).
-----
Testing
In addition to the browser test below that navigates away from and back
to the camera icon with the arrow keys, I recorded a live test [2] of
the CL on my test device in which I only used arrow keys to navigate and
made sure to navigate to the camera icon and navigate away right after
the video stream started playing. I also took photos using the keyboard
to show that the "one click photo" experience enabled by [1] was not
affected.
[1] https://chromium-review.googlesource.com/c/chromium/src/+/914700
[2] https://drive.google.com/file/d/1ZLN750Df4kVX2ctnk35n6GFoHRF5kQ9u
Bug: 818621
Change-Id: Ica5e5bcd2a349d5c0117b76c330935120377290a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1674587
Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Reviewed-by: May Lippert <maybelle@chromium.org>
Commit-Queue: Jordy Greenblatt <jordynass@chromium.org>
Cr-Commit-Position: refs/heads/master@{#673103}
diff --git a/chrome/browser/resources/settings/people_page/change_picture.html b/chrome/browser/resources/settings/people_page/change_picture.html
index d83319e..1dc7a5d0 100644
--- a/chrome/browser/resources/settings/people_page/change_picture.html
+++ b/chrome/browser/resources/settings/people_page/change_picture.html
@@ -88,7 +88,8 @@
capture-video-label="$i18n{captureVideo}"
switch-mode-to-camera-label="$i18n{switchModeToCamera}"
switch-mode-to-video-label="$i18n{switchModeToVideo}"
- camera-video-mode-enabled="[[cameraVideoModeEnabled_]]">
+ camera-video-mode-enabled="[[cameraVideoModeEnabled_]]"
+ on-keys-pressed="onCameraPaneKeysPressed_">
</cr-picture-pane>
<div id="authorCredit"
hidden="[[!isAuthorCreditShown_(selectedItem_)]]">
diff --git a/chrome/browser/resources/settings/people_page/change_picture.js b/chrome/browser/resources/settings/people_page/change_picture.js
index f97aa38e..8dd7052 100644
--- a/chrome/browser/resources/settings/people_page/change_picture.js
+++ b/chrome/browser/resources/settings/people_page/change_picture.js
@@ -237,6 +237,17 @@
videomode ? 'videoModeAccessibleText' : 'photoModeAccessibleText'));
},
+ /**
+ * Callback the iron-a11y-keys "keys-pressed" event bubbles up from the
+ * cr-camera-pane.
+ * @param {!CustomEvent<!{key: string, keyboardEvent: Object}>} event
+ * @private
+ */
+ onCameraPaneKeysPressed_(event) {
+ this.$.pictureList.focus();
+ this.$.pictureList.onKeysPressed(event);
+ },
+
/** @private */
onDiscardImage_: function() {
// Prevent image from being discarded if old image is pending.
diff --git a/chrome/test/data/webui/settings/people_page_change_picture_test.js b/chrome/test/data/webui/settings/people_page_change_picture_test.js
index 22f1eb0..0dae771 100644
--- a/chrome/test/data/webui/settings/people_page_change_picture_test.js
+++ b/chrome/test/data/webui/settings/people_page_change_picture_test.js
@@ -99,6 +99,25 @@
let crPicturePane = null;
let crPictureList = null;
+ const LEFT_KEY_CODE = 37;
+ const RIGHT_KEY_CODE = 39;
+
+ /**
+ * @return {Array<HTMLElement>} Traverses the DOM tree to find the lowest
+ * level active element and returns an array of the node path down the
+ * tree, skipping shadow roots.
+ */
+ function getActiveElementPath() {
+ let node = document.activeElement;
+ let path = [];
+ while (node) {
+ path.push(node);
+ node = (node.shadowRoot || node).activeElement;
+ }
+ return path;
+ }
+
+
suiteSetup(function() {
loadTimeData.overrideValues({
profilePhoto: 'Fake Profile Photo description',
@@ -129,6 +148,57 @@
changePicture.remove();
});
+ test('TraverseCameraIconUsingArrows', function() {
+ // Force the camera to be present.
+ cr.webUIListenerCallback('camera-presence-changed', true);
+ Polymer.dom.flush();
+ assertTrue(crPictureList.cameraPresent);
+
+ // Click camera icon.
+ const cameraImage = crPictureList.$.cameraImage;
+ cameraImage.click();
+ Polymer.dom.flush();
+
+ assertTrue(crPictureList.cameraSelected_);
+ const crCamera = crPicturePane.$$('#camera');
+ assertTrue(!!crCamera);
+
+ // Mock camera's video stream beginning to play.
+ crCamera.$.cameraVideo.dispatchEvent(new Event('canplay'));
+ Polymer.dom.flush();
+
+ // "Take photo" button should be active.
+ let activeElementPath = getActiveElementPath();
+ assertTrue(activeElementPath.includes(crPicturePane));
+ assertFalse(activeElementPath.includes(crPictureList));
+
+ // Press 'Right' key on active element.
+ MockInteractions.pressAndReleaseKeyOn(
+ activeElementPath.pop(), RIGHT_KEY_CODE);
+ Polymer.dom.flush();
+
+ // A profile picture open should be active.
+ activeElementPath = getActiveElementPath();
+ assertFalse(crPictureList.cameraSelected_);
+ assertFalse(activeElementPath.includes(crPicturePane));
+ assertTrue(activeElementPath.includes(crPictureList));
+
+ // Press 'Left' key on active element.
+ MockInteractions.pressAndReleaseKeyOn(
+ activeElementPath.pop(), LEFT_KEY_CODE);
+ Polymer.dom.flush();
+
+ // Mock camera's video stream beginning to play.
+ crCamera.$.cameraVideo.dispatchEvent(new Event('canplay'));
+ Polymer.dom.flush();
+
+ // "Take photo" button should be active again.
+ activeElementPath = getActiveElementPath();
+ assertTrue(crPictureList.cameraSelected_);
+ assertTrue(activeElementPath.includes(crPicturePane));
+ assertFalse(activeElementPath.includes(crPictureList));
+ });
+
test('ChangePictureSelectCamera', function() {
// Force the camera to be absent, even if it's actually present.
cr.webUIListenerCallback('camera-presence-changed', false);
@@ -301,7 +371,7 @@
// Now verify that arrow keys actually select the new image.
browserProxy.resetResolver('selectDefaultImage');
MockInteractions.pressAndReleaseKeyOn(
- changePicture.selectedItem_, 39 /* right */);
+ changePicture.selectedItem_, RIGHT_KEY_CODE);
return browserProxy.whenCalled('selectDefaultImage');
})
.then(function(args) {
diff --git a/ui/webui/resources/cr_elements/chromeos/cr_picture/cr_camera.html b/ui/webui/resources/cr_elements/chromeos/cr_picture/cr_camera.html
index 7d115878..2a06b18 100644
--- a/ui/webui/resources/cr_elements/chromeos/cr_picture/cr_camera.html
+++ b/ui/webui/resources/cr_elements/chromeos/cr_picture/cr_camera.html
@@ -2,6 +2,7 @@
<link rel="import" href="../../shared_style_css.html">
<link rel="import" href="../../cr_icon_button/cr_icon_button.html">
+<link rel="import" href="chrome://resources/polymer/v1_0/iron-a11y-keys/iron-a11y-keys.html">
<link rel="import" href="chrome://resources/polymer/v1_0/paper-spinner/paper-spinner-lite.html">
<link rel="import" href="cr_png_behavior.html">
@@ -146,6 +147,7 @@
<!-- Empty div for even 'space-between' justification -->
</div>
<div>
+ <iron-a11y-keys keys="up down left right"></iron-a11y-keys>
<cr-icon-button id="takePhoto" tabindex="1"
title="[[getTakePhotoLabel_(videomode, takePhotoLabel,
captureVideoLabel)]]" on-click="takePhoto"
diff --git a/ui/webui/resources/cr_elements/chromeos/cr_picture/cr_picture_list.html b/ui/webui/resources/cr_elements/chromeos/cr_picture/cr_picture_list.html
index baa5b428..672ae8a 100644
--- a/ui/webui/resources/cr_elements/chromeos/cr_picture/cr_picture_list.html
+++ b/ui/webui/resources/cr_elements/chromeos/cr_picture/cr_picture_list.html
@@ -57,7 +57,7 @@
<div id="container">
<iron-a11y-keys keys="up down left right space enter"
- on-keys-pressed="onKeysPressed_">
+ on-keys-pressed="onKeysPressed">
</iron-a11y-keys>
<iron-selector id="selector"
on-iron-activate="onIronActivate_" on-iron-select="onIronSelect_"
diff --git a/ui/webui/resources/cr_elements/chromeos/cr_picture/cr_picture_list.js b/ui/webui/resources/cr_elements/chromeos/cr_picture/cr_picture_list.js
index eedea168..03eddc2 100644
--- a/ui/webui/resources/cr_elements/chromeos/cr_picture/cr_picture_list.js
+++ b/ui/webui/resources/cr_elements/chromeos/cr_picture/cr_picture_list.js
@@ -168,30 +168,10 @@
},
/**
- * @param {!CrPicture.ImageElement} image
- */
- setSelectedImage_(image) {
- this.fallbackImage_ = image;
- // If the user is currently taking a photo, do not change the focus.
- if (!this.selectedItem ||
- this.selectedItem.dataset.type != CrPicture.SelectionTypes.CAMERA) {
- this.$.selector.select(this.$.selector.indexOf(image));
- this.selectedItem = image;
- }
- },
-
- /** @private */
- onDefaultImagesChanged_: function() {
- if (this.selectedImageUrl_) {
- this.setSelectedImageUrl(this.selectedImageUrl_);
- }
- },
-
- /**
* Handler for when accessibility-specific keys are pressed.
* @param {!CustomEvent<!{key: string, keyboardEvent: Object}>} e
*/
- onKeysPressed_: function(e) {
+ onKeysPressed: function(e) {
if (!this.selectedItem) {
return;
}
@@ -224,6 +204,26 @@
},
/**
+ * @param {!CrPicture.ImageElement} image
+ */
+ setSelectedImage_(image) {
+ this.fallbackImage_ = image;
+ // If the user is currently taking a photo, do not change the focus.
+ if (!this.selectedItem ||
+ this.selectedItem.dataset.type != CrPicture.SelectionTypes.CAMERA) {
+ this.$.selector.select(this.$.selector.indexOf(image));
+ this.selectedItem = image;
+ }
+ },
+
+ /** @private */
+ onDefaultImagesChanged_: function() {
+ if (this.selectedImageUrl_) {
+ this.setSelectedImageUrl(this.selectedImageUrl_);
+ }
+ },
+
+ /**
* @param {!CrPicture.ImageElement} selected
* @param {boolean} activate
* @private
