Skip to content

Commit

Permalink
[Settings - Themes]: Implementing dropdown in settings to change them…
Browse files Browse the repository at this point in the history
…es (#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
  • Loading branch information
Michael Liao authored Oct 30, 2020
1 parent 92db863 commit 973927c
Show file tree
Hide file tree
Showing 12 changed files with 213 additions and 34 deletions.
14 changes: 14 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
]
}
}
},
Expand Down
8 changes: 6 additions & 2 deletions postBuildStep.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,15 @@ import {
applyInspectorCommonCssRightToolbarPatch,
applyInspectorCommonCssTabSliderPatch,
applyInspectorCommonNetworkPatch,
applyMainThemePatch,
applyMainViewPatch,
applyPersistRequestBlockingTab,
applyRemoveBreakOnContextMenuItem,
applyRemoveNonSupportedRevealContextMenu,
applySetTabIconPatch,
applyShowElementsTab,
applyShowRequestBlockingTab,
applyThemePatch,
} from "./src/host/polyfills/simpleView";
import applySetupTextSelectionPatch from "./src/host/polyfills/textSelection";

Expand Down Expand Up @@ -92,6 +94,7 @@ async function patchFilesForWebView(toolsOutDir: string) {
applyInspectorCommonCssTabSliderPatch,
]);
await patchFileForWebViewWrapper("main/main.js", toolsOutDir, [
applyMainThemePatch,
applyMainViewPatch,
]);
await patchFileForWebViewWrapper("elements/elements.js", toolsOutDir, [
Expand All @@ -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,
])
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand All @@ -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");
});
});
});
30 changes: 30 additions & 0 deletions src/common/settingsProvider.ts
Original file line number Diff line number Diff line change
@@ -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;
}
}
23 changes: 0 additions & 23 deletions src/common/tabSettingsProvider.ts

This file was deleted.

5 changes: 4 additions & 1 deletion src/common/webviewEvents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -12,6 +12,7 @@ export const webviewEventNames: WebviewEvent[] = [
"telemetry",
"websocket",
"getApprovedTabs",
"getThemes",
];

export type WebSocketEvent = "open" | "close" | "error" | "message";
Expand All @@ -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; }
Expand Down
25 changes: 25 additions & 0 deletions src/devtoolsPanel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
12 changes: 10 additions & 2 deletions src/devtoolsPanel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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 });
}

Expand Down
31 changes: 31 additions & 0 deletions src/host/polyfills/simpleView.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
});
});
46 changes: 46 additions & 0 deletions src/host/polyfills/simpleView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
30 changes: 30 additions & 0 deletions src/host/toolsHost.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
encodeMessageForChannel,
IOpenEditorData,
TelemetryData,
ThemeString,
WebSocketEvent,
WebviewEvent,
} from "../common/webviewEvents";
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -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;
}
Expand Down
Loading

0 comments on commit 973927c

Please sign in to comment.