Skip to content

Commit

Permalink
personalization: Fix arrow key traversal in Google Photos photos grid.
Browse files Browse the repository at this point in the history 
When focus traversal was first implemented it was assumed that every
grid row held the same number of items. This is no longer the case.

Bug: b:229972583
Change-Id: Ia679efddb4d2320ed1cd8b2a9832f0cea2a811cb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3629724
Reviewed-by: Jeffrey Young <cowmoo@chromium.org>
Commit-Queue: David Black <dmblack@google.com>
Cr-Commit-Position: refs/heads/main@{#1001126}
  • Loading branch information
David Black authored and Chromium LUCI CQ committed May 9, 2022
1 parent 7630679 commit a5c55a0
Show file tree
Hide file tree
Showing 2 changed files with 168 additions and 1 deletion.
27 changes: 26 additions & 1 deletion ash/webui/personalization_app/resources/trusted/wallpaper/google_photos_photos_element.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,18 @@ export class GooglePhotosPhotos extends WithPersonalizationStore {
model: {index: number, row: GooglePhotosPhoto[]},
}) {
switch (normalizeKeyForRTL(e.key, this.i18n('textdirection') === 'rtl')) {
case 'ArrowDown':
if (e.model.index < this.photosByRow_!.length - 1) {
// To be consistent with default iron-list grid behavior, the down
// arrow should only advance focus to the succeeding grid row if an
// item at the same column index as is currently focused exists.
const nextGridRow = this.photosByRow_[e.model.index + 1];
if (this.focusedColIndex_ >= nextGridRow.length) {
e.preventDefault();
e.stopPropagation();
}
}
return;
case 'ArrowLeft':
if (this.focusedColIndex_ > 0) {
// Left arrow moves focus to the preceding grid item.
Expand All @@ -226,7 +238,8 @@ export class GooglePhotosPhotos extends WithPersonalizationStore {
} else if (e.model.index > 0) {
// Left arrow moves focus to the preceding grid item, wrapping to the
// preceding grid row.
this.focusedColIndex_ = e.model.row.length - 1;
const previousGridRow = this.photosByRow_[e.model.index - 1];
this.focusedColIndex_ = previousGridRow.length - 1;
this.$.grid.focusItem(e.model.index - 1);
}
return;
Expand All @@ -242,6 +255,18 @@ export class GooglePhotosPhotos extends WithPersonalizationStore {
this.$.grid.focusItem(e.model.index + 1);
}
return;
case 'ArrowUp':
if (e.model.index > 0) {
// To be consistent with default iron-list grid behavior, the up arrow
// should only advance focus to the preceding grid row if an item at
// the same column index as is currently focused exists.
const previousGridRow = this.photosByRow_[e.model.index - 1];
if (this.focusedColIndex_ >= previousGridRow.length) {
e.preventDefault();
e.stopPropagation();
}
}
return;
case 'Tab':
// The grid contains a single |focusable| row which becomes a focus trap
// due to the synthetic redirect of focus events to grid items. To
Expand Down
142 changes: 142 additions & 0 deletions chrome/test/data/webui/chromeos/personalization_app/google_photos_photos_element_test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,31 @@ suite('GooglePhotosPhotosTest', function() {
let personalizationStore: TestPersonalizationStore;
let wallpaperProvider: TestWallpaperProvider;

/** Dispatches a keydown event to |element| for the specified |key|. */
function dispatchKeydown(element: HTMLElement, key: string) {
const init: KeyboardEventInit = {bubbles: true, key};
switch (key) {
case 'ArrowDown':
init.keyCode = 40;
break;
case 'ArrowRight':
init.keyCode = 39;
break;
case 'ArrowLeft':
init.keyCode = 37;
break;
case 'ArrowUp':
init.keyCode = 38;
break;
}
element.dispatchEvent(new KeyboardEvent('keydown', init));
}

/** Returns the active element in |googlePhotosPhotosElement|'s shadow DOM. */
function getActiveElement(): Element|null {
return googlePhotosPhotosElement!.shadowRoot!.activeElement;
}

/**
* Returns the match for |selector| in |googlePhotosPhotosElement|'s shadow
* DOM.
Expand Down Expand Up @@ -91,6 +116,16 @@ suite('GooglePhotosPhotosTest', function() {
return {data};
}

/**
* Waits for the specified |element| to be the active element in
* |googlePhotosPhotosElement|'s shadow DOM.
*/
async function waitForActiveElement(element: Element) {
while (googlePhotosPhotosElement!.shadowRoot!.activeElement !== element) {
await waitAfterNextRender(googlePhotosPhotosElement!);
}
}

setup(() => {
const mocks = baseSetup();
personalizationStore = mocks.personalizationStore;
Expand All @@ -103,6 +138,113 @@ suite('GooglePhotosPhotosTest', function() {
googlePhotosPhotosElement = null;
});

test('advances focus', async () => {
// Initialize |photos| to result in the following formation:
// First row
// [1]
// Second row
// [2] [3]
// Third row
// [4]
const photos: GooglePhotosPhoto[] = [
// First row.
{
id: '1',
name: '1',
date: toString16('First row'),
url: {url: '1'},
location: '1'
},
// Second row.
{
id: '2',
name: '2',
date: toString16('Second row'),
url: {url: '2'},
location: '2'
},
{
id: '3',
name: '3',
date: toString16('Second row'),
url: {url: '3'},
location: '3'
},
// Third row.
{
id: '4',
name: '4',
date: toString16('Third row'),
url: {url: '4'},
location: '4'
}
];

// Set values returned by |wallpaperProvider|.
wallpaperProvider.setGooglePhotosPhotos(photos);

// Initialize Google Photos data in the |personalizationStore|.
await initializeGooglePhotosData(wallpaperProvider, personalizationStore);

// Initialize |googlePhotosPhotosElement|.
googlePhotosPhotosElement =
initElement(GooglePhotosPhotos, {hidden: false});
await waitAfterNextRender(googlePhotosPhotosElement);

// Focus the first photo.
const photoSelector =
'wallpaper-grid-item:not([hidden]).photo:not([placeholder])';
const photoEls = querySelectorAll(photoSelector);
assertEquals(photoEls?.length, 4);
(photoEls?.[0] as HTMLElement).focus();
await waitForActiveElement(photoEls?.[0]!);

// Use the right arrow key to traverse to the last photo. Focus should pass
// through all the photos in between.
for (let i = 1; i <= 3; ++i) {
dispatchKeydown(getActiveElement()?.closest('.row')!, 'ArrowRight');
await waitForActiveElement(photoEls?.[i]!);
}

// Use the left arrow key to traverse to the first photo. Focus should pass
// through all the photos in between.
for (let i = 2; i >= 0; --i) {
dispatchKeydown(getActiveElement()?.closest('.row')!, 'ArrowLeft');
await waitForActiveElement(photoEls?.[i]!);
}

// Use the down arrow key to traverse to the last photo. Focus should only
// pass through the photos in between which are in the same column.
for (let i = 1; i <= 3; i = i + 2) {
dispatchKeydown(getActiveElement()?.closest('.row')!, 'ArrowDown');
await waitForActiveElement(photoEls?.[i]!);
}

// Use the up arrow key to traverse to the first photo. Focus should only
// pass through the photos in between which are in the same column.
for (let i = 1; i >= 0; --i) {
dispatchKeydown(getActiveElement()?.closest('.row')!, 'ArrowUp');
await waitForActiveElement(photoEls?.[i]!);
}

// Focus the third photo.
dispatchKeydown(getActiveElement()?.closest('.row')!, 'ArrowRight');
dispatchKeydown(getActiveElement()?.closest('.row')!, 'ArrowRight');
await waitForActiveElement(photoEls?.[2]!);

// Because no photo exists directly below the third photo, the down arrow
// key should do nothing.
dispatchKeydown(getActiveElement()?.closest('.row')!, 'ArrowDown');
await new Promise<void>(resolve => setTimeout(resolve, 100));
assertEquals(getActiveElement(), photoEls?.[2]);

// Because no photo exists directly above the third photo, the up arrow key
// should do nothing.
dispatchKeydown(getActiveElement()?.closest('.row')!, 'ArrowUp');
await new Promise<void>(resolve => setTimeout(resolve, 100));
assertEquals(getActiveElement(), photoEls?.[2]);
});

[true, false].forEach(
(dismissFromUser: boolean) =>
test('displays error when photos fail to load', async () => {
Expand Down

0 comments on commit a5c55a0

Please sign in to comment.