Fixed remounting WidgetInstances

CHANGELOG.md

@@ -37,6 +37,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   measurements to ensure accurate rendering. Fixes [#158][158].
 - `Input` selection handling when dragging below or above the text field is now
   handled correctly.
+- Nested hierarchies of widgets stored in a reused `WidgetInstance` are now
+  properly unmounted and remounted. For widgets that store `MountedWidget`s, in
+  their `mounted` events the widgets should remount their children if needed.
+
+  This fix not only fixes underlying issues with how unmounting was occuring,
+  but also fixes `Stack`, `Grid`, and `WidgetRef` to automatically remount as
+  needed.
 
 [158]: https://github.com/khonsulabs/cushy/issues/158
 

src/context.rs

@@ -852,10 +852,10 @@ pub trait AsEventContext {
                 let Some(mut unmount_context) = context.for_other(&to_unmount) else {
                     continue;
                 };
+                let child = unmount_context.widget.widget().clone();
                 child.lock().as_widget().unmounted(&mut unmount_context);
                 unmount_context.widget.tree.remove_child(
-                    child,
+                    &child,
-                    &unmount_context.widget.current_node,
                     &mut unmount_context.widget.pending_state.unmount_queue,
                 );
             }

src/tree.rs

@@ -66,21 +66,9 @@ impl Tree {
         }
     }
 
