Skip to content

Conversation

richiemcilroy
Copy link
Member

@richiemcilroy richiemcilroy commented Oct 17, 2025

Refines logic for reopening and focusing the main window when certain windows are closed, specifically on Windows, e.g. Settings or the Editor. Adds helper functions to check for open editor windows and to reopen the main window if needed.

Summary by CodeRabbit

  • Bug Fixes
    • Refined window management on Windows to maintain proper UI state when closing editor windows.
    • Improved main window restoration after editor window closure to ensure consistent application behavior.
    • Enhanced handling of settings and dialog windows during editor operations on Windows.

Refines logic for reopening and focusing the main window when certain windows are closed, specifically on Windows. Adds helper functions to check for open editor windows and to reopen the main window if needed.
Copy link
Contributor

coderabbitai bot commented Oct 17, 2025

Walkthrough

Windows-specific window management improvements to the Tauri desktop app. Introduces helper functions for detecting and reopening editor and main windows, then refactors the Editor window close logic to handle different window types (Settings, Upgrade, ModeSelect) with distinct behaviors and Windows-specific restoration logic.

Changes

Cohort / File(s) Summary
Windows Window Management Helpers & Editor Close Refactor
apps/desktop/src-tauri/src/lib.rs
Adds platform-specific has_open_editor_window() and reopen_main_window() helper functions. Refactors CapWindowId handling in Editor window close path: separates Settings logic from Upgrade/ModeSelect, adds selective window visibility toggling (TargetSelectOverlay, Main, Camera) based on label parsing, and integrates Windows-specific main/editor window restoration after Editor closes.

Sequence Diagram

sequenceDiagram
    participant User
    participant EditorWindow as Editor Window
    participant WindowManager
    participant MainWindow as Main Window
    participant SettingsWindow as Settings Window

    User->>EditorWindow: Closes window
    EditorWindow->>WindowManager: Close event (CapWindowId::Editor)
    
    alt Settings window active
        WindowManager->>WindowManager: Parse window label
        WindowManager->>SettingsWindow: Hide
        WindowManager->>MainWindow: Show
        WindowManager->>WindowManager: Check for open editor (Windows)
        alt No editor open (Windows)
            WindowManager->>MainWindow: Reopen main window
        end
    else Upgrade or ModeSelect active
        WindowManager->>MainWindow: Show relevant windows
        WindowManager->>WindowManager: Return early
    end
    
    WindowManager->>User: UI restored
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

The changes introduce platform-conditional logic with multiple control-flow branches, new helper functions, and interconnected window state management. While localized to one file, the refactoring of window lifecycle handling and Windows-specific restoration logic demands careful verification of edge cases and state transitions across different window types.

Poem

🐰 Windows dance, editor bows,
Main stage waits through nested rows,
Settings yield, helpers flow,
UI restored, smooth as snow! ✨🪟

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "feat: Improve window management on Windows platform" accurately captures the main objective of the changeset. The pull request introduces Windows-specific window management helpers (has_open_editor_window and reopen_main_window) and refines the logic for handling window close events on Windows, particularly for the Settings and Editor windows. The title is clear, specific to the Windows platform, and avoids vague terminology. A teammate scanning the repository history would immediately understand that this change addresses window management improvements specific to Windows.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch main-window-reopen

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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (4)
apps/desktop/src-tauri/src/lib.rs (4)

2389-2393: Avoid focus-steal when “frontstage” windows are present (Upgrade/ModeSelect/Signin).

On Windows, closing an Editor currently reopens/focuses Main whenever Settings isn’t open. This can pull focus away from Upgrade/ModeSelect (and potentially Signin) if they’re visible. Guard against those before reopening Main.

Apply:

-                                #[cfg(target_os = "windows")]
-                                if CapWindowId::Settings.get(&app).is_none() {
-                                    reopen_main_window(&app);
-                                }
+                                #[cfg(target_os = "windows")]
+                                {
+                                    let has_blocking =
+                                        CapWindowId::Upgrade.get(&app).is_some()
+                                        || CapWindowId::ModeSelect.get(&app).is_some()
+                                        // Signin isn't a CapWindowId; check by label.
+                                        || app.webview_windows().keys().any(|l| l == "signin");
+                                    if CapWindowId::Settings.get(&app).is_none() && !has_blocking {
+                                        reopen_main_window(&app);
+                                    }
+                                }

2395-2415: Centralize “restore core windows” logic and restore prior visibility state.

The Settings branch re-implements the same show loop used for Upgrade/ModeSelect and always shows Main/Camera/Target overlays. This duplicates logic and ignores a window’s prior visibility (e.g., a user‑hidden Camera window gets shown again).

  • Extract a helper (e.g., restore_core_windows(app)) to DRY this loop across branches.
  • Optionally track and restore previous visibility per window (store when hiding for Settings, restore only those). If deferring stateful restore, at least centralize the loop now to reduce drift.

Would you like a small helper implementation to dedupe these sites?


2535-2541: LGTM; tiny micro‑opt optional.

The editor detection is clear and correct. Optionally, prefilter with starts_with("editor-") before parsing to avoid constructing CapWindowId on non‑editor labels, though it’s negligible here.


2542-2557: Unminimize before focusing; optionally spawn show/focus to match Windows async pattern.

If Main exists but is minimized, set_focus may fail to bring it front. Unminimize first. Also consider spawning show/focus to mirror your “keep async on Windows” note elsewhere.

 #[cfg(target_os = "windows")]
 fn reopen_main_window(app: &AppHandle) {
     if let Some(main) = CapWindowId::Main.get(app) {
-        let _ = main.show();
-        let _ = main.set_focus();
+        // Ensure window is visible and not minimized before focusing.
+        let _ = main.show();
+        let _ = main.unminimize();
+        let _ = main.set_focus();
     } else {
-        let handle = app.clone();
-        tokio::spawn(async move {
+        let handle = app.clone();
+        tokio::spawn(async move {
             let _ = ShowCapWindow::Main {
                 init_target_mode: None,
             }
             .show(&handle)
             .await;
         });
     }
 }

If you prefer, replace tokio::spawn with tauri::async_runtime::spawn for consistency with Tauri’s runtime.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a0cc059 and b0dc95a.

📒 Files selected for processing (1)
  • apps/desktop/src-tauri/src/lib.rs (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Format Rust code using rustfmt and ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.

Files:

  • apps/desktop/src-tauri/src/lib.rs
🧬 Code graph analysis (1)
apps/desktop/src-tauri/src/lib.rs (3)
apps/desktop/src-tauri/src/windows.rs (8)
  • app (214-214)
  • app (366-366)
  • app (456-456)
  • app (771-771)
  • app (992-992)
  • label (117-119)
  • id (765-791)
  • from_str (56-89)
apps/desktop/src/utils/tauri.ts (2)
  • Camera (356-356)
  • ShowCapWindow (464-464)
apps/desktop/src-tauri/src/target_select_overlay.rs (1)
  • spawn (197-237)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
  • GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
  • GitHub Check: Analyze (rust)

@richiemcilroy richiemcilroy merged commit cc4da96 into main Oct 17, 2025
15 checks passed
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.

1 participant