Merge pull request #607 from TheJoeFin/copilot/sub-pr-606

Refactor post-grab actions: fix memory leaks, improve performance, enhance maintainability

Author: TheJoeFin

Text-Grab/Models/ButtonInfo.cs

@@ -43,7 +43,15 @@ public override bool Equals(object? obj)
 
     public override int GetHashCode()
     {
-        return System.HashCode.Combine(ButtonText, SymbolText, Background, Command, ClickEvent, IsRelevantForFullscreenGrab, IsRelevantForEditWindow);
+        return System.HashCode.Combine(
+            ButtonText, 
+            SymbolText, 
+            Background, 
+            Command, 
+            ClickEvent, 
+            IsRelevantForFullscreenGrab, 
+            IsRelevantForEditWindow,
+            DefaultCheckState);
     }
 
     // a constructor which takes a collapsible button

Text-Grab/Utilities/PostGrabActionManager.cs

@@ -22,13 +22,10 @@ public static List<ButtonInfo> GetAvailablePostGrabActions()
         List<ButtonInfo> allPostGrabActions = [.. GetDefaultPostGrabActions()];
 
         // Add other relevant actions from AllButtons that are marked as relevant for FullscreenGrab
-        foreach (ButtonInfo button in ButtonInfo.AllButtons)
-        {
-            if (button.IsRelevantForFullscreenGrab && !allPostGrabActions.Any(b => b.ButtonText == button.ButtonText))
-            {
-                allPostGrabActions.Add(button);
-            }
-        }
+        IEnumerable<ButtonInfo> relevantActions = ButtonInfo.AllButtons
+            .Where(button => button.IsRelevantForFullscreenGrab && !allPostGrabActions.Any(b => b.ButtonText == button.ButtonText));
+        
+        allPostGrabActions.AddRange(relevantActions);
 
         return [.. allPostGrabActions.OrderBy(b => b.OrderNumber)];
     }
@@ -144,11 +141,12 @@ public static bool GetCheckState(ButtonInfo action)
             try
             {
                 Dictionary<string, bool>? checkStates = JsonSerializer.Deserialize<Dictionary<string, bool>>(statesJson);
-                if (checkStates is not null && checkStates.TryGetValue(action.ButtonText, out bool storedState))
+                if (checkStates is not null 
+                    && checkStates.TryGetValue(action.ButtonText, out bool storedState)
+                    && action.DefaultCheckState == DefaultCheckState.LastUsed)
                 {
                     // If the action is set to LastUsed, use the stored state
-                    if (action.DefaultCheckState == DefaultCheckState.LastUsed)
-                        return storedState;
+                    return storedState;
                 }
             }
             catch (JsonException)
