fix(desktop): avoid hidden mosaic tabs with persistence by andreasasprou · Pull Request #774 · superset-sh/superset

andreasasprou · 2026-01-16T07:26:45Z

Links

ExecPlan: apps/desktop/plans/20260114-2050-workspace-scoped-tabs-state.md

Summary

Stop stacking hidden Mosaic TabView trees when terminal persistence is enabled.
Refit terminals on tab activation to keep persisted sessions visible after remount.
Trigger Monaco layout on tab visibility changes for file/diff viewers.

Before:

Why / Context

Terminal persistence kept multiple tab UIs mounted with visibility: hidden. That stack of hidden Mosaic windows could corrupt toolbar/header state when switching tabs, leading to blank or partially rendered file viewer panes. Rendering only the active tab UI avoids hidden Mosaic state while keeping terminal persistence behavior intact.

How It Works

Remove the warm terminal tab rendering path so only the active TabView is mounted.
On tab show, force a terminal fit/resize/refresh cycle to resync xterm dimensions.
Propagate isTabVisible to file/diff viewers so Monaco layouts only run when visible.

Manual QA Checklist

Terminal Features

Switch between terminal tabs with persistence enabled (headers stay intact).
Switch between terminal tab and file viewer tab (file viewer toolbar renders).

File Operations

File viewer renders correctly after tab switching.

Manual QA validated in dev server by reporter.

Testing

bun run typecheck --filter=@superset/desktop
bun run lint
bun test apps/desktop/src/renderer/stores/tabs/utils.test.ts

Typecheck emits existing tsr generate route warnings (pre-existing).

Design Decisions

Avoid hidden Mosaic trees: keeping only the active tab UI mounted prevents toolbar corruption while allowing terminal sessions to persist via detach/reattach.

Follow-ups

The workspace-scoped tabs refactor (ExecPlan above) remains pending and should further reduce cross-tab state edge cases.

Risks / Rollout / Rollback

Risk: terminal reattach could regress performance/visual stability when switching tabs.
Rollout: normal desktop release.
Rollback: revert fix(desktop): avoid hidden mosaic tabs.

Summary by CodeRabbit

Bug Fixes
- Fixed potential editor rendering issues with layout adjustments on load.
Refactor
- Simplified terminal and tab component APIs by removing internal visibility prop usage.
- Removed terminal persistence logic, reverting to standard tab-switching behavior.
Tests
- Improved terminal session lifecycle test sequencing for better reliability.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Note

Eliminates hidden TabView stacks and related visibility plumbing, simplifying tab rendering and improving terminal/editor stability.

Simplifies TabsContent to render only the active tab; removes warm terminal persistence path and isTabVisible across TabView, TabPane, and Terminal
Updates Terminal to focus based on isFocused, drop visibility-based input gating, adjust attach priority, and streamline write/resize handling
Triggers Monaco layout in DiffViewer after mount/visibility to avoid stale sizing
Test: stabilizes session kill flow by waiting for session readiness and reusing a pre-created exit event promise

^{Written by Cursor Bugbot for commit a94428a. This will update automatically on new commits. Configure here.}

coderabbitai · 2026-01-16T07:26:57Z

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

📝 Walkthrough

Walkthrough

This PR removes the isTabVisible prop from the Terminal and related tab view components, simplifies tab rendering by eliminating terminal persistence features, adds a Monaco editor layout effect, and updates test synchronization timing for session lifecycle.

Changes

Cohort / File(s)	Summary
Tab visibility prop removal `apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/index.tsx`, `apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsx`, `apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx`, `apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/types.ts`	Removes `isTabVisible` prop from TabView, TabPane, and Terminal components; eliminates visibility tracking, focus gating, and attachment priority logic dependent on visibility state; Terminal now driven solely by focus state.
Terminal persistence removal `apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx`	Deletes terminal persistence logic including WARM_TERMINAL_TAB_LIMIT, visibility-based rendering branches, and mounted tab tracking; simplifies render path to only display active tab without per-workspace terminal visibility or warm lists.
Monaco layout handling `apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffViewer/DiffViewer.tsx`	Adds effect that schedules Monaco editor layout pass via `requestAnimationFrame` when editor mounts and Monaco becomes ready.
Test synchronization `apps/desktop/src/main/terminal-host/session-lifecycle.test.ts`	Introduces session readiness check and pre-capture of exit promise before session termination, aligning exit event handling with earlier readiness verification.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

