Skip to content

Extract leetcode webview base class & add md settings listener #270

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Apr 16, 2019
Merged

Extract leetcode webview base class & add md settings listener #270

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d4c026f
Extract leetcode webview base class & add md setings listener
Vigilans Apr 14, 2019
4c34172
Make solution provider extend webview base
Vigilans Apr 14, 2019
d0ccd53
Make preview provider extend webview base
Vigilans Apr 14, 2019
82ec804
Rename result provider to submission provider & make it extend webvie…
Vigilans Apr 14, 2019
df68b0e
Submission provider fix
Vigilans Apr 14, 2019
90c9480
Make `onDidChangeConfiguration` inheritable
Vigilans Apr 14, 2019
9f0999e
Make two event callbacks inheritable class method
Vigilans Apr 14, 2019
85a7164
Bring `onDidDispose` to webview class member
Vigilans Apr 15, 2019
084e844
Replace `this.context` field with `this.listener` & Further update in…
Vigilans Apr 16, 2019
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
4 changes: 2 additions & 2 deletions src/commands/submit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { leetCodeExecutor } from "../leetCodeExecutor";
import { leetCodeManager } from "../leetCodeManager";
import { DialogType, promptForOpenOutputChannel, promptForSignIn } from "../utils/uiUtils";
import { getActiveFilePath } from "../utils/workspaceUtils";
import { leetCodeResultProvider } from "../webview/leetCodeResultProvider";
import { leetCodeSubmissionProvider } from "../webview/leetCodeSubmissionProvider";

