Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
TheJoeFin merged 5 commits into from
Jan 11, 2026
Merged

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

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
39a00f5
Initial plan
Copilot Jan 11, 2026
542cf17
Address PR review comments: fix hash code, improve string concatenati…
Copilot Jan 11, 2026
e618c81
Fix TrimEachLine logic and use Tag-based menu item identification
Copilot Jan 11, 2026
ddd73c3
Remove unused import and extract magic strings to constants
Copilot Jan 11, 2026
481aad0
Use explicit types instead of var to respect editor config style
Copilot Jan 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion Text-Grab/Models/ButtonInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 16 additions & 15 deletions Text-Grab/Utilities/PostGrabActionManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
var relevantActions = ButtonInfo.AllButtons
Copy link
Owner

@TheJoeFin TheJoeFin Jan 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Respect editor config style and use explicit types

Copy link
Contributor Author

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to explicit type IEnumerable<ButtonInfo> in commit 481aad0.

.Where(button => button.IsRelevantForFullscreenGrab && !allPostGrabActions.Any(b => b.ButtonText == button.ButtonText));

allPostGrabActions.AddRange(relevantActions);

return [.. allPostGrabActions.OrderBy(b => b.OrderNumber)];
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
var trimmedLines = stringSplit
Copy link
Owner

@TheJoeFin TheJoeFin Jan 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use explicit types

Copy link
Contributor Author

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to explicit type string[] in commit 481aad0.

.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":
Expand Down
56 changes: 46 additions & 10 deletions Text-Grab/Views/FullscreenGrab.xaml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
{
Expand All @@ -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++;
}
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down