feat: add query panel tab #1231

gaurpulkit · 2024-06-19T12:49:01Z

Overview

Problem

#683 (comment)

Solution

Showing query panel in tabs

Screenshot/Demo

A picture is worth a thousand words. Please highlight the changes if applicable.

How to test

Steps to be followed to verify the solution or code changes
Mention if there is any settings configuration added/changed/deleted

Checklist

I have run this code and it appears to resolve the stated issue
README.md updated and added information about my change

mdesmet

What happens if vscode is restarted?
I don't think we should allow to clear the results of result document.

mdesmet

LGTM I would not name it tab but VirtualDocument as in https://github.com/microsoft/vscode-extension-samples/blob/main/virtual-document-sample/README.md

mdesmet · 2024-06-19T15:14:12Z

src/webview_provider/QueryResultTab.ts

+      usersService,
+    );
+
+    const t = this;


What's this for?

mdesmet · 2024-06-19T15:22:47Z

webview_panels/src/modules/queryPanel/QueryPanel.tsx

@@ -30,8 +31,9 @@ const QueryPanel = (): JSX.Element => {
          />
        </Stack>
        <Stack>
-          <ShowOldUxButton />
+          {!isTab && <ShowOldUxButton />}


Suggested change

{!isTab && <ShowOldUxButton />}

{isPanel && <ShowOldUxButton />}

saravmajestic · 2024-06-20T02:23:12Z

src/webview_provider/newDocsGenPanel.ts

@@ -29,6 +29,7 @@ import { UsersService } from "../services/usersService";
 import { ConversationProvider } from "../comment_provider/conversationProvider";
 import { DbtDocsView } from "./DbtDocsView";
 import { ConversationService } from "../services/conversationService";
+import { QueryResultTab } from "./QueryResultTab";


this can be part of queryResultPanel.ts. I have added DbtDocsView, because it was related to doc editor

saravmajestic · 2024-06-20T02:24:06Z

webview_panels/src/modules/queryPanel/QueryPanel.tsx

@@ -30,8 +31,9 @@ const QueryPanel = (): JSX.Element => {
          />
        </Stack>
        <Stack>
-          <ShowOldUxButton />
+          {!isTab && <ShowOldUxButton />}
          <ClearResultsButton />


clear results is also not needed in tab

saravmajestic · 2024-06-20T02:25:34Z

webview_panels/src/modules/queryPanel/useQueryPanelListeners.ts

+      void executeRequestInSync("getQueryTabData", {}).then((data) => {
+        if (data) {
+          const typedData = data as QueryPanelStateProps;
+          setTabData(data as Record<string, unknown>);


why is this typecasted to unknown? can you use QueryPanelStateProps?

saravmajestic · 2024-06-20T02:26:58Z

webview_panels/src/modules/queryPanel/useQueryPanelListeners.ts

@@ -154,7 +158,24 @@ const useQueryPanelListeners = (): { loading: boolean } => {
    };
  }, [onMesssage]);

-  return { loading };
+  useEffect(() => {
+    if (!tabData) {


Looks like you can avoid storing all the data in tabData. This is only needed to store whether this is a tab or not. You can just store this state value as boolean. This will avoid storing duplicate data in memory

saravmajestic · 2024-06-20T02:29:13Z

src/webview_provider/QueryResultTab.ts

+import { UsersService } from "../services/usersService";
+
+@provideSingleton(QueryResultTab)
+export class QueryResultTab extends AltimateWebviewProvider {


this new class can be avoided and can be combined with existing queryResultsPanel.

anandgupta42 · 2024-06-20T03:28:18Z

@gaurpulkit make sure we add some tests for this as well. Also, please make sure to test it out manually in both native desktop environment and also in codespaces

…er-user into add-query-panel-tab

gaurpulkit · 2024-06-20T03:59:00Z

What happens if vscode is restarted? I don't think we should allow to clear the results of result document.

It clears the tabs for now

…scode-dbt-power-user into add-query-panel-tab

saravmajestic · 2024-06-24T02:08:38Z

src/webview_provider/queryResultPanel.ts


  protected _panel: WebviewView | undefined;
+  protected _tabPanel: WebviewPanel | undefined = undefined;


this can be private

saravmajestic · 2024-06-24T02:11:13Z

src/webview_provider/queryResultPanel.ts

+      return;
+    }
+
+    this._webview.onDidReceiveMessage(this.handleCommand, this, []);


use setupWebviewHooks

saravmajestic · 2024-06-24T02:12:21Z

webview_panels/src/uiCore/components/gradientButton/GradientButton.tsx

+import Tooltip from "../tooltip/Tooltip";
+import classes from "./styles.module.scss";
+
+export const GradientButton = (props: ButtonProps): JSX.Element => (


add a sample in webview_panels\src\uiCore\uiToolkitStories\Components.stories.tsx

saravmajestic · 2024-06-24T02:14:17Z

webview_panels/src/modules/queryPanel/QueryPanel.tsx

+          {isPanel && <ShowOldUxButton />}
+          {isPanel && <ClearResultsButton />}
+          {isPanel && <ShowInTabButton />}
          <HelpButton />


Suggested change

{isPanel && <ShowOldUxButton />}

{isPanel && <ClearResultsButton />}

{isPanel && <ShowInTabButton />}

<HelpButton />

{isPanel && (

<>

<ShowOldUxButton />

<ClearResultsButton />

<ShowInTabButton />

</>

)}

<HelpButton />

webview_panels/src/modules/queryPanel/useQueryPanelListeners.ts

saravmajestic · 2024-06-25T02:55:49Z

src/webview_provider/queryResultPanel.ts

@@ -312,6 +354,32 @@ export class QueryResultPanel extends AltimateWebviewProvider {
    this._panel!.onDidChangeVisibility(sendQueryPanelViewEvent);
  }

+  private setupTabWebviewHooks() {
+    this._tabPanel!.webview.onDidReceiveMessage(


it will be better to reuse the existing onDidReceiveMessage handler, otherwise this tab will lose the other command handling. You should be using existing setupWebviewHooks so that all the commands will go through same flow. Otherwise it will be difficult to maintain multiple versions of the query results panel

I already tried to do that but the webview of tab isn't available when only panel opens. So, we need to set it when it opens.

saravmajestic · 2024-06-25T02:57:18Z

src/webview_provider/queryResultPanel.ts

+
+    this.setupTabWebviewHooks();
+
+    this._webview.html = this.getHtml(


It is better to use renderWebviewView, which has few conditions to render different type of views based on configuration.

Again, the setup time of tab and panel will be different

saravmajestic · 2024-06-25T02:58:07Z

src/webview_provider/queryResultPanel.ts

+      this,
+      this._disposables,
+    );
+    const sendQueryTabViewEvent = () => {


extract this as separate private function to avoid multiple bindings

add query panel tab

b725c92

gaurpulkit requested review from saravmajestic and mdesmet June 19, 2024 12:49

mdesmet reviewed Jun 19, 2024

View reviewed changes

saravmajestic requested changes Jun 20, 2024

View reviewed changes

Merge branch 'master' of https://github.com/AltimateAI/vscode-dbt-pow…

e832a29

…er-user into add-query-panel-tab

anandgupta42 and others added 3 commits June 20, 2024 06:16

Merge branch 'master' into add-query-panel-tab

6458430

changes

202fd5d

Merge branch 'add-query-panel-tab' of https://github.com/AltimateAI/v…

576b339

…scode-dbt-power-user into add-query-panel-tab

gaurpulkit requested a review from saravmajestic June 20, 2024 07:44

gaurpulkit and others added 3 commits June 21, 2024 13:42

add elapsedTime in tab

484481e

Add gradient button

b4222c9

Merge branch 'master' into add-query-panel-tab

a15627e

saravmajestic requested changes Jun 24, 2024

View reviewed changes

changes

8057747

saravmajestic requested changes Jun 25, 2024

View reviewed changes

gaurpulkit and others added 2 commits June 25, 2024 09:13

changes

1506647

fix: for review comments

22d55e1

saravmajestic approved these changes Jun 25, 2024

View reviewed changes

mdesmet merged commit 8cc6e7b into master Jun 25, 2024
7 checks passed

mdesmet deleted the add-query-panel-tab branch June 25, 2024 09:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add query panel tab #1231

feat: add query panel tab #1231

gaurpulkit commented Jun 19, 2024

mdesmet left a comment

mdesmet left a comment

mdesmet Jun 19, 2024

mdesmet Jun 19, 2024

saravmajestic Jun 20, 2024

saravmajestic Jun 20, 2024

saravmajestic Jun 20, 2024

saravmajestic Jun 20, 2024

saravmajestic Jun 20, 2024

anandgupta42 commented Jun 20, 2024

gaurpulkit commented Jun 20, 2024

saravmajestic Jun 24, 2024

saravmajestic Jun 24, 2024

saravmajestic Jun 24, 2024

saravmajestic Jun 24, 2024

saravmajestic Jun 25, 2024

gaurpulkit Jun 25, 2024

saravmajestic Jun 25, 2024

gaurpulkit Jun 25, 2024

saravmajestic Jun 25, 2024

	{!isTab && <ShowOldUxButton />}
	{isPanel && <ShowOldUxButton />}


		protected _panel: WebviewView \| undefined;
		protected _tabPanel: WebviewPanel \| undefined = undefined;


		this.setupTabWebviewHooks();

		this._webview.html = this.getHtml(

feat: add query panel tab #1231

feat: add query panel tab #1231

Conversation

gaurpulkit commented Jun 19, 2024

Overview

Problem

Solution

Screenshot/Demo

How to test

Checklist

mdesmet left a comment

Choose a reason for hiding this comment

mdesmet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anandgupta42 commented Jun 20, 2024

gaurpulkit commented Jun 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment