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.

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

fix: Fixed bug with new panels not always showing initial content #200

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

fix: Fixed bug with new panels not always showing initial content #200

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6c5f85a
Fixed bug with new panels not always showing initial content (DH-18347)
bmingles Jan 7, 2025
8888c3a
Comments (DH-18347)
bmingles Jan 7, 2025
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
35 changes: 27 additions & 8 deletions src/controllers/PanelController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,17 @@ export class PanelController extends ControllerBase {
await waitFor(0);

let lastPanel: vscode.WebviewPanel | null = null;
let lastFirstTimeActiveSubscription: vscode.Disposable | null = null;
let lastPanelIsNew = false;
let lastCreatedPanelFirstTimeActiveSubscription: vscode.Disposable | null =
null;

for (const variable of variables) {
const { id, title } = variable;
if (!this._panelService.hasPanel(serverUrl, id)) {
if (this._panelService.hasPanel(serverUrl, id)) {
lastPanelIsNew = false;
} else {
lastPanelIsNew = true;

const panel = vscode.window.createWebviewPanel(
'dhPanel', // Identifies the type of the webview. Used internally
title,
Expand All @@ -147,7 +153,8 @@ export class PanelController extends ControllerBase {
}
}
);
lastFirstTimeActiveSubscription = onFirstTimeActiveSubscription;
lastCreatedPanelFirstTimeActiveSubscription =
onFirstTimeActiveSubscription;

const onDidReceiveMessageSubscription =
panel.webview.onDidReceiveMessage(({ data }) => {
Expand All @@ -170,11 +177,23 @@ export class PanelController extends ControllerBase {
lastPanel = panel;
}

// Panels get created in an active state, so the last panel won't necessarily
// change from inactive to active. Remove the firstTimeActiveSubscription
// and refresh explicitly.
lastFirstTimeActiveSubscription?.dispose();
this._onRefreshPanelsContent(serverUrl, variables.slice(-1));
// Panels get refreshed as follows:
// 1. Existing panels - don't get updated here. These get refreshed by a
// `subscribeToFieldUpdates` handler in DhcService which issues a
// `REFRESH_VARIABLE_PANELS_CMD` command for variables matching existing
// panels.
// 2. Newly created panels that are not the initially active panel (aka. any
Copy link
Member

Choose a reason for hiding this comment

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

This logic is missing the case where the newly created panel is not the initially active panel but it is already visible:
image

This can happen with the example you provided.
Steps to Reproduce:

  1. Run the snippet creating t1 and t2
  2. Move both panels to a separate tab stack than the code
  3. Close t1
  4. Run the code snippet again

Expected Results
4. t2 should refresh in the location it's in, t1 should open up and load if visible

Actual Results
4. t2 becomes active but t1 shows up as a blank panel. Once selected it becomes visible.

I think there's a follow up here as well, since what I really expect is all the panels to open in the same stack. It's weird/annoying to have the panel open up in the same stack as the code, since now my Editor is blocked. I'd rather it would open all the panels in a new stack on the first run, and on subsequent runs open in the same panel if they already exist, or the same stack as one of the panels that already exists.

// new panel that is not the last panel in the list) - these get refreshed
// the first time the panel becomes active via a one-time
// `onDidChangeViewState` handler.
// 3. Newly created panel that is the initially active panel - this one won't
// call the `onDidChangeViewState` handler since it is initialized as
// active. For this case, we dispose the one-time `onDidChangeViewState`
// subscription and refresh the panel explicitly.
if (lastPanelIsNew && lastCreatedPanelFirstTimeActiveSubscription) {
lastCreatedPanelFirstTimeActiveSubscription.dispose();
this._onRefreshPanelsContent(serverUrl, variables.slice(-1));
}

lastPanel?.reveal();
};
Expand Down
Loading