From 24e3099b7b722f6753598d11202a01b3eb9a1835 Mon Sep 17 00:00:00 2001 From: Aleksander Date: Sun, 12 Apr 2026 12:59:29 +0200 Subject: [PATCH] dash-frontend: remove dangling popups immediately --- dash-frontend/src/frontend.rs | 13 +-- dash-frontend/src/util/popup_manager.rs | 83 ++++++++++--------- dash-frontend/src/views/app_launcher.rs | 2 +- dash-frontend/src/views/game_launcher.rs | 2 +- .../src/views/remote_skymap_downloader.rs | 2 +- dash-frontend/src/views/remote_skymap_list.rs | 2 +- 6 files changed, 48 insertions(+), 56 deletions(-) diff --git a/dash-frontend/src/frontend.rs b/dash-frontend/src/frontend.rs index 178deac3..192cbc06 100644 --- a/dash-frontend/src/frontend.rs +++ b/dash-frontend/src/frontend.rs @@ -28,7 +28,7 @@ use crate::{ assets, tab::{Tab, TabType, apps::TabApps, games::TabGames, home::TabHome, monado::TabMonado, settings::TabSettings}, util::{ - popup_manager::{MountPopupOnceParams, MountPopupParams, PopupManager, PopupManagerParams}, + popup_manager::{MountPopupOnceParams, PopupManager, PopupManagerParams}, toast_manager::ToastManager, }, views, @@ -101,7 +101,6 @@ pub enum FrontendTask { SetTab(TabType), RefreshClock, RefreshBackground, - MountPopup(MountPopupParams), MountPopupOnce(MountPopupOnceParams), RefreshPopupManager, ShowAudioSettings, @@ -305,15 +304,6 @@ impl Frontend { Ok(()) } - fn mount_popup(&mut self, params: MountPopupParams, data: &mut T) -> anyhow::Result<()> { - let config = self.interface.general_config(data); - - self - .popup_manager - .mount_popup(&self.globals, &mut self.layout, &self.tasks, params, config)?; - Ok(()) - } - fn mount_popup_once(&mut self, params: MountPopupOnceParams, data: &mut T) -> anyhow::Result<()> { let config = self.interface.general_config(data); @@ -357,7 +347,6 @@ impl Frontend { FrontendTask::SetTab(tab_type) => self.set_tab(params.data, tab_type)?, FrontendTask::RefreshClock => self.update_time(params.data)?, FrontendTask::RefreshBackground => self.update_background(params.data)?, - FrontendTask::MountPopup(popup_params) => self.mount_popup(popup_params, params.data)?, FrontendTask::MountPopupOnce(popup_params) => self.mount_popup_once(popup_params, params.data)?, FrontendTask::RefreshPopupManager => self.refresh_popup_manager()?, FrontendTask::ShowAudioSettings => self.action_show_audio_settings()?, diff --git a/dash-frontend/src/util/popup_manager.rs b/dash-frontend/src/util/popup_manager.rs index 0fe6c60f..97aa7f13 100644 --- a/dash-frontend/src/util/popup_manager.rs +++ b/dash-frontend/src/util/popup_manager.rs @@ -37,6 +37,7 @@ pub struct MountedPopup { #[derive(Default)] struct MountedPopupState { mounted_popup: Option, + closed_callback: Option, } #[derive(Default, Clone)] @@ -71,6 +72,13 @@ impl Default for PopupHolder { } } +impl PopupHolderState { + fn close(&mut self) { + self.view = None; + self.popup_handle.close(); + } +} + // we can't derive(Clone) due to the fact that ViewType is non-cloneable impl Clone for PopupHolder { fn clone(&self) -> Self { @@ -82,9 +90,7 @@ impl Clone for PopupHolder { impl PopupHolder { pub fn close(&self) { - let mut state = self.state.borrow_mut(); - state.view = None; - state.popup_handle.close(); + self.state.borrow_mut().close(); } pub fn set_view(&self, handle: PopupHandle, view: ViewType) { @@ -129,8 +135,12 @@ impl PopupHolder { where ViewType: 'static, { - let this = self.clone(); - Box::new(move || this.close()) + let weak_state = Rc::downgrade(&self.state); + Box::new(move || { + if let Some(state) = weak_state.upgrade() { + state.borrow_mut().close(); + } + }) } } @@ -152,22 +162,21 @@ pub struct PopupContentFuncData<'a> { pub id_content: WidgetID, } -#[derive(Clone)] -pub struct MountPopupParams { - pub title: Translation, - pub on_content: Rc anyhow::Result<()>>, -} +type PopupClosedCallback = Box; // we need to implement Clone here, but the underlying function can be called only once. // on_content will be cleared after the first call #[derive(Clone)] pub struct MountPopupOnceParams { title: Translation, - on_content: Rc anyhow::Result<()>>>>>, + on_content: Rc anyhow::Result>>>>, } impl MountPopupOnceParams { - pub fn new(title: Translation, on_content: Box anyhow::Result<()>>) -> Self { + pub fn new( + title: Translation, + on_content: Box anyhow::Result>, + ) -> Self { Self { title, on_content: Rc::new(RefCell::new(Some(on_content))), @@ -186,10 +195,16 @@ impl State { fn refresh_stack(&mut self, alterables: &mut EventAlterables) { // show only the topmost popup self.popup_stack.retain(|weak| { - let Some(popup) = weak.upgrade() else { - return false; + let retain = { + let Some(popup) = weak.upgrade() else { + return false; + }; + popup.borrow_mut().mounted_popup.is_some() }; - popup.borrow_mut().mounted_popup.is_some() + if !retain { + log::debug!("removing popup from popup_stack"); + } + retain }); for (idx, popup) in self.popup_stack.iter().enumerate() { @@ -257,6 +272,7 @@ impl PopupManager { let mounted_popup_state = MountedPopupState { mounted_popup: Some(mounted_popup), + closed_callback: None, }; let popup_handle = PopupHandle { @@ -264,13 +280,21 @@ impl PopupManager { }; let mut state = self.state.borrow_mut(); + log::debug!("pushing popup to popup_stack"); state.popup_stack.push(Rc::downgrade(&popup_handle.state)); but_back.on_click({ let popup_handle = Rc::downgrade(&popup_handle.state); Rc::new(move |_common, _evt| { if let Some(popup_handle) = popup_handle.upgrade() { - popup_handle.borrow_mut().mounted_popup = None; // will call Drop + if let Some(closed_callback) = { + let mut state = popup_handle.borrow_mut(); + state.mounted_popup = None; // will call Drop + state.closed_callback.take() + } { + log::debug!("closed_callback called"); + closed_callback(); + } } Ok(()) }) @@ -280,7 +304,7 @@ impl PopupManager { Ok((popup_handle, id_content)) } - /// Mount a new popup on top of the existing popup stack (non-cloneable version). + /// Mount a new popup on top of the existing popup stack. /// Only the topmost popup is visible. pub fn mount_popup_once( &mut self, @@ -298,35 +322,14 @@ impl PopupManager { let (popup_handle, id_content) = self.mount_popup_prepare(globals, layout, frontend_tasks, ¶ms.title)?; // mount user-set popup content - on_content_func(PopupContentFuncData { + let closed_callback = on_content_func(PopupContentFuncData { layout, handle: popup_handle.clone(), id_content, config, })?; - Ok(()) - } - - /// Mount a new popup on top of the existing popup stack. - /// Only the topmost popup is visible. - pub fn mount_popup( - &mut self, - globals: &WguiGlobals, - layout: &mut Layout, - frontend_tasks: &FrontendTasks, - params: MountPopupParams, - config: &GeneralConfig, - ) -> anyhow::Result<()> { - let (popup_handle, id_content) = self.mount_popup_prepare(globals, layout, frontend_tasks, ¶ms.title)?; - - // mount user-set popup content - (*params.on_content)(PopupContentFuncData { - layout, - handle: popup_handle.clone(), - id_content, - config, - })?; + popup_handle.state.borrow_mut().closed_callback = Some(closed_callback); Ok(()) } diff --git a/dash-frontend/src/views/app_launcher.rs b/dash-frontend/src/views/app_launcher.rs index b8179ce8..93d4fb68 100644 --- a/dash-frontend/src/views/app_launcher.rs +++ b/dash-frontend/src/views/app_launcher.rs @@ -441,7 +441,7 @@ pub fn mount_popup(frontend_tasks: FrontendTasks, globals: WguiGlobals, entry: D })?; popup.set_view(data.handle, view); - Ok(()) + Ok(popup.get_close_callback()) }), ))); } diff --git a/dash-frontend/src/views/game_launcher.rs b/dash-frontend/src/views/game_launcher.rs index 36581044..d70a8239 100644 --- a/dash-frontend/src/views/game_launcher.rs +++ b/dash-frontend/src/views/game_launcher.rs @@ -216,7 +216,7 @@ pub fn mount_popup( })?; popup.set_view(data.handle, view); - Ok(()) + Ok(popup.get_close_callback()) }), ))); } diff --git a/dash-frontend/src/views/remote_skymap_downloader.rs b/dash-frontend/src/views/remote_skymap_downloader.rs index c4dd89c0..66700b6b 100644 --- a/dash-frontend/src/views/remote_skymap_downloader.rs +++ b/dash-frontend/src/views/remote_skymap_downloader.rs @@ -187,7 +187,7 @@ pub fn mount_popup( })?; popup.set_view(data.handle, view); - Ok(()) + Ok(popup.get_close_callback()) }), ))); } diff --git a/dash-frontend/src/views/remote_skymap_list.rs b/dash-frontend/src/views/remote_skymap_list.rs index a6544760..7b7f8f59 100644 --- a/dash-frontend/src/views/remote_skymap_list.rs +++ b/dash-frontend/src/views/remote_skymap_list.rs @@ -237,7 +237,7 @@ pub fn mount_popup( })?; popup.set_view(data.handle, view); - Ok(()) + Ok(popup.get_close_callback()) }), ))); }