export async function submitSolution(uri?: vscode.Uri): Promise<void> {
if (!leetCodeManager.getUser()) {
Expand All @@ -21,7 +21,7 @@ export async function submitSolution(uri?: vscode.Uri): Promise<void> {

try {
const result: string = await leetCodeExecutor.submitSolution(filePath);
await leetCodeResultProvider.show(result);
await leetCodeSubmissionProvider.show(result);
} catch (error) {
await promptForOpenOutputChannel("Failed to submit the solution. Please open the output channel for details.", DialogType.error);
}
Expand Down
4 changes: 2 additions & 2 deletions src/commands/test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { isWindows, usingCmd } from "../utils/osUtils";
import { DialogType, promptForOpenOutputChannel, showFileSelectDialog } from "../utils/uiUtils";
import { getActiveFilePath } from "../utils/workspaceUtils";
import * as wsl from "../utils/wslUtils";
import { leetCodeResultProvider } from "../webview/leetCodeResultProvider";
import { leetCodeSubmissionProvider } from "../webview/leetCodeSubmissionProvider";

export async function testSolution(uri?: vscode.Uri): Promise<void> {
try {
Expand Down Expand Up @@ -81,7 +81,7 @@ export async function testSolution(uri?: vscode.Uri): Promise<void> {
if (!result) {
return;
}
await leetCodeResultProvider.show(result);
await leetCodeSubmissionProvider.show(result);
} catch (error) {
await promptForOpenOutputChannel("Failed to test the solution. Please open the output channel for details.", DialogType.error);
}
Expand Down
6 changes: 3 additions & 3 deletions src/extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import { leetCodeManager } from "./leetCodeManager";
import { leetCodeStatusBarController } from "./statusbar/leetCodeStatusBarController";
import { DialogType, promptForOpenOutputChannel } from "./utils/uiUtils";
import { leetCodePreviewProvider } from "./webview/leetCodePreviewProvider";
import { leetCodeResultProvider } from "./webview/leetCodeResultProvider";
import { leetCodeSolutionProvider } from "./webview/leetCodeSolutionProvider";
import { leetCodeSubmissionProvider } from "./webview/leetCodeSubmissionProvider";

export async function activate(context: vscode.ExtensionContext): Promise<void> {
try {
Expand All @@ -34,14 +34,14 @@ export async function activate(context: vscode.ExtensionContext): Promise<void>

const leetCodeTreeDataProvider: LeetCodeTreeDataProvider = new LeetCodeTreeDataProvider(context);
leetCodePreviewProvider.initialize(context);
leetCodeResultProvider.initialize(context);
leetCodeSolutionProvider.initialize(context);
leetCodeSubmissionProvider.initialize(context);

context.subscriptions.push(
leetCodeStatusBarController,
leetCodeChannel,
leetCodePreviewProvider,
leetCodeResultProvider,
leetCodeSubmissionProvider,
leetCodeSolutionProvider,
leetCodeExecutor,
vscode.window.createTreeView("leetCodeExplorer", { treeDataProvider: leetCodeTreeDataProvider, showCollapseAll: true }),
Expand Down
70 changes: 70 additions & 0 deletions src/webview/LeetCodeWebview.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
// Copyright (c) jdneo. All rights reserved.
// Licensed under the MIT license.

import { ConfigurationChangeEvent, Disposable, ExtensionContext, ViewColumn, WebviewPanel, window, workspace } from "vscode";
import { markdownEngine } from "./markdownEngine";

export abstract class LeetCodeWebview implements Disposable {

protected panel: WebviewPanel | undefined;
private context: ExtensionContext;
private listener: Disposable;

public initialize(context: ExtensionContext): void {
const { onDidChangeConfiguration } = this.getWebviewOption();
this.context = context;
if (onDidChangeConfiguration) {
this.listener = workspace.onDidChangeConfiguration(onDidChangeConfiguration, this);
} else {
this.listener = workspace.onDidChangeConfiguration((event: ConfigurationChangeEvent) => {
if (event.affectsConfiguration("markdown") && this.panel) {
this.panel.webview.html = this.getWebviewContent();
}
}, this);
}
}

public dispose(): void {
this.listener.dispose();
if (this.panel) {
this.panel.dispose();
}
}

protected showWebviewInternal(): this is { panel: WebviewPanel } {
Copy link
Member

Choose a reason for hiding this comment

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

Could you explain more about this method? The return type is this, but the actual returned value is true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

param is type is a user-defined type guard in typescript's grammar.

It reduced the scope of union type:

function isTypeDefined(obj: Type | undefined): obj is Type {
    return obj !== undefined;
}

let a: Type | undefined;
a.method() // ts error, a may be undefined

if (isTypeDefined(a)) {
    a.method(); // now a's type is only `Type`, `undefined` is reduced
}

this is type is a way to guard a class member's type:
microsoft/TypeScript#26212

this is { panel: WebviewPanel } makes sure this.panel is not undefined.

So the code here do not need to use this.panel!.method() to assert defined:

if (this.showWebviewInternal()) {
    this.panel.title = `${node.name}: Preview`; // now this.panel's undefined type is reduced
    this.panel.reveal(ViewColumn.One);
}

Copy link
Member

Choose a reason for hiding this comment

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

Then why we are not just calling showWebviewInternal() but put it in the if block.

I do not see any benefits to do this.

Copy link
Contributor Author

@Vigilans Vigilans Apr 15, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

You mean the code below?

this.showWebviewInternal();
this.panel!.title = `${node.name}: Preview`; // this.panel's undefined type is not reduced
this.panel!.reveal(ViewColumn.One);

Copy link
Member

Choose a reason for hiding this comment

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

The main problem here is, we should extract the fields in ILeetCodeWebviewOption as the member of the base class. The reason for doing that:

  1. Each web view has its own unique viewType
  2. Each web view has its own location to display - the viewColumn
  3. Each web view has the member title to determine the title of the editor.

So after the extraction, what you need to do is:

  1. Move all the logic about the panel update into showWebviewInternal()
  2. Update the title field when needed.
this.title = xxxx
this.showWebviewInternal();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've brought everything except sideMode in next PR to this PR. It includes the improvement on the webview base class, like mentioned above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you really think there is no relationship? Did you ever test yourself what is the visual effect when the title keeps A so long long long long long name: preview and A so long long long long long name: solution in Column 2? Will the user be able to easily change between the tabs?

Why do you pay more attention to something in the next PR which I haven't given illustration yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just want leetcode tabs in column 2 look like this way:
image

Otherwise, in my small-screen PC, a So long long long a name: Preview just occupies all the room in the Column 2.

Copy link
Member

Choose a reason for hiding this comment

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

Of cause I need to understand what you will do in the next PR since they have some relationship. This will determine whether we need to keep getWebviewOption() or make them as the class member.

The codes written in a good way should not only have good experience but also good at self-explain. This is very important to attract more community contribution. That's why I'm keeping asking you those questions. From the code itself, I cannot get your point. There's no GIF/picture to illustrate that. Then how could I know your point is that the tab is so long that make the experience bad?

Besides, From my understanding, the reason to rename to the title to Description is that you hope the size of the tab is not too big when VS Code has two columns. The real reason actually is having limited resolution, and multiple columns is one of the possible reason to cause the limited resolution.

Take the VS Code embedded Markdown extension as an example, You can see there is no optimization when the tab is too long.
image

Copy link
Member

Choose a reason for hiding this comment

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

I'm not saying that we should not do the optimation. It's acceptable to do it as long as the code logic is easy to understand.

if (!this.panel) {
const { viewType, title, viewColumn, onDidReceiveMessage } = this.getWebviewOption();

this.panel = window.createWebviewPanel(viewType, title, viewColumn || ViewColumn.One, {
enableScripts: true,
enableCommandUris: true,
enableFindWidget: true,
retainContextWhenHidden: true,
localResourceRoots: markdownEngine.localResourceRoots,
});

this.panel.onDidDispose(() => {
this.panel = undefined;
}, null, this.context.subscriptions);

if (onDidReceiveMessage) {
this.panel.webview.onDidReceiveMessage(onDidReceiveMessage, this, this.context.subscriptions);
}
}

this.panel.webview.html = this.getWebviewContent();
return true;
}

protected abstract getWebviewOption(): ILeetCodeWebviewOption;

protected abstract getWebviewContent(): string;
}

export interface ILeetCodeWebviewOption {
viewType: string;
title: string;
viewColumn?: ViewColumn;
onDidReceiveMessage?: (message: any) => Promise<void>;
onDidChangeConfiguration?: (event: ConfigurationChangeEvent) => Promise<void>;
}
115 changes: 48 additions & 67 deletions src/webview/leetCodePreviewProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,90 +1,44 @@
// Copyright (c) jdneo. All rights reserved.
// Licensed under the MIT license.

import { commands, Disposable, ExtensionContext, ViewColumn, WebviewPanel, window } from "vscode";
import { commands, ViewColumn } from "vscode";
import { leetCodeExecutor } from "../leetCodeExecutor";
import { IProblem } from "../shared";
import { ILeetCodeWebviewOption, LeetCodeWebview } from "./LeetCodeWebview";
import { markdownEngine } from "./markdownEngine";

class LeetCodePreviewProvider implements Disposable {
class LeetCodePreviewProvider extends LeetCodeWebview {

private context: ExtensionContext;
private node: IProblem;
private panel: WebviewPanel | undefined;

public initialize(context: ExtensionContext): void {
this.context = context;
}
private description: IDescription;

public async show(node: IProblem): Promise<void> {
// Fetch problem first before creating webview panel
const descString: string = await leetCodeExecutor.getDescription(node);

this.node = node;
if (!this.panel) {
this.panel = window.createWebviewPanel("leetcode.preview", "Preview Problem", ViewColumn.One, {
enableScripts: true,
enableCommandUris: true,
enableFindWidget: true,
retainContextWhenHidden: true,
localResourceRoots: markdownEngine.localResourceRoots,
});
this.description = this.parseDescription(descString, node);
if (this.showWebviewInternal()) {
this.panel.webview.html = this.getWebviewContent();
this.panel.title = `${node.name}: Preview`;
this.panel.reveal(ViewColumn.One);
}
}

this.panel.webview.onDidReceiveMessage(async (message: IWebViewMessage) => {
protected getWebviewOption(): ILeetCodeWebviewOption {
return {
viewType: "leetcode.preview",
title: "Preview Problem",
onDidReceiveMessage: async (message: IWebViewMessage): Promise<void> => {
switch (message.command) {
case "ShowProblem": {
await commands.executeCommand("leetcode.showProblem", this.node);
break;
}
}
}, this, this.context.subscriptions);

this.panel.onDidDispose(() => {
this.panel = undefined;
}, null, this.context.subscriptions);
}

const description: IDescription = this.parseDescription(descString, node);
this.panel.webview.html = this.getWebViewContent(description);
this.panel.title = `${node.name}: Preview`;
this.panel.reveal(ViewColumn.One);
}

public dispose(): void {
if (this.panel) {
this.panel.dispose();
}
}

private parseDescription(descString: string, problem: IProblem): IDescription {
const [
/* title */, ,
url, ,
/* tags */, ,
/* langs */, ,
category,
difficulty,
likes,
dislikes,
/* accepted */,
/* submissions */,
/* testcase */, ,
...body
] = descString.split("\n");
return {
title: problem.name,
url,
tags: problem.tags,
companies: problem.companies,
category: category.slice(2),
difficulty: difficulty.slice(2),
likes: likes.split(": ")[1].trim(),
dislikes: dislikes.split(": ")[1].trim(),
body: body.join("\n").replace(/<pre>\s*([^]+?)\s*<\/pre>/g, "<pre><code>$1</code></pre>"),
},
};
}

private getWebViewContent(desc: IDescription): string {
protected getWebviewContent(): string {
const mdStyles: string = markdownEngine.getStyles();
const buttonStyle: string = `
<style>
Expand All @@ -106,7 +60,7 @@ class LeetCodePreviewProvider implements Disposable {
}
</style>
`;
const { title, url, category, difficulty, likes, dislikes, body } = desc;
const { title, url, category, difficulty, likes, dislikes, body } = this.description;
const head: string = markdownEngine.render(`# [${title}](${url})`);
const info: string = markdownEngine.render([
`| Category | Difficulty | Likes | Dislikes |`,
Expand All @@ -117,7 +71,7 @@ class LeetCodePreviewProvider implements Disposable {
`<details>`,
`<summary><strong>Tags</strong></summary>`,
markdownEngine.render(
desc.tags
this.description.tags
.map((t: string) => `[\`${t}\`](https://leetcode.com/tag/${t})`)
.join(" | "),
),
Expand All @@ -127,7 +81,7 @@ class LeetCodePreviewProvider implements Disposable {
`<details>`,
`<summary><strong>Companies</strong></summary>`,
markdownEngine.render(
desc.companies
this.description.companies
.map((c: string) => `\`${c}\``)
.join(" | "),
),
Expand Down Expand Up @@ -159,6 +113,33 @@ class LeetCodePreviewProvider implements Disposable {
`;
}

private parseDescription(descString: string, problem: IProblem): IDescription {
const [
/* title */, ,
url, ,
/* tags */, ,
/* langs */, ,
category,
difficulty,
likes,
dislikes,
/* accepted */,
/* submissions */,
/* testcase */, ,
...body
] = descString.split("\n");
return {
title: problem.name,
url,
tags: problem.tags,
companies: problem.companies,
category: category.slice(2),
difficulty: difficulty.slice(2),
likes: likes.split(": ")[1].trim(),
dislikes: dislikes.split(": ")[1].trim(),
body: body.join("\n").replace(/<pre>\s*([^]+?)\s*<\/pre>/g, "<pre><code>$1</code></pre>"),
};
}
}

interface IDescription {
Expand Down
54 changes: 0 additions & 54 deletions src/webview/leetCodeResultProvider.ts
View file
Open in desktop

This file was deleted.

Loading