Skip to content

Commit 394e8c5

Browse files
Address code review nitpicks: improve code clarity and organization
- Add parentheses to Delete key condition for clarity - Introduce _isClearCanvasModalVisible field to track modal state instead of checking UI - Extract CancelActiveToolSafely() method to reduce code duplication Co-authored-by: RuntimeRascal <2422222+RuntimeRascal@users.noreply.github.com>
1 parent 7f9bfbb commit 394e8c5

2 files changed

Lines changed: 24 additions & 6 deletions

File tree

Src/GhostDraw/Core/GlobalKeyboardHook.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ private nint HookCallback(int nCode, nint wParam, nint lParam)
217217
}
218218

219219
// Check for Delete key press (clear canvas - only when drawing mode is active)
220-
if (vkCode == VK_DELETE && isKeyDown && _isDrawingModeActive)
220+
if ((vkCode == VK_DELETE) && isKeyDown && _isDrawingModeActive)
221221
{
222222
_logger.LogDebug("Delete key pressed - clear canvas confirmation request");
223223
ClearCanvasPressed?.Invoke(this, EventArgs.Empty);

Src/GhostDraw/Views/OverlayWindow.xaml.cs

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -778,7 +778,7 @@ public bool HandleEscapeKey()
778778
try
779779
{
780780
// Check if confirmation modal is visible first (highest priority)
781-
if (ClearCanvasModalGrid.Visibility == Visibility.Visible)
781+
if (_isClearCanvasModalVisible)
782782
{
783783
_logger.LogDebug("ESC pressed while confirmation modal visible - canceling clear");
784784
HideClearCanvasModal();
@@ -897,6 +897,7 @@ private void HideScreenshotSavedToast()
897897
private Action? _clearCanvasCancelCallback;
898898
private IDrawingTool? _toolBeforeModal;
899899
private bool _wasDrawingBeforeModal;
900+
private bool _isClearCanvasModalVisible;
900901

901902
/// <summary>
902903
/// Shows the clear canvas confirmation modal
@@ -916,17 +917,15 @@ public void ShowClearCanvasConfirmation(Action onConfirm, Action onCancel)
916917
_wasDrawingBeforeModal = _isDrawing;
917918

918919
// Cancel any in-progress drawing
919-
if (_activeTool != null)
920-
{
921-
_activeTool.Cancel(DrawingCanvas);
922-
}
920+
CancelActiveToolSafely();
923921

924922
// Temporarily disable drawing and reset cursor
925923
_isDrawing = false;
926924
this.Cursor = WpfCursors.Arrow;
927925

928926
// Show the modal
929927
ClearCanvasModalGrid.Visibility = Visibility.Visible;
928+
_isClearCanvasModalVisible = true;
930929

931930
_logger.LogDebug("Clear canvas confirmation modal shown");
932931
}
@@ -1012,6 +1011,7 @@ private void HideClearCanvasModal()
10121011
try
10131012
{
10141013
ClearCanvasModalGrid.Visibility = Visibility.Collapsed;
1014+
_isClearCanvasModalVisible = false;
10151015
_logger.LogDebug("Clear canvas confirmation modal hidden");
10161016
}
10171017
catch (Exception ex)
@@ -1053,6 +1053,24 @@ private void RestoreToolStateAfterModal()
10531053
}
10541054
}
10551055

1056+
/// <summary>
1057+
/// Safely cancels the active tool if one is active
1058+
/// </summary>
1059+
private void CancelActiveToolSafely()
1060+
{
1061+
try
1062+
{
1063+
if (_activeTool != null)
1064+
{
1065+
_activeTool.Cancel(DrawingCanvas);
1066+
}
1067+
}
1068+
catch (Exception ex)
1069+
{
1070+
_logger.LogError(ex, "Failed to cancel active tool");
1071+
}
1072+
}
1073+
10561074
#endregion
10571075

10581076
#region Undo/Redo

0 commit comments

Comments
 (0)