Skip to content

Commit

Permalink
os_settings: Delete global_scroll_target_behavior.js
Browse files Browse the repository at this point in the history
The use in chrome://os-settings/osLanguages/editDictionary is a remnant
of the old browser settings design, where all settings elements are
displayed on a single long scrollable page: https://crrev.com/2792463002

The use in chrome://os-settings/osLanguages/japaneseManageUserDictionary
copied the existing use in editDictionary.

This is not being used any more, and introduces issues for the
TypeScript migration as the mixin that replaces it is tightly coupled to
the routes used - which are different for OS settings and browser
settings. Using the mixin from browser settings causes runtime errors
due to this.

Both routes still work after this CL, and no visible behaviour has been
changed.

Bug: None
Change-Id: I11273c693a478b625bbdf0257b1fba3f072cf589
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4111831
Reviewed-by: Wes Okuhara <wesokuhara@google.com>
Commit-Queue: Michael Cui <mlcui@google.com>
Cr-Commit-Position: refs/heads/main@{#1085224}
  • Loading branch information
mlcui-corp authored and Chromium LUCI CQ committed Dec 20, 2022
1 parent d633fa0 commit 0c4826e
Show file tree
Hide file tree
Showing 7 changed files with 3 additions and 185 deletions.
9 changes: 0 additions & 9 deletions chrome/browser/resources/settings/chromeos/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,6 @@ js_type_check("closure_compile_local_module") {
deps = [
":deep_linking_behavior",
":find_shortcut_behavior",
":global_scroll_target_behavior",
":icon",
":lazy_load",
":metrics_recorder",
Expand Down Expand Up @@ -371,14 +370,6 @@ js_library("find_shortcut_behavior") {
]
}

js_library("global_scroll_target_behavior") {
deps = [
":route_observer_behavior",
":router",
"//ash/webui/common/resources:promise_resolver",
]
}

js_library("metrics_recorder") {
deps = [
"//chrome/browser/ui/webui/settings/ash/search:mojo_bindings_webui_js",
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,6 @@ js_library("os_add_languages_dialog") {
js_library("os_edit_dictionary_page") {
deps = [
":languages_browser_proxy",
"..:global_scroll_target_behavior",
"..:os_route",
"//third_party/polymer/v3_0/components-chromium/iron-list:iron-list",
"//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled",
Expand All @@ -208,7 +207,6 @@ js_library("os_edit_dictionary_page") {

js_library("os_japanese_manage_user_dictionary_page") {
deps = [
"..:global_scroll_target_behavior",
"..:os_route",
"//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled",
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,7 @@ import '../../settings_shared.css.js';
import {I18nBehavior, I18nBehaviorInterface} from 'chrome://resources/ash/common/i18n_behavior.js';
import {mixinBehaviors, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';

import {Route} from '../router.js';
import {GlobalScrollTargetBehavior, GlobalScrollTargetBehaviorInterface} from '../global_scroll_target_behavior.js';
import {recordSettingChange} from '../metrics_recorder.js';
import {routes} from '../os_route.js';

import {LanguagesBrowserProxy, LanguagesBrowserProxyImpl} from './languages_browser_proxy.js';
import {getTemplate} from './os_edit_dictionary_page.html.js';
Expand All @@ -42,10 +39,9 @@ const NewWordState = {
* @constructor
* @extends {PolymerElement}
* @implements {I18nBehaviorInterface}
* @implements {GlobalScrollTargetBehaviorInterface}
*/
const OsSettingsEditDictionaryPageElementBase =
mixinBehaviors([I18nBehavior, GlobalScrollTargetBehavior], PolymerElement);
mixinBehaviors([I18nBehavior], PolymerElement);

/** @polymer */
class OsSettingsEditDictionaryPageElement extends
Expand All @@ -66,16 +62,6 @@ class OsSettingsEditDictionaryPageElement extends
value: '',
},

/**
* Needed for GlobalScrollTargetBehavior.
* @type {!Route}
* @override
*/
subpageRoute: {
type: Object,
value: routes.OS_LANGUAGES_EDIT_DICTIONARY,
},

/** @private {!Array<string>} */
words_: {
type: Array,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,15 @@ import '../../settings_shared.css.js';
import {I18nBehavior, I18nBehaviorInterface} from 'chrome://resources/ash/common/i18n_behavior.js';
import {mixinBehaviors, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';

import {Route} from '../router.js';
import {GlobalScrollTargetBehavior, GlobalScrollTargetBehaviorInterface} from '../global_scroll_target_behavior.js';
import {routes} from '../os_route.js';

import {getTemplate} from './os_japanese_manage_user_dictionary_page.html.js';

/**
* @constructor
* @extends {PolymerElement}
* @implements {I18nBehaviorInterface}
* @implements {GlobalScrollTargetBehaviorInterface}
*/
const OsSettingsJapaneseManageUserDictionaryPageElementBase =
mixinBehaviors([I18nBehavior, GlobalScrollTargetBehavior], PolymerElement);
mixinBehaviors([I18nBehavior], PolymerElement);

/** @polymer */
class OsSettingsJapaneseManageUserDictionaryPageElement extends
Expand All @@ -40,17 +35,7 @@ class OsSettingsJapaneseManageUserDictionaryPageElement extends
}

static get properties() {
return {
/**
* Needed for GlobalScrollTargetBehavior.
* @type {!Route}
* @override
*/
subpageRoute: {
type: Object,
value: routes.OS_LANGUAGES_JAPANESE_MANAGE_USER_DICTIONARY,
},
};
return {};
}
}

Expand Down
1 change: 0 additions & 1 deletion chrome/browser/resources/settings/chromeos/os_settings.gni
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,6 @@ non_web_component_files = [
"chromeos/device_page/layout_mixin.ts",
"chromeos/ensure_lazy_loaded.ts",
"chromeos/find_shortcut_behavior.js",
"chromeos/global_scroll_target_behavior.js",
"chromeos/google_assistant_page/google_assistant_browser_proxy.ts",
"chromeos/guest_os/guest_os_browser_proxy.ts",
"chromeos/icon.js",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import {Debouncer, DomIf, microTask, PolymerElement, timeOut} from 'chrome://res
import {loadTimeData} from '../../i18n_setup.js';
import {SettingsPrefsElement} from '../../prefs/prefs.js';
import {castExists} from '../assert_extras.js';
import {setGlobalScrollTarget} from '../global_scroll_target_behavior.js';
import {recordClick, recordNavigation, recordPageBlur, recordPageFocus, recordSettingChange} from '../metrics_recorder.js';
import {convertPrefToSettingMetric} from '../metrics_utils.js';
import {OSPageVisibility, osPageVisibility} from '../os_page_visibility.js';
Expand Down Expand Up @@ -277,7 +276,6 @@ class OsSettingsUiElement extends OsSettingsUiElementBase {

// Preload bold Roboto so it doesn't load and flicker the first time used.
document.fonts.load('bold 12px Roboto');
setGlobalScrollTarget(this.$.container);

const scrollToTop = (top: number) => new Promise<void>(resolve => {
if (this.$.container.scrollTop === top) {
Expand Down

0 comments on commit 0c4826e

Please sign in to comment.