From 973927c4951eecf45e4a250907bf69d7687a038a Mon Sep 17 00:00:00 2001 From: Michael Liao Date: Fri, 30 Oct 2020 14:17:22 -0700 Subject: [PATCH] [Settings - Themes]: Implementing dropdown in settings to change themes (#229) * First pass - not working * Working - needs tests * Adding tests * Removing Chromium themes and adding settings description * Remove experimental iframe flag * Creating ThemeString type and moving switch logic to toolsHost * Updating tests * Rebasing/conflict resolution --- package.json | 14 ++++++ postBuildStep.ts | 8 +++- ...vider.test.ts => settingsProvider.test.ts} | 21 ++++++--- src/common/settingsProvider.ts | 30 ++++++++++++ src/common/tabSettingsProvider.ts | 23 ---------- src/common/webviewEvents.ts | 5 +- src/devtoolsPanel.test.ts | 25 ++++++++++ src/devtoolsPanel.ts | 12 ++++- src/host/polyfills/simpleView.test.ts | 31 +++++++++++++ src/host/polyfills/simpleView.ts | 46 +++++++++++++++++++ src/host/toolsHost.ts | 30 ++++++++++++ src/test/helpers.ts | 2 + 12 files changed, 213 insertions(+), 34 deletions(-) rename src/common/{tabSettingsProvider.test.ts => settingsProvider.test.ts} (50%) create mode 100644 src/common/settingsProvider.ts delete mode 100644 src/common/tabSettingsProvider.ts diff --git a/package.json b/package.json index 1c91fa20..86cb7ec0 100644 --- a/package.json +++ b/package.json @@ -183,6 +183,20 @@ "Microsoft Edge Tools for VS Code will use Microsoft Edge Dev version", "Microsoft Edge Tools for VS Code will use Microsoft Edge Canary version" ] + }, + "vscode-edge-devtools.themes": { + "type": "string", + "description": "Set the theme of the Microsoft Edge Tools extension (reload required after changing).", + "enum": [ + "System preference", + "Light", + "Dark" + ], + "enumDescriptions": [ + "Microsoft Edge Tools for VS Code will use the system preferred setting for themes", + "Microsoft Edge Tools for VS Code will use the Edge DevTools light theme", + "Microsoft Edge Tools for VS Code will use the Edge DevTools dark theme" + ] } } }, diff --git a/postBuildStep.ts b/postBuildStep.ts index c0ba47a2..3f489808 100644 --- a/postBuildStep.ts +++ b/postBuildStep.ts @@ -17,6 +17,7 @@ import { applyInspectorCommonCssRightToolbarPatch, applyInspectorCommonCssTabSliderPatch, applyInspectorCommonNetworkPatch, + applyMainThemePatch, applyMainViewPatch, applyPersistRequestBlockingTab, applyRemoveBreakOnContextMenuItem, @@ -24,6 +25,7 @@ import { applySetTabIconPatch, applyShowElementsTab, applyShowRequestBlockingTab, + applyThemePatch, } from "./src/host/polyfills/simpleView"; import applySetupTextSelectionPatch from "./src/host/polyfills/textSelection"; @@ -92,6 +94,7 @@ async function patchFilesForWebView(toolsOutDir: string) { applyInspectorCommonCssTabSliderPatch, ]); await patchFileForWebViewWrapper("main/main.js", toolsOutDir, [ + applyMainThemePatch, applyMainViewPatch, ]); await patchFileForWebViewWrapper("elements/elements.js", toolsOutDir, [ @@ -117,17 +120,18 @@ async function patchFilesForWebView(toolsOutDir: string) { applyShowElementsTab, applyShowRequestBlockingTab, ]); - await patchFileForWebViewWrapper("root/root.js", toolsOutDir, [ applyRuntimeImportScriptPathPrefixPatch, ]); - await patchFileForWebViewWrapper("quick_open/quick_open.js", toolsOutDir, [ applyCommandMenuPatch, applyQuickOpenPatch, ]); await patchFileForWebViewWrapper("browser_debugger/browser_debugger.js", toolsOutDir, [ applyRemoveBreakOnContextMenuItem, + ]); + await patchFileForWebViewWrapper("themes/themes.js", toolsOutDir, [ + applyThemePatch, ]) } diff --git a/src/common/tabSettingsProvider.test.ts b/src/common/settingsProvider.test.ts similarity index 50% rename from src/common/tabSettingsProvider.test.ts rename to src/common/settingsProvider.test.ts index c6bc3434..90890da3 100644 --- a/src/common/tabSettingsProvider.test.ts +++ b/src/common/settingsProvider.test.ts @@ -6,7 +6,7 @@ import { createFakeVSCode } from "../test/helpers"; jest.mock("vscode", () => createFakeVSCode(), { virtual: true }); -describe("tabSettingsProvider", () => { +describe("settingsProvider", () => { beforeEach(() => { const mockVSCode = createFakeVSCode(); jest.doMock("vscode", () => mockVSCode, { virtual: true }); @@ -15,20 +15,29 @@ describe("tabSettingsProvider", () => { describe("GetTabConfiguration", () => { it("test that singleton provides the right instance", async () => { - const tsp = await import("../common/tabSettingsProvider"); - const instance = tsp.TabSettingsProvider.instance; - const instanceB = tsp.TabSettingsProvider.instance; + const settingsProvider = await import("../common/settingsProvider"); + const instance = settingsProvider.SettingsProvider.instance; + const instanceB = settingsProvider.SettingsProvider.instance; expect(instance).not.toEqual(null); expect(instance).toEqual(instanceB); }); it("test that the right value is retrieved for networkEnabled configuration", async () => { jest.requireMock("vscode"); - const tsp = await import("../common/tabSettingsProvider"); - const instance = tsp.TabSettingsProvider.instance; + const settingsProvider = await import("../common/settingsProvider"); + const instance = settingsProvider.SettingsProvider.instance; expect(instance).not.toEqual(null); const result = instance.isNetworkEnabled(); expect(result).toEqual(true); }); + + it("test that the right value is retrieved for themeString configuration", async () => { + jest.requireMock("vscode"); + const settingsProvider = await import("../common/settingsProvider"); + const instance = settingsProvider.SettingsProvider.instance; + expect(instance).not.toEqual(null); + const result = instance.getThemeSettings(); + expect(result).toEqual("System preference"); + }); }); }); diff --git a/src/common/settingsProvider.ts b/src/common/settingsProvider.ts new file mode 100644 index 00000000..393c0ae0 --- /dev/null +++ b/src/common/settingsProvider.ts @@ -0,0 +1,30 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +import * as vscode from "vscode"; +import { SETTINGS_STORE_NAME } from "../utils"; +import { ThemeString } from "./webviewEvents"; + +export class SettingsProvider { + + private static singletonInstance: SettingsProvider; + + public isNetworkEnabled(): boolean { + const settings = vscode.workspace.getConfiguration(SETTINGS_STORE_NAME); + const networkEnabled: boolean = settings.get("enableNetwork") || false; + return networkEnabled; + } + + public getThemeSettings(): ThemeString { + const settings = vscode.workspace.getConfiguration(SETTINGS_STORE_NAME); + const themeString: ThemeString = settings.get("themes") || "System preference"; + return themeString; + } + + public static get instance() { + if (!SettingsProvider.singletonInstance) { + SettingsProvider.singletonInstance = new SettingsProvider(); + } + + return SettingsProvider.singletonInstance; + } +} diff --git a/src/common/tabSettingsProvider.ts b/src/common/tabSettingsProvider.ts deleted file mode 100644 index 0256a8da..00000000 --- a/src/common/tabSettingsProvider.ts +++ /dev/null @@ -1,23 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. -import * as vscode from "vscode"; -import { SETTINGS_STORE_NAME } from "../utils"; - -export class TabSettingsProvider { - - private static singletonInstance: TabSettingsProvider; - - public isNetworkEnabled(): boolean { - const settings = vscode.workspace.getConfiguration(SETTINGS_STORE_NAME); - const networkEnabled: boolean = settings.get("enableNetwork") || false; - return networkEnabled; - } - - public static get instance() { - if (!TabSettingsProvider.singletonInstance) { - TabSettingsProvider.singletonInstance = new TabSettingsProvider(); - } - - return TabSettingsProvider.singletonInstance; - } -} diff --git a/src/common/webviewEvents.ts b/src/common/webviewEvents.ts index 1de386f5..f7edff18 100644 --- a/src/common/webviewEvents.ts +++ b/src/common/webviewEvents.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. export type WebviewEvent = "getState" | "getUrl" | "openInEditor" | "ready" | "setState" | "telemetry" | "websocket" -| "getApprovedTabs"; +| "getApprovedTabs" | "getThemes"; export const webviewEventNames: WebviewEvent[] = [ "getState", "getUrl", @@ -12,6 +12,7 @@ export const webviewEventNames: WebviewEvent[] = [ "telemetry", "websocket", "getApprovedTabs", + "getThemes", ]; export type WebSocketEvent = "open" | "close" | "error" | "message"; @@ -22,6 +23,8 @@ export const webSocketEventNames: WebSocketEvent[] = [ "message", ]; +export type ThemeString = "System preference" | "Light" | "Dark"; + export type TelemetryEvent = "enumerated" | "performance" | "error"; export interface ITelemetryMeasures { [key: string]: number; } diff --git a/src/devtoolsPanel.test.ts b/src/devtoolsPanel.test.ts index 6350164e..0ac84931 100644 --- a/src/devtoolsPanel.test.ts +++ b/src/devtoolsPanel.test.ts @@ -492,6 +492,31 @@ describe("devtoolsPanel", () => { expect(mockPanel.webview.postMessage).toHaveBeenCalledWith(expectedPostedMessage); }); + it("calls getThemes", async () => { + const expectedId = { id: 0 }; + const expectedState = { themeString: "System preference" }; + (context.workspaceState.get as jest.Mock).mockReturnValue(expectedState); + + const dtp = await import("./devtoolsPanel"); + dtp.DevToolsPanel.createOrShow(context, mockTelemetry, "", mockRuntimeConfig); + + hookedEvents.get("getThemes")!(JSON.stringify(expectedId)); + expect(mockWebviewEvents.encodeMessageForChannel).toHaveBeenCalledWith( + expect.any(Function), + "getThemes", + { + themeString: expectedState.themeString, + id: expectedId.id, + }, + ); + + // Ensure that the encoded message is actually passed over to the webview + const expectedPostedMessage = "encodedMessage"; + const { callback, thisObj } = getFirstCallback(mockWebviewEvents.encodeMessageForChannel); + callback.call(thisObj, expectedPostedMessage); + expect(mockPanel.webview.postMessage).toHaveBeenCalledWith(expectedPostedMessage); + }); + it("shows an error for unmapped urls", async () => { const expectedRequest = { column: 5, diff --git a/src/devtoolsPanel.ts b/src/devtoolsPanel.ts index 4006bc1a..ac2f0607 100644 --- a/src/devtoolsPanel.ts +++ b/src/devtoolsPanel.ts @@ -5,7 +5,7 @@ import * as vscode from "vscode"; import * as debugCore from "vscode-chrome-debug-core"; import TelemetryReporter from "vscode-extension-telemetry"; -import { TabSettingsProvider } from "./common/tabSettingsProvider"; +import { SettingsProvider } from "./common/settingsProvider"; import { encodeMessageForChannel, IOpenEditorData, @@ -56,6 +56,7 @@ export class DevToolsPanel { this.panelSocket.on("telemetry", (msg) => this.onSocketTelemetry(msg)); this.panelSocket.on("getState", (msg) => this.onSocketGetState(msg)); this.panelSocket.on("getApprovedTabs", (msg) => this.onSocketGetApprovedTabs(msg)); + this.panelSocket.on("getThemes", (msg) => this.onSocketGetThemes(msg)); this.panelSocket.on("setState", (msg) => this.onSocketSetState(msg)); this.panelSocket.on("getUrl", (msg) => this.onSocketGetUrl(msg)); this.panelSocket.on("openInEditor", (msg) => this.onSocketOpenInEditor(msg)); @@ -164,7 +165,14 @@ export class DevToolsPanel { private onSocketGetApprovedTabs(message: string) { const { id } = JSON.parse(message) as { id: number }; encodeMessageForChannel((msg) => this.panel.webview.postMessage(msg), "getApprovedTabs", { - enableNetwork: TabSettingsProvider.instance.isNetworkEnabled(), + enableNetwork: SettingsProvider.instance.isNetworkEnabled(), + id }); + } + + private onSocketGetThemes(message: string) { + const { id } = JSON.parse(message) as { id: number }; + encodeMessageForChannel((msg) => this.panel.webview.postMessage(msg), "getThemes", { + themeString: SettingsProvider.instance.getThemeSettings(), id }); } diff --git a/src/host/polyfills/simpleView.test.ts b/src/host/polyfills/simpleView.test.ts index c6076bc0..36b264b9 100644 --- a/src/host/polyfills/simpleView.test.ts +++ b/src/host/polyfills/simpleView.test.ts @@ -294,4 +294,35 @@ describe("simpleView", () => { expect(result).not.toEqual(null); expect(result).toEqual(expect.stringContaining(expectedResult)); }); + + it("applyThemePatch correctly modifies themes to use theme parameter", async () => { + const filePath = "themes/themes.js"; + const fileContents = getTextFromFile(filePath); + if (!fileContents) { + throw new Error(`Could not find file: ${filePath}`); + } + const expectedResult = "function init(theme)"; + const expectedResult2 = "if(theme){themeSetting.set(theme);}"; + const apply = await import("./simpleView"); + const result = apply.applyThemePatch(fileContents); + expect(result).not.toEqual(null); + expect(result).toEqual(expect.stringContaining(expectedResult)); + expect(result).toEqual(expect.stringContaining(expectedResult2)); + }); + + it("applyMainThemePatch correctly modifes main.js to pass in themes from settings", async () => { + const filePath = "main/main.js"; + const fileContents = getTextFromFile(filePath); + if (!fileContents) { + throw new Error(`Could not find file: ${filePath}`); + } + + const expectedResult = "resolve(theme);"; + const expectedResult2 = "await this.getThemePromise()"; + const apply = await import("./simpleView"); + const result = apply.applyMainThemePatch(fileContents); + expect(result).not.toEqual(null); + expect(result).toEqual(expect.stringContaining(expectedResult)); + expect(result).toEqual(expect.stringContaining(expectedResult2)); + }); }); diff --git a/src/host/polyfills/simpleView.ts b/src/host/polyfills/simpleView.ts index a8fb0d91..fb481cdc 100644 --- a/src/host/polyfills/simpleView.ts +++ b/src/host/polyfills/simpleView.ts @@ -401,3 +401,49 @@ export function applyRemoveNonSupportedRevealContextMenu(content: string) { return null; } } + +export function getThemes(callback: (arg0: object) => void) { + InspectorFrontendHost.InspectorFrontendHostInstance.getThemes(callback); +} + +export function applyThemePatch(content: string) { + // Sets the theme of the DevTools + const parameterPattern = /function init\(\)/; + if (content.match(parameterPattern)) { + content = content.replace(parameterPattern, `function init(theme)`); + } else { + return null; + } + + const setPattern = /const settingDescriptor/; + if (content.match(setPattern)) { + return content.replace(setPattern, `if(theme){themeSetting.set(theme);} const settingDescriptor`); + } else { + return null; + } +} + +export function applyMainThemePatch(content: string) { + // Sets the theme of the DevTools + const injectFunctionsPattern = /async _createAppUI/; + if (content.match(injectFunctionsPattern)) { + content = content.replace(injectFunctionsPattern, `${getThemes.toString().slice(9)} getThemePromise(){ + const promise = new Promise(function(resolve){ + this.getThemes((object)=>{ + const theme = object.theme; + resolve(theme); + }); + }.bind(this)); + return promise; + } async _createAppUI`); + } else { + return null; + } + + const createAppPattern = /;init\(\);/; + if (content.match(createAppPattern)) { + return content.replace(createAppPattern, `;const theme = await this.getThemePromise(); init(theme);`); + } else { + return null; + } +} diff --git a/src/host/toolsHost.ts b/src/host/toolsHost.ts index 37fea2fb..aea1da68 100644 --- a/src/host/toolsHost.ts +++ b/src/host/toolsHost.ts @@ -5,6 +5,7 @@ import { encodeMessageForChannel, IOpenEditorData, TelemetryData, + ThemeString, WebSocketEvent, WebviewEvent, } from "../common/webviewEvents"; @@ -86,6 +87,12 @@ export default class ToolsHost { encodeMessageForChannel((msg) => window.parent.postMessage(msg, "*"), "getApprovedTabs", {id}); } + public getThemes(callback: (arg0: object) => void) { + const id = this.getHostCallbacksNextId++; + this.getHostCallbacks.set(id, callback); + encodeMessageForChannel((msg) => window.parent.postMessage(msg, "*"), "getThemes", {id}); + } + public onMessageFromChannel(e: WebviewEvent, args: string): boolean { switch (e) { case "getState": { @@ -113,6 +120,29 @@ export default class ToolsHost { }); break; } + + case "getThemes": { + const parsedArgs = JSON.parse(args); + const id: number = parsedArgs.id; + const themeString: ThemeString = parsedArgs.themeString; + let theme; + switch(themeString) { + case 'System preference': + theme = 'systemPreferred'; + break; + case 'Light': + theme = 'default'; + break; + case 'Dark': + theme = 'dark'; + break; + default: + theme = null; + } + this.fireGetHostCallback(id, { + theme, + }) + } } return true; } diff --git a/src/test/helpers.ts b/src/test/helpers.ts index 9afa89be..10475312 100644 --- a/src/test/helpers.ts +++ b/src/test/helpers.ts @@ -62,6 +62,8 @@ export function createFakeVSCode() { switch(name) { case "enableNetwork": return true; + case "themeString": + return "System preference"; default: return undefined; }