Skip to content

update emain-window to do repositioning in a simpler way #3087

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
sawka merged 2 commits into from
Mar 20, 2026
Merged

update emain-window to do repositioning in a simpler way #3087

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f250fb9
update emain-window to do repositioning in a simpler way (the old way…
sawka Mar 19, 2026
408f52b
fix nits
sawka Mar 20, 2026
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
2 changes: 1 addition & 1 deletion electron.vite.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ export default defineConfig({
"**/*.md",
"**/*.mdx",
"**/*.json",
"emain/**",
"**/emain/**",
"**/*.txt",
"**/*.log",
],
Expand Down
55 changes: 13 additions & 42 deletions emain/emain-window.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ export function calculateWindowBounds(
): { x: number; y: number; width: number; height: number } {
let winWidth = winSize?.width;
let winHeight = winSize?.height;
let winPosX = pos?.x ?? 100;
let winPosY = pos?.y ?? 100;
const winPosX = pos?.x ?? 100;
const winPosY = pos?.y ?? 100;

if (
(winWidth == null || winWidth === 0 || winHeight == null || winHeight === 0) &&
Expand Down Expand Up @@ -86,7 +86,7 @@ export function calculateWindowBounds(
winWidth = Math.max(winWidth, MinWindowWidth);
winHeight = Math.max(winHeight, MinWindowHeight);

let winBounds = {
const winBounds = {
x: winPosX,
y: winPosY,
width: winWidth,
Expand Down Expand Up @@ -393,6 +393,8 @@ export class WaveBrowserWindow extends BaseWindow {
private async initializeTab(tabView: WaveTabView, primaryStartupTab: boolean) {
const clientId = await getClientId();
await this.awaitWithDevTimeout(tabView.initPromise, "initPromise", tabView.waveTabId);
const winBounds = this.getContentBounds();
tabView.setBounds({ x: 0, y: 0, width: winBounds.width, height: winBounds.height });
this.contentView.addChildView(tabView);
Comment on lines +396 to 398
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 20, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Check file size and structure
wc -l emain/emain-window.ts

Repository: wavetermdev/waveterm

Length of output: 89

🏁 Script executed:

# Read the relevant section around lines 396-398 with context
sed -n '380,420p' emain/emain-window.ts

Repository: wavetermdev/waveterm

Length of output: 1801

🏁 Script executed:

# Look for the initializeTab method definition
rg -n "initializeTab" emain/emain-window.ts -B 2 -A 15

Repository: wavetermdev/waveterm

Length of output: 1805

🏁 Script executed:

# Search for waveReadyPromise usage
rg -n "waveReadyPromise" emain/emain-window.ts -B 2 -A 2

Repository: wavetermdev/waveterm

Length of output: 338

🏁 Script executed:

# Look for finalizePositioning method
rg -n "finalizePositioning" emain/emain-window.ts -B 2 -A 10

Repository: wavetermdev/waveterm

Length of output: 1898

🏁 Script executed:

# Find the WaveTabView class and understand positionTabOnScreen/positionTabOffScreen
rg -n "positionTabOnScreen|positionTabOffScreen" emain/emain-window.ts -B 2 -A 5

Repository: wavetermdev/waveterm

Length of output: 1568

🏁 Script executed:

# Check what happens before and after initializeTab is called - look at setTabViewIntoWindow
rg -n "setTabViewIntoWindow" emain/emain-window.ts -B 5 -A 30

Repository: wavetermdev/waveterm

Length of output: 3304

🏁 Script executed:

# Search for where activeTabView is set to understand which tab becomes active
rg -n "activeTabView\s*=" emain/emain-window.ts -B 2 -A 2

Repository: wavetermdev/waveterm

Length of output: 971

🏁 Script executed:

# Look for any other view positioning or visibility logic around initialization
rg -n "addChildView|removeChildView|setVisible" emain/emain-window.ts -B 1 -A 1

Repository: wavetermdev/waveterm

Length of output: 506

🏁 Script executed:

# Search for the WaveTabView class definition and positionTabOnScreen implementation
rg -n "class WaveTabView" emain/ -A 30

Repository: wavetermdev/waveterm

Length of output: 2124

🏁 Script executed:

# Look for positionTabOnScreen and positionTabOffScreen in the codebase
rg -n "positionTabOnScreen\s*(" emain/ -A 10

Repository: wavetermdev/waveterm

Length of output: 160

🏁 Script executed:

# Check if there's any initialization of the WaveTabView that might affect visibility
rg -n "new WaveTabView" emain/ -B 5 -A 10

Repository: wavetermdev/waveterm

Length of output: 1277

🏁 Script executed:

# Check what getOrCreateWebViewForTab does and if it initializes the view
rg -n "getOrCreateWebViewForTab" emain/ -A 20

Repository: wavetermdev/waveterm

Length of output: 4365

🏁 Script executed:

# Search for positionTabOnScreen and positionTabOffScreen definitions
rg -n "positionTabOnScreen|positionTabOffScreen" emain/emain-tabview.ts -A 15

Repository: wavetermdev/waveterm

Length of output: 981

🏁 Script executed:

# Check if there's any visibility or z-order related code in WaveTabView
rg -n "setVisible|zIndex|isVisible" emain/emain-tabview.ts -B 2 -A 5

Repository: wavetermdev/waveterm

Length of output: 46

🏁 Script executed:

# Look for any initialization that might set the view's initial position
rg -n "setBounds" emain/emain-tabview.ts -B 3 -A 3

Repository: wavetermdev/waveterm

Length of output: 402

🏁 Script executed:

# Check if there's any logic that positions the old tab off-screen BEFORE adding the new tab
rg -n "positionTabOffScreen|removeChildView" emain/emain-window.ts -B 3 -A 2

Repository: wavetermdev/waveterm

Length of output: 755

🏁 Script executed:

# Look for any code that might position tabs off-screen when they lose focus or become inactive
rg -n "isActiveTab\s*=" emain/emain-window.ts -B 5 -A 5

Repository: wavetermdev/waveterm

Length of output: 715

🏁 Script executed:

# Check the full context of where finalizePositioning is called relative to setTabViewIntoWindow
sed -n '448,475p' emain/emain-window.ts

Repository: wavetermdev/waveterm

Length of output: 1349

🏁 Script executed:

# Final verification: Check if there are any other similar patterns or related code
rg -n "addChildView" emain/emain-window.ts -B 5 -A 2

Repository: wavetermdev/waveterm

Length of output: 591

🏁 Script executed:

# Verify if positionTabOffScreen is the intended pattern
rg -n "positionTabOffScreen\(" emain/emain-window.ts

Repository: wavetermdev/waveterm

Length of output: 121

Attach the new WaveTabView off-screen until initialization completes.

Lines 397–398 position the tab at full window bounds before waveReadyPromise resolves. Since addChildView appends to the child list and Electron uses child order for z-layer ordering, this uninitialized tab becomes topmost and obscures the current tab during slow initialization or workspace switches. Move it off-screen here and let finalizePositioning() bring it on-screen only after init succeeds.

💡 Suggested change 
         const winBounds = this.getContentBounds();
-        tabView.setBounds({ x: 0, y: 0, width: winBounds.width, height: winBounds.height });
+        tabView.setBounds({ x: -15000, y: -15000, width: winBounds.width, height: winBounds.height });
         this.contentView.addChildView(tabView);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const winBounds = this.getContentBounds();
tabView.setBounds({ x: 0, y: 0, width: winBounds.width, height: winBounds.height });
this.contentView.addChildView(tabView);
const winBounds = this.getContentBounds();
tabView.setBounds({ x: -15000, y: -15000, width: winBounds.width, height: winBounds.height });
this.contentView.addChildView(tabView);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@emain/emain-window.ts` around lines 396 - 398, The new WaveTabView (tabView)
is being added at full window bounds before initialization finishes, making it
topmost and obscuring the current tab; set the tab off-screen before adding it
and only let it move on-screen after initialization completes. Specifically,
call tabView.setBounds(...) with off-screen coordinates (e.g., x: -9999, y: 0,
width: winBounds.width, height: winBounds.height or similar off-screen x) before
this.contentView.addChildView(tabView), then wait for waveReadyPromise and call
tabView.finalizePositioning() (or the existing positioning helper) to set the
real on-screen bounds and z-order once init succeeds; use getContentBounds(),
tabView.setBounds, this.contentView.addChildView, waveReadyPromise, and
tabView.finalizePositioning() to implement this change.

const initOpts: WaveInitOpts = {
tabId: tabView.waveTabId,
Expand All @@ -406,7 +408,7 @@ export class WaveBrowserWindow extends BaseWindow {
tabView.savedInitOpts = { ...initOpts };
tabView.savedInitOpts.activate = false;
delete tabView.savedInitOpts.primaryTabStartup;
let startTime = Date.now();
const startTime = Date.now();
console.log(
"before wave ready, init tab, sending wave-init",
tabView.waveTabId,
Expand Down Expand Up @@ -456,14 +458,12 @@ export class WaveBrowserWindow extends BaseWindow {
this.allLoadedTabViews.set(tabView.waveTabId, tabView);
if (!tabInitialized) {
console.log("initializing a new tab", primaryStartupTab ? "(primary startup)" : "");
const p1 = this.initializeTab(tabView, primaryStartupTab);
const p2 = this.repositionTabsSlowly(100);
await Promise.all([p1, p2]);
await this.initializeTab(tabView, primaryStartupTab);
this.finalizePositioning();
} else {
console.log("reusing an existing tab, calling wave-init", tabView.waveTabId);
const p1 = this.repositionTabsSlowly(35);
const p2 = tabView.webContents.send("wave-init", tabView.savedInitOpts); // reinit
await Promise.all([p1, p2]);
tabView.webContents.send("wave-init", tabView.savedInitOpts); // reinit
this.finalizePositioning();
}

// something is causing the new tab to lose focus so it requires manual refocusing
Expand All @@ -480,35 +480,6 @@ export class WaveBrowserWindow extends BaseWindow {
}, 30);
}

private async repositionTabsSlowly(delayMs: number) {
const activeTabView = this.activeTabView;
const winBounds = this.getContentBounds();
if (activeTabView == null) {
return;
}
if (activeTabView.isOnScreen()) {
activeTabView.setBounds({
x: 0,
y: 0,
width: winBounds.width,
height: winBounds.height,
});
} else {
activeTabView.setBounds({
x: winBounds.width - 10,
y: winBounds.height - 10,
width: winBounds.width,
height: winBounds.height,
});
}
await delay(delayMs);
if (this.activeTabView != activeTabView) {
// another tab view has been set, do not finalize this layout
return;
}
this.finalizePositioning();
}

private finalizePositioning() {
if (this.isDestroyed()) {
return;
Expand Down Expand Up @@ -546,7 +517,7 @@ export class WaveBrowserWindow extends BaseWindow {
private removeTabViewLater(tabId: string, delayMs: number) {
setTimeout(() => {
this.removeTabView(tabId, false);
}, 1000);
}, delayMs);
}

// the queue and this function are used to serialize operations that update the window contents view
Expand Down Expand Up @@ -744,7 +715,7 @@ ipcMain.on("set-active-tab", async (event, tabId) => {
await ww?.setActiveTab(tabId, true);
});

ipcMain.on("create-tab", async (event, opts) => {
ipcMain.on("create-tab", async (event, _opts) => {
const senderWc = event.sender;
const ww = getWaveWindowByWebContentsId(senderWc.id);
if (ww != null) {
Expand Down Expand Up @@ -818,7 +789,7 @@ ipcMain.on("delete-workspace", (event, workspaceId) => {

const workspaceList = await WorkspaceService.ListWorkspaces();

const workspaceHasWindow = !!workspaceList.find((wse) => wse.workspaceid === workspaceId)?.windowid;
const _workspaceHasWindow = !!workspaceList.find((wse) => wse.workspaceid === workspaceId)?.windowid;
Copy link
Contributor

@kilo-code-bot kilo-code-bot bot Mar 19, 2026

Choose a reason for hiding this comment

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

WARNING: Unused variable - _workspaceHasWindow is computed but never used in the function. The variable was previously named workspaceHasWindow (without underscore prefix), suggesting it may have had usage that was accidentally removed. Either remove this variable or restore its intended usage.


const choice = dialog.showMessageBoxSync(this, {
type: "question",
Expand Down
Loading