From 64e0899efa07a281ed44288cf6fab12e83e47f8a Mon Sep 17 00:00:00 2001 From: Mike Jackson Date: Wed, 18 Mar 2020 17:46:54 -0700 Subject: [PATCH] Avoid automatically reloading DevTools on Theme change Avoid automatically reloading the DevTools when the user changes the theme within the DevTools . The user will be notified that a Reload is required, and the settings page now has a "Reload DevTools" button so the user can apply the changes immediately. Reloading the DevTools automatically can result in the user losing any state with their current debugging sessions (e.g. pause location, console logs, style changes, etc). https://imgur.com/a/47ICsdi Bug: 1001549 Change-Id: I338dea7610a9a51c8292743a84c274a60e034b34 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2107500 Commit-Queue: Mike Jackson Reviewed-by: Robert Paveza --- front_end/emulation/emulation_strings.grdp | 3 --- front_end/emulation/sensors.css | 5 ----- front_end/langpacks/shared_strings.grdp | 6 ++++++ front_end/main/MainImpl.js | 1 - front_end/main/main_strings.grdp | 3 --- front_end/main/module.json | 1 + front_end/settings/SettingsScreen.js | 5 ++++- front_end/settings/settings_strings.grdp | 6 +++--- front_end/ui/SettingsUI.js | 14 ++++++++++++-- front_end/ui/inspectorCommon.css | 5 +++++ 10 files changed, 31 insertions(+), 18 deletions(-) diff --git a/front_end/emulation/emulation_strings.grdp b/front_end/emulation/emulation_strings.grdp index 46f07d901c..2d7a4fd8a1 100644 --- a/front_end/emulation/emulation_strings.grdp +++ b/front_end/emulation/emulation_strings.grdp @@ -192,9 +192,6 @@ Timezone ID - - *Requires reload - Capture screenshot diff --git a/front_end/emulation/sensors.css b/front_end/emulation/sensors.css index 73e532db17..c5e3541d95 100644 --- a/front_end/emulation/sensors.css +++ b/front_end/emulation/sensors.css @@ -293,11 +293,6 @@ fieldset.device-orientation-override-section { background: #f1f1f1; } -.reload-warning { - align-self: center; - margin-left: 10px; -} - button.text-button { margin: 0 10px; } diff --git a/front_end/langpacks/shared_strings.grdp b/front_end/langpacks/shared_strings.grdp index 758c0f1ca8..f75d23a6ae 100644 --- a/front_end/langpacks/shared_strings.grdp +++ b/front_end/langpacks/shared_strings.grdp @@ -295,6 +295,9 @@ Size + + Reload DevTools + Open file @@ -325,6 +328,9 @@ Clear all + + *Requires reload + (index) diff --git a/front_end/main/MainImpl.js b/front_end/main/MainImpl.js index cc001a1265..f8cce6e213 100644 --- a/front_end/main/MainImpl.js +++ b/front_end/main/MainImpl.js @@ -201,7 +201,6 @@ export class MainImpl { const themeSetting = Common.Settings.Settings.instance().createSetting('uiTheme', 'systemPreferred'); UI.UIUtils.initializeUIUtils(document, themeSetting); - themeSetting.addChangeListener(Components.Reload.reload.bind(Components)); UI.UIUtils.installComponentRootStyles(/** @type {!Element} */ (document.body)); diff --git a/front_end/main/main_strings.grdp b/front_end/main/main_strings.grdp index 7974a305c9..41cd5711ae 100644 --- a/front_end/main/main_strings.grdp +++ b/front_end/main/main_strings.grdp @@ -75,9 +75,6 @@ Toggle dock side - - Reload DevTools - Find next/previous diff --git a/front_end/main/module.json b/front_end/main/module.json index 6c7a62dcc8..0d52bcc8a4 100644 --- a/front_end/main/module.json +++ b/front_end/main/module.json @@ -266,6 +266,7 @@ "settingName": "uiTheme", "settingType": "enum", "defaultValue": "systemPreferred", + "reloadRequired": true, "options": [ { "title": "Switch to system preferred color theme", diff --git a/front_end/settings/SettingsScreen.js b/front_end/settings/SettingsScreen.js index 640ae5350a..4d8b3e5c95 100644 --- a/front_end/settings/SettingsScreen.js +++ b/front_end/settings/SettingsScreen.js @@ -165,7 +165,10 @@ export class GenericSettingsTab extends SettingsTab { self.runtime.extensions(UI.SettingsUI.SettingUI).forEach(this._addSettingUI.bind(this)); this._appendSection().appendChild( - UI.UIUtils.createTextButton(Common.UIString.UIString('Restore defaults and reload'), restoreAndReload)); + UI.UIUtils.createTextButton(Common.UIString.UIString('Reload DevTools'), Components.Reload.reload)); + + this._appendSection().appendChild(UI.UIUtils.createTextButton( + Common.UIString.UIString('Restore defaults and reload DevTools'), restoreAndReload)); function restoreAndReload() { Common.Settings.Settings.instance().clearAll(); diff --git a/front_end/settings/settings_strings.grdp b/front_end/settings/settings_strings.grdp index d87d76feca..02b8dc8270 100644 --- a/front_end/settings/settings_strings.grdp +++ b/front_end/settings/settings_strings.grdp @@ -1,8 +1,5 @@ - - Restore defaults and reload - Blackbox @@ -30,6 +27,9 @@ WARNING: + + Restore defaults and reload DevTools + No blackboxed patterns diff --git a/front_end/ui/SettingsUI.js b/front_end/ui/SettingsUI.js index 22a5550be6..6bef2dda92 100644 --- a/front_end/ui/SettingsUI.js +++ b/front_end/ui/SettingsUI.js @@ -62,11 +62,12 @@ export const createSettingCheckbox = function(name, setting, omitParagraphElemen /** * @param {string} name * @param {!Array} options + * @param {boolean} reloadRequired * @param {!Common.Settings.Setting} setting * @param {string=} subtitle * @return {!Element} */ -const createSettingSelect = function(name, options, setting, subtitle) { +const createSettingSelect = function(name, options, reloadRequired, setting, subtitle) { const settingSelectElement = createElement('p'); const label = settingSelectElement.createChild('label'); const select = settingSelectElement.createChild('select', 'chrome-select'); @@ -84,6 +85,12 @@ const createSettingSelect = function(name, options, setting, subtitle) { select.add(new Option(optionName, option.value)); } + const reloadWarning = reloadRequired ? settingSelectElement.createChild('span', 'reload-warning hidden') : null; + if (reloadWarning) { + reloadWarning.textContent = ls`*Requires reload`; + ARIAUtils.markAsAlert(reloadWarning); + } + setting.addChangeListener(settingChanged); settingChanged(); select.addEventListener('change', selectChanged, false); @@ -101,6 +108,9 @@ const createSettingSelect = function(name, options, setting, subtitle) { function selectChanged() { // Don't use event.target.value to avoid conversion of the value to string. setting.set(options[select.selectedIndex].value); + if (reloadWarning) { + reloadWarning.classList.remove('hidden'); + } } }; @@ -156,7 +166,7 @@ export const createControlForSetting = function(setting, subtitle) { return createSettingCheckbox(uiTitle, setting); case 'enum': if (Array.isArray(descriptor['options'])) { - return createSettingSelect(uiTitle, descriptor['options'], setting, subtitle); + return createSettingSelect(uiTitle, descriptor['options'], descriptor['reloadRequired'], setting, subtitle); } console.error('Enum setting defined without options'); return null; diff --git a/front_end/ui/inspectorCommon.css b/front_end/ui/inspectorCommon.css index 1703fd4965..e839beabeb 100644 --- a/front_end/ui/inspectorCommon.css +++ b/front_end/ui/inspectorCommon.css @@ -451,6 +451,11 @@ span[is=dt-icon-label] { background-color: #9e9e9e; } +.reload-warning { + align-self: center; + margin-left: 10px; +} + button.link { border: none; background: none;