Skip to content

Commit

Permalink
Fixed the multiple window closing issue, where the two multiwindow ex…
Browse files Browse the repository at this point in the history
…amples weren't working. (#3499)

* fixed window when multiple windows not closing

* fixed window when multiple windows not closing

* fixed accidental includes

* polished

* fix last window closing hiding app and rc hardcount

* find the suspect strong count holders and fix them

* Use the proper type alias

* fmt

---------

Co-authored-by: Jonathan Kelley <[email protected]>
  • Loading branch information
sertschgi and jkelleyrtp authored Jan 13, 2025
1 parent 36522cf commit 25a64d9
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 19 deletions.
6 changes: 5 additions & 1 deletion examples/multiwindow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,13 @@
//! own context, root elements, etc.
use dioxus::prelude::*;
use dioxus::{desktop::Config, desktop::WindowCloseBehaviour};

fn main() {
dioxus::LaunchBuilder::desktop().launch(app);
dioxus::LaunchBuilder::desktop()
// We can choose the close behavior of the last window to hide. See WindowCloseBehaviour for more options.
.with_cfg(Config::new().with_close_behaviour(WindowCloseBehaviour::LastWindowHides))
.launch(app);
}

fn app() -> Element {
Expand Down
21 changes: 14 additions & 7 deletions packages/desktop/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use tao::{
dpi::PhysicalSize,
event::Event,
event_loop::{ControlFlow, EventLoop, EventLoopBuilder, EventLoopProxy, EventLoopWindowTarget},
window::WindowId,
window::{Window, WindowId},
};

/// The single top-level object that manages all the running windows, assets, shortcuts, etc
Expand Down Expand Up @@ -198,11 +198,14 @@ impl App {
}
}

LastWindowHides if self.webviews.len() > 1 => {
self.webviews.remove(&id);
}

LastWindowHides => {
let Some(webview) = self.webviews.get(&id) else {
return;
};
hide_app_window(&webview.desktop_context.webview);
if let Some(webview) = self.webviews.get(&id) {
hide_last_window(&webview.desktop_context.window);
}
}

