Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Settings - Themes]: Implementing dropdown in settings to change themes #229

Merged
merged 8 commits into from
Oct 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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));
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vidorteg - I checked, and we do need this bind call to access the getThemes function

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