mosaic fix #66 — Directly addresses visibility prop propagation for terminal tabs; this PR removes the isTabVisible prop that the related PR adds/alters
conditional render #152 — Replaces multi-tab terminal persistence rendering with simplified single-tab flow, aligning with TabsContent's removal of warm terminal logic
feat(desktop): add editable Monaco diff view for uncommitted changes #387 — Modifies DiffViewer.tsx to handle Monaco readiness, same file affected by the layout effect addition in this PR

Suggested reviewers

saddlepaddle
AviPeltz

Poem

🐰 Props removed with care and grace,
Tab visibility finds its place—
Focus reigns supreme and free,
Persistence fades, simplicity!
Monaco layouts dance with ease,
Sessions sync with newfound peace.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix(desktop): avoid hidden mosaic tabs with persistence' clearly summarizes the main change: removing hidden tab stacks to fix Mosaic state corruption issues while preserving terminal persistence.
Description check	✅ Passed	The description includes all required sections: summary of changes with before/after context, why/context explaining the problem, how it works with implementation details, manual QA checklist, testing commands, design decisions, follow-ups, and risks/rollout information.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Kitenite

Nice this removes the stuff I was concerned about in Terminal UI

…nce-tab-ui

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

This PR is being reviewed by Cursor Bugbot

Details

You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

cursor · 2026-01-17T01:46:43Z

+				modifiedEditor.layout();
+			}
+		});
+	}, [isTabVisible, isMonacoReady, isEditorMounted]);


DiffViewer layout calls wrong editor reference

Medium Severity

The new effect for triggering Monaco layout on tab visibility changes calls modifiedEditor.layout() instead of diffEditorRef.current?.layout(). For a diff editor, calling layout on just the modified (right-side) editor won't properly resize the original (left-side) editor or recalculate the split between panes. The diffEditorRef.current holds the IStandaloneDiffEditor which manages the entire split view and is the correct target for layout calls.

github-actions · 2026-01-17T03:50:08Z

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

⚠️ Neon database branch
⚠️ Electric Fly.io app

Thank you for your contribution! 🎉

fix(desktop): avoid hidden mosaic tabs

cc65477

test(desktop): stabilize terminal kill lifecycle

18a4626

Kitenite approved these changes Jan 17, 2026

View reviewed changes

Merge remote-tracking branch 'origin/main' into fix/terminal-persiste…

9c67d82

…nce-tab-ui

cursor Bot reviewed Jan 17, 2026

View reviewed changes

Kitenite added 2 commits January 16, 2026 19:25

Remove unneeded props

9d08803

Lint

a94428a

Kitenite merged commit 5b10748 into main Jan 17, 2026
5 of 6 checks passed

Kitenite deleted the fix/terminal-persistence-tab-ui branch January 17, 2026 03:49

coderabbitai Bot mentioned this pull request Jan 17, 2026

fix(desktop): improve terminal overlay UI styling #790

Merged

3 tasks

This was referenced Jan 31, 2026

Clean up Terminal api #1083

Merged

Terminal lifecycle refactor #1089

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(desktop): avoid hidden mosaic tabs with persistence#774

fix(desktop): avoid hidden mosaic tabs with persistence#774
Kitenite merged 5 commits intomainfrom
fix/terminal-persistence-tab-ui

andreasasprou commented Jan 16, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jan 16, 2026 •

edited

Loading

Other AI code review bot(s) detected

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

Kitenite left a comment •

edited

Loading

Uh oh!

cursor Bot left a comment

Uh oh!

cursor Bot Jan 17, 2026

Uh oh!

Uh oh!

github-actions Bot commented Jan 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

andreasasprou commented Jan 16, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why / Context

How It Works

Manual QA Checklist

Terminal Features

File Operations

Testing

Design Decisions

Follow-ups

Risks / Rollout / Rollback

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Other AI code review bot(s) detected

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

Kitenite left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

This PR is being reviewed by Cursor Bugbot

Uh oh!

cursor Bot Jan 17, 2026

Choose a reason for hiding this comment

DiffViewer layout calls wrong editor reference

Uh oh!

Uh oh!

github-actions Bot commented Jan 17, 2026

🧹 Preview Cleanup Complete

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

andreasasprou commented Jan 16, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jan 16, 2026 •

edited

Loading

Kitenite left a comment •

edited

Loading