CloseWindow => {
Expand Down Expand Up @@ -529,9 +532,13 @@ struct PreservedWindowState {
monitor: String,
}

/// Different hide implementations per platform
/// Hide the last window when using LastWindowHides.
///
/// On macOS, if we use `set_visibility(false)` on the window, it will hide the window but not show
/// it again when the user switches back to the app. `NSApplication::hide:` has the correct behaviour,
/// so we need to special case it.
#[allow(unused)]
pub fn hide_app_window(window: &wry::WebView) {
fn hide_last_window(window: &Window) {
#[cfg(target_os = "windows")]
{
use tao::platform::windows::WindowExtWindows;
Expand Down
7 changes: 6 additions & 1 deletion packages/desktop/src/desktop_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ pub fn window() -> DesktopContext {
/// A handle to the [`DesktopService`] that can be passed around.
pub type DesktopContext = Rc<DesktopService>;

/// A weak handle to the [`DesktopService`] to ensure safe passing.
/// The problem without this is that the tao window is never dropped and therefore cannot be closed.
/// This was due to the Rc that had still references because of multiple copies when creating a webview.
pub type WeakDesktopContext = Weak<DesktopService>;

/// An imperative interface to the current window.
///
/// To get a handle to the current window, use the [`use_window`] hook.
Expand Down Expand Up @@ -101,7 +106,7 @@ impl DesktopService {
/// You can use this to control other windows from the current window.
///
/// Be careful to not create a cycle of windows, or you might leak memory.
pub fn new_window(&self, dom: VirtualDom, cfg: Config) -> Weak<DesktopService> {
pub fn new_window(&self, dom: VirtualDom, cfg: Config) -> WeakDesktopContext {
let window = WebviewInstance::new(cfg, dom, self.shared.clone());

let cx = window.dom.in_runtime(|| {
Expand Down
18 changes: 13 additions & 5 deletions packages/desktop/src/document.rs
Original file line number Diff line number Diff line change
@@ -1,34 +1,42 @@
use crate::{query::Query, DesktopContext, WeakDesktopContext};
use dioxus_core::prelude::queue_effect;
use dioxus_document::{
create_element_in_head, Document, Eval, EvalError, Evaluator, LinkProps, MetaProps,
ScriptProps, StyleProps,
};
use generational_box::{AnyStorage, GenerationalBox, UnsyncStorage};

use crate::{query::Query, DesktopContext};
use generational_box::{AnyStorage, GenerationalBox, UnsyncStorage};

/// Code for the Dioxus channel used to communicate between the dioxus and javascript code
pub const NATIVE_EVAL_JS: &str = include_str!("./js/native_eval.js");

/// Represents the desktop-target's provider of evaluators.
#[derive(Clone)]
pub struct DesktopDocument {
pub(crate) desktop_ctx: DesktopContext,
pub(crate) desktop_ctx: WeakDesktopContext,
}

impl DesktopDocument {
pub fn new(desktop_ctx: DesktopContext) -> Self {
let desktop_ctx = std::rc::Rc::downgrade(&desktop_ctx);
Self { desktop_ctx }
}
}

impl Document for DesktopDocument {
fn eval(&self, js: String) -> Eval {
Eval::new(DesktopEvaluator::create(self.desktop_ctx.clone(), js))
Eval::new(DesktopEvaluator::create(
self.desktop_ctx
.upgrade()
.expect("Window to exist when document is alive"),
js,
))
}

fn set_title(&self, title: String) {
self.desktop_ctx.window.set_title(&title);
if let Some(ctx) = self.desktop_ctx.upgrade() {
ctx.set_title(&title);
}
}

/// Create a new meta tag in the head
Expand Down
2 changes: 1 addition & 1 deletion packages/desktop/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ pub mod trayicon;
// Public exports
pub use assets::AssetRequest;
pub use config::{Config, WindowCloseBehaviour};
pub use desktop_context::{window, DesktopContext, DesktopService};
pub use desktop_context::{window, DesktopContext, DesktopService, WeakDesktopContext};
pub use event_handlers::WryEventHandler;
pub use hooks::*;
pub use shortcut::{ShortcutHandle, ShortcutRegistryError};
Expand Down
10 changes: 6 additions & 4 deletions packages/desktop/src/webview.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use crate::document::DesktopDocument;
use crate::element::DesktopElement;
use crate::file_upload::DesktopFileDragEvent;
use crate::menubar::DioxusMenu;
Expand All @@ -12,6 +11,7 @@ use crate::{
waker::tao_waker,
Config, DesktopContext, DesktopService,
};
use crate::{document::DesktopDocument, WeakDesktopContext};
use base64::prelude::BASE64_STANDARD;
use dioxus_core::{Runtime, ScopeId, VirtualDom};
use dioxus_document::Document;
Expand All @@ -28,7 +28,7 @@ use wry::{DragDropEvent, RequestAsyncResponder, WebContext, WebViewBuilder};
pub(crate) struct WebviewEdits {
runtime: Rc<Runtime>,
pub wry_queue: WryQueue,
desktop_context: Rc<OnceCell<DesktopContext>>,
desktop_context: Rc<OnceCell<WeakDesktopContext>>,
}

impl WebviewEdits {
Expand All @@ -40,7 +40,7 @@ impl WebviewEdits {
}
}

fn set_desktop_context(&self, context: DesktopContext) {
fn set_desktop_context(&self, context: WeakDesktopContext) {
_ = self.desktop_context.set(context);
}

Expand Down Expand Up @@ -115,6 +115,8 @@ impl WebviewEdits {
return Default::default();
};

let desktop_context = desktop_context.upgrade().unwrap();

let query = desktop_context.query.clone();
let recent_file = desktop_context.file_hover.clone();

Expand Down Expand Up @@ -412,7 +414,7 @@ impl WebviewInstance {
));

// Provide the desktop context to the virtual dom and edit handler
edits.set_desktop_context(desktop_context.clone());
edits.set_desktop_context(Rc::downgrade(&desktop_context));
let provider: Rc<dyn Document> = Rc::new(DesktopDocument::new(desktop_context.clone()));
let history_provider: Rc<dyn History> = Rc::new(MemoryHistory::default());
dom.in_runtime(|| {
Expand Down

0 comments on commit 25a64d9

Please sign in to comment.