-    pub fn remove_child(
+    pub fn remove_child(&self, child: &MountedWidget, children_to_unmount: &mut Vec<WidgetId>) {
-        &self,
-        child: &MountedWidget,
-        parent: &MountedWidget,
-        children_to_unmount: &mut Vec<WidgetId>,
-    ) {
         let mut data = self.data.lock();
-        data.remove_child(child.node_id, parent.node_id, children_to_unmount);
+        data.remove_child(child.node_id, children_to_unmount);
-
-        if child.widget.is_default() {
-            data.defaults.retain(|id| *id != child.node_id);
-        }
-        if child.widget.is_escape() {
-            data.escapes.retain(|id| *id != child.node_id);
-        }
     }
 
     pub(crate) fn set_layout(&self, widget: LotId, rect: Rect<Px>) {
@@ -285,6 +273,11 @@ impl Tree {
         data.widget_from_id(id, self)
     }
 
+    pub(crate) fn widget_is_valid(&self, id: LotId) -> bool {
+        let data = self.data.lock();
+        data.nodes.get(id).is_some()
+    }
+
     pub(crate) fn widget_from_node(&self, id: LotId) -> Option<MountedWidget> {
         let data = self.data.lock();
         data.widget_from_node(id, self)
@@ -390,7 +383,9 @@ impl Tree {
         id: LotId,
     ) -> (Styles, Option<Value<ThemePair>>, Option<Value<ThemeMode>>) {
         let data = self.data.lock();
-        let node = data.nodes.get(id).expect("missing widget");
+        let Some(node) = data.nodes.get(id) else {
+            return Default::default();
+        };
         (
             node.effective_styles.clone(),
             node.theme.clone(),
@@ -481,37 +476,42 @@ impl TreeData {
         }
     }
 
-    fn remove_child(
+    fn remove_child(&mut self, child: LotId, children_to_unmount: &mut Vec<WidgetId>) {
-        &mut self,
-        child: LotId,
-        parent: LotId,
-        children_to_unmount: &mut Vec<WidgetId>,
-    ) {
         let Some(removed_node) = self.nodes.remove(child) else {
             return;
         };
         self.nodes_by_id.remove(&removed_node.widget.id());
 
-        if let Some(parent) = self.nodes.get_mut(parent) {
+        if let Some(parent) = removed_node.parent {
-            let index = parent
+            if let Some(parent_node) = self.nodes.get_mut(parent) {
-                .children
+                if let Some(index) = parent_node
-                .iter()
+                    .children
-                .enumerate()
+                    .iter()
-                .find_map(|(index, c)| (*c == child).then_some(index))
+                    .enumerate()
-                .expect("child not found in parent");
+                    .find_map(|(index, c)| (*c == child).then_some(index))
-            parent.children.remove(index);
+                {
+                    parent_node.children.remove(index);
+                }
+            }
         }
 
         children_to_unmount.extend(
             removed_node
                 .children
                 .into_iter()
-                .filter_map(|id| self.nodes.get(id).map(|node| node.widget.id())),
+                .filter_map(|id| dbg!(self.nodes.get(id).map(|node| node.widget.id()))),
         );
 
         if let Some(next_focus) = removed_node.widget.next_focus() {
             self.previous_focuses.remove(&next_focus);
         }
+        self.render_info.order.retain(|info| info.node != child);
+        if removed_node.widget.is_default() {
+            self.defaults.retain(|id| *id != child);
+        }
+        if removed_node.widget.is_escape() {
+            self.escapes.retain(|id| *id != child);
+        }
     }
 
     pub(crate) fn widget_hierarchy(&self, mut widget: LotId, tree: &Tree) -> Vec<MountedWidget> {

src/widget.rs

@@ -306,6 +306,9 @@ pub trait Widget: Send + Debug + 'static {
     }
 
     /// The widget has been mounted into a parent widget.
+    ///
+    /// Widgets that contain [`MountedWidget`] references should call
+    /// [`MountedWidget::remount_if_needed`] in this function.
     #[allow(unused_variables)]
     fn mounted(&mut self, context: &mut EventContext<'_>) {}
 
@@ -1718,6 +1721,11 @@ where
 }
 
 /// A [`Widget`] that has been attached to a widget hierarchy.
+///
+/// Because [`WidgetInstance`]s can be reused, a mounted widget can be unmounted
+/// and eventually remounted. To ensure the widget is in a consistent state, all
+/// types that own `MountedWidget`s should call
+/// [`MountedWidget::remount_if_needed`] during their `mount()` functions.
 #[derive(Clone)]
 pub struct MountedWidget {
     pub(crate) node_id: LotId,
@@ -1736,6 +1744,22 @@ impl MountedWidget {
         self.tree.upgrade().expect("tree missing")
     }
 
+    /// Remounts this widget, if it was previously unmounted.
+    pub fn remount_if_needed(&mut self, context: &mut EventContext<'_>) {
+        if !self.is_mounted() {
+            *self = context.push_child(self.widget.clone());
+        }
+    }
+
+    /// Returns true if this widget is still mounted in a window.
+    #[must_use]
+    pub fn is_mounted(&self) -> bool {
+        let Some(tree) = self.tree.upgrade() else {
+            return false;
+        };
+        tree.widget_is_valid(self.node_id)
+    }
+
     /// Locks the widget for exclusive access. Locking widgets should only be
     /// done for brief moments of time when you are certain no deadlocks can
     /// occur due to other widget locks being held.
@@ -2414,6 +2438,9 @@ impl WidgetRef {
         let mut context = context.as_event_context();
         self.mounted
             .entry(&context)
+            .and_modify(|w| {
+                w.remount_if_needed(&mut context.as_event_context());
+            })
             .or_insert_with(|| context.push_child(self.instance.clone()))
     }
 

src/widgets/grid.rs

@@ -144,6 +144,14 @@ impl<const COLUMNS: usize> Widget for Grid<COLUMNS> {
         }
     }
 
+    fn mounted(&mut self, context: &mut EventContext<'_>) {
+        for row in &mut self.live_rows {
+            for col in row {
+                col.remount_if_needed(context);
+            }
+        }
+    }
+
     fn layout(
         &mut self,
         available_space: Size<ConstraintLimit>,

src/widgets/stack.rs

@@ -126,6 +126,12 @@ impl Widget for Stack {
         }
     }
 
+    fn mounted(&mut self, context: &mut EventContext<'_>) {
+        for child in &mut self.synced_children {
+            child.remount_if_needed(context);
+        }
+    }
+
     fn layout(
         &mut self,
         available_space: Size<ConstraintLimit>,

src/widgets/switcher.rs

@@ -57,7 +57,7 @@ impl WrapperWidget for Switcher {
         let current_source = self.source.get_tracking_invalidate(context);
         if &current_source != self.child.widget() {
             self.child.unmount_in(context);
-            self.child = WidgetRef::new(self.source.get());
+            self.child = WidgetRef::new(current_source);
         }
         context.invalidate_when_changed(&self.source);
         available_space