@@ -202,11 +200,14 @@ public static async Task<string> ExecutePostGrabAction(ButtonInfo action, string
 
             case "TrimEachLine_Click":
                 string[] stringSplit = text.Split(Environment.NewLine);
-                string finalString = "";
-                foreach (string line in stringSplit)
-                    if (!string.IsNullOrWhiteSpace(line))
-                        finalString += line.Trim() + Environment.NewLine;
-                result = finalString;
+                string[] trimmedLines = stringSplit
+                    .Where(line => !string.IsNullOrWhiteSpace(line))
+                    .Select(line => line.Trim())
+                    .ToArray();
+
+                result = trimmedLines.Length == 0
+                    ? string.Empty
+                    : string.Join(Environment.NewLine, trimmedLines) + Environment.NewLine;
                 break;
 
             case "RemoveDuplicateLines_Click":

Text-Grab/Views/FullscreenGrab.xaml.cs

@@ -48,6 +48,8 @@ public partial class FullscreenGrab : Window
     private const double MaxZoomScale = 16.0;
     private const double EdgePanThresholdPercent = 0.10;
     private const double EdgePanSpeed = 8.0;
+    private const string EditPostGrabActionsTag = "EditPostGrabActions";
+    private const string ClosePostGrabMenuTag = "ClosePostGrabMenu";
     private readonly DispatcherTimer edgePanTimer;
 
     #endregion Fields
@@ -270,6 +272,12 @@ private void LoadDynamicPostGrabActions()
         // Get the PostGrabStayOpen setting
         bool stayOpen = DefaultSettings.PostGrabStayOpen;
 
+        // Remove any existing keyboard handler to avoid duplicates
+        contextMenu.PreviewKeyDown -= FullscreenGrab_KeyDown;
+        
+        // Add keyboard handling once for the entire context menu
+        contextMenu.PreviewKeyDown += FullscreenGrab_KeyDown;
+
         int index = 1;
         foreach (ButtonInfo action in enabledActions)
         {
@@ -286,9 +294,6 @@ private void LoadDynamicPostGrabActions()
             // Wire up click handler
             menuItem.Click += PostActionMenuItem_Click;
 
-            // Add keyboard handling
-            contextMenu.PreviewKeyDown += FullscreenGrab_KeyDown;
-
             contextMenu.Items.Add(menuItem);
             index++;
         }
@@ -298,15 +303,17 @@ private void LoadDynamicPostGrabActions()
         // Add "Edit this list..." menu item
         MenuItem editPostGrabMenuItem = new()
         {
-            Header = "Edit this list..."
+            Header = "Edit this list...",
+            Tag = EditPostGrabActionsTag
         };
         editPostGrabMenuItem.Click += EditPostGrabActions_Click;
         contextMenu.Items.Add(editPostGrabMenuItem);
 
         // Add "Close this menu" menu item
         MenuItem hidePostGrabMenuItem = new()
         {
-            Header = "Close this menu"
+            Header = "Close this menu",
+            Tag = ClosePostGrabMenuTag
         };
         hidePostGrabMenuItem.Click += HidePostGrabActions_Click;
         contextMenu.Items.Add(hidePostGrabMenuItem);
@@ -1010,6 +1017,36 @@ private void Window_Unloaded(object sender, RoutedEventArgs e)
         if (RegionClickCanvas.Children.Contains(selectBorder))
             RegionClickCanvas.Children.Remove(selectBorder);
 
+        // Clean up dynamically created post-grab action menu items
+        if (NextStepDropDownButton.Flyout is ContextMenu contextMenu)
+        {
+            contextMenu.PreviewKeyDown -= FullscreenGrab_KeyDown;
+            
+            foreach (object item in contextMenu.Items)
+            {
+                if (item is MenuItem menuItem)
+                {
+                    if (menuItem.Tag is ButtonInfo)
+                    {
+                        menuItem.Click -= PostActionMenuItem_Click;
+                    }
+                    else if (menuItem.Tag is string tag)
+                    {
+                        if (tag == EditPostGrabActionsTag)
+                        {
+                            menuItem.Click -= EditPostGrabActions_Click;
+                        }
+                        else if (tag == ClosePostGrabMenuTag)
+                        {
+                            menuItem.Click -= HidePostGrabActions_Click;
+                        }
+                    }
+                }
+            }
+            
+            contextMenu.Items.Clear();
+        }
+
         CurrentScreen = null;
         dpiScale = null;
         TextFromOCR = null;
@@ -1100,12 +1137,11 @@ private void TableToggleButton_Click(object? sender = null, RoutedEventArgs? e =
     private void PostActionMenuItem_Click(object sender, RoutedEventArgs e)
     {
         // Save check state for LastUsed tracking
-        if (sender is MenuItem menuItem && menuItem.Tag is ButtonInfo action)
+        if (sender is MenuItem menuItem 
+            && menuItem.Tag is ButtonInfo action
+            && action.DefaultCheckState == DefaultCheckState.LastUsed)
         {
-            if (action.DefaultCheckState == DefaultCheckState.LastUsed)
-            {
-                PostGrabActionManager.SaveCheckState(action, menuItem.IsChecked);
-            }
+            PostGrabActionManager.SaveCheckState(action, menuItem.IsChecked);
         }
 
         CheckIfAnyPostActionsSelected();