Fixes DialogHost visibility on VM disconnect (#3069) (#3092)

* Fixes DialogHost visibility on VM disconnect (#3069) * Adding UI test Reverted change to make sure that we remove dead weak references. --------- Co-authored-by: Kevin Bost <[email protected]>
MaterialDesignInXAML · Feb 20, 2023 · ef7214b · ef7214b · peijiehuang · Feb 20, 2023
1 parent 557bb1b
commit ef7214b
Show file tree

Hide file tree

Showing 5 changed files with 104 additions and 8 deletions.
diff --git a/MaterialDesignThemes.UITests/Samples/DialogHost/LoadAndUnloadControl.xaml b/MaterialDesignThemes.UITests/Samples/DialogHost/LoadAndUnloadControl.xaml
@@ -0,0 +1,29 @@
+<UserControl x:Class="MaterialDesignThemes.UITests.Samples.DialogHost.LoadAndUnloadControl"
+             xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
+             xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
+             xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" 
+             xmlns:d="http://schemas.microsoft.com/expression/blend/2008" 
+             xmlns:local="clr-namespace:MaterialDesignThemes.UITests.Samples.DialogHost"
+             xmlns:materialDesign="http://materialdesigninxaml.net/winfx/xaml/themes"
+             mc:Ignorable="d" 
+             d:DesignHeight="450" d:DesignWidth="800">
+  <Grid x:Name="RootGrid">
+    <Grid.ColumnDefinitions>
+      <ColumnDefinition Width="200" />
+      <ColumnDefinition />
+    </Grid.ColumnDefinitions>
+    <StackPanel>
+      <Button x:Name="LoadDialogHost" Click="LoadDialogHost_Click" Content="Load"/>
+      <Button x:Name="UnloadDialogHost" Click="UnloadDialogHost_Click" Content="Unload" />
+      <Button x:Name="ToggleIsOpen" Click="ToggleIsOpen_Click" Content="Toggle IsOpen" />
+    </StackPanel>
+
+    <materialDesign:DialogHost Grid.Column="1" x:Name="DialogHost">
+      <materialDesign:DialogHost.DialogContent>
+        <Border Padding="100">
+          <Button x:Name="CloseButton" Content="Close" Command="{x:Static materialDesign:DialogHost.CloseDialogCommand}" />
+        </Border>
+      </materialDesign:DialogHost.DialogContent>
+    </materialDesign:DialogHost>
+  </Grid>
+</UserControl>
diff --git a/MaterialDesignThemes.UITests/Samples/DialogHost/LoadAndUnloadControl.xaml.cs b/MaterialDesignThemes.UITests/Samples/DialogHost/LoadAndUnloadControl.xaml.cs
@@ -0,0 +1,27 @@
+namespace MaterialDesignThemes.UITests.Samples.DialogHost;
+
+/// <summary>
+/// Interaction logic for LoadAndUnloadControl.xaml
+/// </summary>
+public partial class LoadAndUnloadControl
+{
+    public LoadAndUnloadControl()
+    {
+        InitializeComponent();
+    }
+
+    private void LoadDialogHost_Click(object sender, RoutedEventArgs e)
+    {
+        RootGrid.Children.Add(DialogHost);
+    }
+
+    private void UnloadDialogHost_Click(object sender, RoutedEventArgs e)
+    {
+        RootGrid.Children.Remove(DialogHost);
+    }
+
+    private void ToggleIsOpen_Click(object sender, RoutedEventArgs e)
+    {
+        DialogHost.IsOpen = !DialogHost.IsOpen;
+    }
+}
diff --git a/MaterialDesignThemes.UITests/TestBase.cs b/MaterialDesignThemes.UITests/TestBase.cs
@@ -6,6 +6,7 @@
 [assembly: GenerateHelpers(typeof(TimePicker))]
 [assembly: GenerateHelpers(typeof(DrawerHost))]
 [assembly: GenerateHelpers(typeof(ColorPicker))]
+[assembly: GenerateHelpers(typeof(DialogHost))]
 
 namespace MaterialDesignThemes.UITests;
 

diff --git a/MaterialDesignThemes.UITests/WPF/DialogHosts/DialogHostTests.cs b/MaterialDesignThemes.UITests/WPF/DialogHosts/DialogHostTests.cs
@@ -1,4 +1,4 @@
-using System.ComponentModel;
+using System.ComponentModel;
 using System.Windows.Media;
 using MaterialDesignThemes.UITests.Samples.DialogHost;
 
@@ -315,4 +315,37 @@ await Wait.For(async () =>
 
         recorder.Success();
     }
+
+    [Fact]
+    [Description("Issue 3069")]
+    public async Task DialogHost_WithOpenDialog_ShowsPopupWhenLoaded()
+    {
+        await using var recorder = new TestRecorder(App);
+
+        IVisualElement<Grid> rootGrid = (await LoadUserControl<LoadAndUnloadControl>()).As<Grid>();
+
+        IVisualElement<Button> loadButton = await rootGrid.GetElement<Button>("LoadDialogHost");
+        IVisualElement<Button> unloadButton = await rootGrid.GetElement<Button>("UnloadDialogHost");
+        IVisualElement<Button> toggleButton = await rootGrid.GetElement<Button>("ToggleIsOpen");
+
+        IVisualElement<DialogHost> dialogHost = await rootGrid.GetElement<DialogHost>("DialogHost");
+        IVisualElement<Button> closeButton = await dialogHost.GetElement<Button>("CloseButton");
+
+        await toggleButton.LeftClick();
+
+        await Wait.For(async () => Assert.True(await dialogHost.GetIsOpen()));
+        await Wait.For(async () => Assert.True(await closeButton.GetIsVisible()));
+
+        await unloadButton.LeftClick();
+
+        await Wait.For(async () => Assert.False(await closeButton.GetIsVisible()));
+
+        await loadButton.LeftClick();
+
+        await Wait.For(async () => Assert.True(await closeButton.GetIsVisible()));
+
+        await closeButton.LeftClick();
+
+        await Wait.For(async () => Assert.False(await dialogHost.GetIsOpen()));
+    }
 }
diff --git a/MaterialDesignThemes.Wpf/DialogHost.cs b/MaterialDesignThemes.Wpf/DialogHost.cs
@@ -854,10 +854,10 @@ private void OnUnloaded(object sender, RoutedEventArgs routedEventArgs)
         {
             foreach (var weakRef in LoadedInstances.ToList())
             {
-                if (!weakRef.TryGetTarget(out DialogHost? dialogHost) ||
-                    Equals(dialogHost, this))
+                if (!weakRef.TryGetTarget(out DialogHost? dialogHost) || ReferenceEquals(dialogHost, this))
                 {
                     LoadedInstances.Remove(weakRef);
+                    break;
                 }
             }
         }
@@ -866,16 +866,22 @@ private void OnLoaded(object sender, RoutedEventArgs routedEventArgs)
         {
             foreach (var weakRef in LoadedInstances.ToList())
             {
-                if (!weakRef.TryGetTarget(out DialogHost? dialogHost))
-                {
-                    LoadedInstances.Remove(weakRef);
-                }
-                if (Equals(dialogHost, this))
+                if (weakRef.TryGetTarget(out DialogHost? dialogHost) && ReferenceEquals(dialogHost, this))
                 {
                     return;
                 }
             }
+
             LoadedInstances.Add(new WeakReference<DialogHost>(this));
+
+            if (IsOpen && _popup is { } popup)
+            {
+                if (!popup.IsOpen)
+                {
+                    popup.IsOpen = true;
+                    (popup as PopupEx)?.RefreshPosition();
+                }
+            }
         }
     }
 }