Skip to content

Commit

Permalink
ChromeOS Settings: Fix focus restoration when exiting languages/input…
Browse files Browse the repository at this point in the history
…Methods.

 - Remove what seems to be unnecessary hardcoded paths from
   settings-animated-pages.
 - Fix usage of |focusConfig| mechanism from os-settings-languages-page.
   Before this CL, os-settings-languages-page was not using the correct
   |focusConfig| instance.

Bug: 1003992
Change-Id: I17dc56115f166b4e862d8a2ec7e0d41388902dfc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1894748
Reviewed-by: Jordy Greenblatt <jordynass@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711968}
  • Loading branch information
freshp86 authored and Commit Bot committed Nov 2, 2019
1 parent eef2ff5 commit 0a90d35
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -57,16 +57,10 @@ Polymer({
/** @private */
showAddLanguagesDialog_: Boolean,

/** @private {!Map<string, string>} */
focusConfig_: {
/** @type {!Map<string, (string|Function)>} */
focusConfig: {
type: Object,
value: function() {
const map = new Map();
if (settings.routes.INPUT_METHODS) {
map.set(settings.routes.INPUT_METHODS.path, '#manageInputMethods');
}
return map;
},
observer: 'focusConfigChanged_',
},

/** @private */
Expand All @@ -81,6 +75,20 @@ Polymer({
/** @private {boolean} */
isChangeInProgress_: false,

/**
* @param {!Map<string, (string|Function)>} newConfig
* @param {?Map<string, (string|Function)>} oldConfig
* @private
*/
focusConfigChanged_: function(newConfig, oldConfig) {
// focusConfig is set only once on the parent, so this observer should only
// fire once.
assert(!oldConfig);
this.focusConfig.set(
settings.routes.INPUT_METHODS.path,
() => cr.ui.focusWithoutInk(this.$.manageInputMethods));
},

/**
* Stamps and opens the Add Languages dialog, registering a listener to
* disable the dialog's dom-if again on close.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
associated-control="[[$$('#languagesSubpageTrigger')]]"
page-title="$i18n{osLanguagesPageTitle}">
<os-settings-languages-page
focus-config="[[focusConfig_]]"
language-helper="{{languageHelper}}"
languages="{{languages}}"
prefs="{{prefs}}">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

<link rel="import" href="chrome://resources/html/assert.html">
<link rel="import" href="chrome://resources/html/cr.html">
<link rel="import" href="chrome://resources/html/cr/ui.html">
<link rel="import" href="chrome://resources/html/cr/ui/focus_without_ink.html">
<link rel="import" href="chrome://resources/html/util.html">
<link rel="import" href="chrome://resources/polymer/v1_0/iron-pages/iron-pages.html">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,37 +79,6 @@ Polymer({
return;
}

const subpagePaths = [];
if (settings.routes.SITE_SETTINGS_COOKIES) {
subpagePaths.push(settings.routes.SITE_SETTINGS_COOKIES.path);
}

if (settings.routes.SITE_SETTINGS_SITE_DATA) {
subpagePaths.push(settings.routes.SITE_SETTINGS_SITE_DATA.path);
}

if (settings.routes.SITE_SETTINGS_ALL) {
subpagePaths.push(settings.routes.SITE_SETTINGS_ALL.path);
}

// <if expr="chromeos">
if (settings.routes.INTERNET_NETWORKS) {
subpagePaths.push(settings.routes.INTERNET_NETWORKS.path);
}
// </if>

// Only handle iron-select events from div elements and the
// given whitelist of settings-subpage instances.
const whitelist = ['settings-subpage#site-settings', 'div[route-path]'];
whitelist.push.apply(
whitelist,
subpagePaths.map(path => `settings-subpage[route-path="${path}"]`));
const query = whitelist.join(', ');

if (!e.detail.item.matches(query)) {
return;
}

// Ensure focus-config was correctly specified as a Polymer property.
assert(this.focusConfig instanceof Map);

Expand Down

0 comments on commit 0a90d35

Please sign in to comment.