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

Multiple Webview Tabs #2093

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Multiple Webview Tabs #2093

wants to merge 7 commits into from

Conversation

ScottCarda-MS
Copy link
Contributor

@ScottCarda-MS ScottCarda-MS commented Jan 7, 2025

Instead of always consuming the last Q# webview tab, open a new webview tab when activated with a new operation. Activation with the same operation will still consume/update the last webview for that operation. Works on circuit and histogram panels.

@ScottCarda-MS ScottCarda-MS marked this pull request as ready for review January 8, 2025 00:48
Copy link
Member

@minestarks minestarks left a comment

Choose a reason for hiding this comment

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

I like this change.

(Next time we touch webviewPanel.ts we really should refactor out the different panel types.)

@@ -332,6 +332,7 @@ export function updateCircuitPanel(
calculating?: boolean;
},
) {
const panelId = params?.operation?.operation || projectName;
Copy link
Member

Choose a reason for hiding this comment

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

In the rare case where two operations from two different programs have the same name, the panel IDs won't be unique. But I'm okay with that. The panel titles look identical to the user anyway, so if they really want to differentiate the panels, they can use different operation names.

@@ -14,7 +14,7 @@ export async function showDocumentationCommand(extensionUri: Uri) {

// Reveal panel and show 'Loading...' for immediate feedback.
sendMessageToPanel(
"documentation", // This is needed to route the message to the proper panel
{ panelType: "documentation", id: "" }, // This is needed to route the message to the proper panel
Copy link
Member

Choose a reason for hiding this comment

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

Though "" technically works, I dislike the use of empty string to mean "none". Making id an optional field and simply not passing it in here would make the semantics a bit clearer I think.

? "Q# Documentation"
: type == "estimates"
? "Q# Estimates"
: `${id} ${panelTypeToTitle[type]}`;
Copy link
Member

Choose a reason for hiding this comment

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

I think a title like Q# Histogram - NameOfFile.qs would look more consistent with the other tabs.

Screenshot 2025-01-24 134235

props: CircuitProps;
};

type DocumentationState = {
viewType: "documentation";
panelId: string;
Copy link
Member

Choose a reason for hiding this comment

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

Do you really need to pass in panelId for Documentation and Estimates, since they don't actually have distinct panel IDs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants