Skip to content

Commit

Permalink
CrOS Settings: Focus page after menu item select
Browse files Browse the repository at this point in the history
When infinite scroll is removed, clicking a menu item should shift
focus to the page contents.

Screen recording: http://shortn/_dngeqXdTXx

Bug: b/285968876
Test: browser_tests --gtest_filter="OSSettingsMainPageContainer*"
Test: browser_tests --gtest_filter="OSSettingsOsSettingsUi*"
Change-Id: Iff143d5cbe1e6b414df73ff7fa03090bab1f264d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4614412
Commit-Queue: Wes Okuhara <wesokuhara@google.com>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1159453}
  • Loading branch information
Wes Okuhara authored and Chromium LUCI CQ committed Jun 19, 2023
1 parent 3ee9d86 commit dd1c309
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,16 @@
<style>
:host {
/* Do not show outline when focused. */
outline: none;
}

/* When infinite scroll is removed, only active pages should be visible. */
:host-context(.revamp-wayfinding-enabled):host(:not([active])) {
display: none;
}
</style>
<!-- This element should be focusable (e.g. After this page's respective menu
item is selected). Set tabindex="-1" so this element is focusable, but not
reachable via sequential keyboard navigation. -->
<div id="focusHost" tabindex="-1"></div>
<slot></slot>
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export class PageDisplayerElement extends PolymerElement {
type: Boolean,
value: false,
reflectToAttribute: true,
observer: 'onActiveChanged',
},

section: {
Expand All @@ -41,6 +42,16 @@ export class PageDisplayerElement extends PolymerElement {

assert(this.section in Section, `Invalid section: ${this.section}.`);
}

override focus() {
this.shadowRoot!.getElementById('focusHost')!.focus();
}

private onActiveChanged(): void {
if (this.active) {
this.focus();
}
}
}

declare global {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ export const MainPageMixin = dedupingMixin(

// Show the respective page for |route|
page.active = true;

this.dispatchCustomEvent_('show-container');
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ suite('<main-page-container>', function() {
* Asserts the following:
* - Only one page is marked active
* - Active page does not have style "display: none"
* - Active page is focused
* - Inactive pages have style "display: none"
*/
function assertOnlyActivePageIsVisible(section) {
Expand All @@ -215,6 +216,7 @@ suite('<main-page-container>', function() {
numActive++;
assertNotEquals('none', displayStyle);
assertEquals(section, page.section);
assertEquals(page, mainPageContainer.shadowRoot.activeElement);
} else {
assertEquals('none', displayStyle);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ suite('<os-settings-ui> page visibility', () => {
* Asserts the following:
* - Only one page is marked active
* - Active page does not have style "display: none"
* - Active page is focused
* - Inactive pages have style "display: none"
*/
function assertOnlyActivePageIsVisible(pageName: PageName): void {
Expand All @@ -77,6 +78,7 @@ suite('<os-settings-ui> page visibility', () => {
numActive++;
assertNotEquals('none', displayStyle);
assertEquals(Section[pageName], page.section);
assertEquals(page, mainPageContainer.shadowRoot!.activeElement);
} else {
assertEquals('none', displayStyle);
}
Expand Down

0 comments on commit dd1c309

Please sign in to comment.