Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (2)

1-35: 🛠️ Refactor suggestion

Duplicate license header – remove the second block

Two identical #region License blocks appear back-to-back. This bloats the file and will trigger static-analysis “duplicate-text” warnings.

-#region License
-/*
-Copyright …
-*/
-#endregion
-#region License
+#region License
 /*
 Copyright …
 */
 #endregion

---

3946-3963: ⚠️ Potential issue

REST helper leaks RestClient – make it per-request

restClient is a field but every call to GetDeviceMetricOverAPI re-initialises it, overwriting any concurrent request.
Because the driver is used from parallel Ginger runners this causes race conditions.

Instantiate a local RestClient inside the method or guard access with a lock.

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

Ginger/Ginger/Actions/ActionEditPages/ActMobileDeviceEditPage.xaml.cs (1)

224-406: 🧹 Nitpick (assertive)

Consider refactoring the SetControlsView method for better maintainability.

The SetControlsView method is getting large with numerous switch cases (now more than 20). As more mobile actions are added, this method will become difficult to maintain.

Consider extracting each case or group of related cases into separate methods:

private void SetControlsView()
{
    ResetAllPanelsVisibility();
    
    switch (mAct.MobileDeviceAction)
    {
        case ActMobileDevice.eMobileDeviceAction.PressKey:
        case ActMobileDevice.eMobileDeviceAction.LongPressKey:
            ConfigureKeyPressView();
            break;
            
        // Other cases...
        
        case ActMobileDevice.eMobileDeviceAction.StartActivity:
            ConfigureStartActivityView();
            break;
    }
}

private void ResetAllPanelsVisibility()
{
    xKeyPressPnl.Visibility = Visibility.Collapsed;
    xXY1Pnl.Visibility = Visibility.Collapsed;
    // Reset all other panels...
}

private void ConfigureKeyPressView()
{
    xKeyPressPnl.Visibility = Visibility.Visible;
}

private void ConfigureStartActivityView()
{
    xAppPnl.Visibility = Visibility.Visible;
    xInputLabelVE.Content = "Activity:";
    xInputTextBlock.Text = "";
    xInputPnl.Visibility = Visibility.Visible;
}

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

Ginger/Ginger/Actions/ActionEditPages/ActMobileDeviceEditPage.xaml.cs (6)

28-28: Unused imports detected.

The imports System.Drawing.Drawing2D and System.Linq appear to be unused in this class. Consider removing them to keep the code clean and reduce unnecessary dependencies.

-using System.Drawing.Drawing2D;
-using System.Linq;

Also applies to: 30-30

---

86-88: Consider clearer naming convention for folder path controls.

The naming of xFolderPathTxtBox and xFolderPathTextBox is too similar and could lead to confusion during maintenance. These controls appear to have different purposes (device folder path vs local folder path).

-xFolderPathTxtBox.Init(Context.GetAsContext(mAct.Context), mAct.FolderPathInput, nameof(ActMobileDevice.Value), true, true, UCValueExpression.eBrowserType.Folder, "*");
-
-xFolderPathTextBox.Init(Context.GetAsContext(mAct.Context), mAct.LocalFolderPathInput, nameof(ActMobileDevice.Value), true, true, UCValueExpression.eBrowserType.Folder, "*");
+xDeviceFolderPathTextBox.Init(Context.GetAsContext(mAct.Context), mAct.FolderPathInput, nameof(ActMobileDevice.Value), true, true, UCValueExpression.eBrowserType.Folder, "*");
+
+xLocalFolderPathTextBox.Init(Context.GetAsContext(mAct.Context), mAct.LocalFolderPathInput, nameof(ActMobileDevice.Value), true, true, UCValueExpression.eBrowserType.Folder, "*");

---

375-379: Add input validation for time duration inputs.

For LockForDuration action that requires time in seconds, there's no validation to ensure users enter numeric values or reasonable durations. Consider adding validation to prevent runtime errors.

case ActMobileDevice.eMobileDeviceAction.LockForDuration:
    xInputLabelVE.Content = "Set a Time Duration for Lock(Seconds):";
    xInputTextBlock.Text = "";
    xInputPnl.Visibility = Visibility.Visible;
+   // Add validation for numeric input in seconds
+   xInputVE.ValueTextBox.PreviewTextInput += (s, e) => 
+   {
+       e.Handled = !char.IsDigit(e.Text[0]) && !char.IsPunctuation(e.Text[0]);
+   };
    break;

---

384-389: Consider adding documentation for the new actions.

The new action GrantAppPermission is added without comments explaining its purpose or expected inputs. Adding XML documentation would improve maintainability.

+/// <summary>
+/// Configures UI for granting app permissions
+/// </summary>
case ActMobileDevice.eMobileDeviceAction.GrantAppPermission:
    xAppPnl.Visibility = Visibility.Visible;
    xInputLabelVE.Content = "Permission:";
    xInputTextBlock.Text = ""; 
    xInputPnl.Visibility = Visibility.Visible;
    break;

---

399-404: Add input validation for time duration inputs.

For SendAppToBackground action that requires time in seconds, there's no validation to ensure users enter numeric values or reasonable durations. Consider adding validation to prevent runtime errors.

case ActMobileDevice.eMobileDeviceAction.SendAppToBackground:
    xInputLabelVE.Content = "Set a Time Duration for Backgroud(Seconds):";
    xInputTextBlock.Text = "";
    xInputPnl.Visibility = Visibility.Visible; 
+   // Add validation for numeric input in seconds
+   xInputVE.ValueTextBox.PreviewTextInput += (s, e) => 
+   {
+       e.Handled = !char.IsDigit(e.Text[0]) && !char.IsPunctuation(e.Text[0]);
+   };
    break;

---

405-409: Consider adding documentation for the new actions.

The new action StartActivity is added without comments explaining its purpose or expected inputs. Adding XML documentation would improve maintainability.

+/// <summary>
+/// Configures UI for starting a specific activity within an application
+/// </summary>
case ActMobileDevice.eMobileDeviceAction.StartActivity:
    xAppPnl.Visibility = Visibility.Visible;
    xInputLabelVE.Content = "Activity:";
    xInputTextBlock.Text = "";
    xInputPnl.Visibility = Visibility.Visible;
    break;

Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs (1)

527-528: Clarify iOS-specific keyboard action in enum description.

The description "Type Using keyboard" doesn't clearly indicate that this is primarily for iOS keyboard simulation (as shown in the implementation and UI note). Consider updating the description to be more specific.

-    [EnumValueDescription("Type Using keyboard")]
+    [EnumValueDescription("Type Using iOS keyboard")]
     TypeUsingIOSkeyboard,

Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (5)

55-55: NUglify.Html directive is still unused – please drop it

Identical feedback was provided in an earlier review; the directive remains and is still unused.
Safe to delete.

---

4127-4139: SetFilePath still suffers from shadowing, path bugs & throws compilation errors

Issues (unchanged from the previous review):

1. Local fileName shadows the parameter FileName.
2. When fileName exists the returned path becomes "{FilePath}.{FileType}", losing the original file name and duplicating the extension.
3. No path separator is inserted when the caller forgets a trailing slash.
4. Uses the invalid throw new($"…") syntax flagged above.

Suggested minimal, compilable fix:

-public string SetFilePath(string FileType, string FilePath, string FileName)
+public string SetFilePath(string fileType, string directory, string baseName)
{
-    if (!string.IsNullOrEmpty(FilePath) && Directory.Exists(FilePath))
+    if (!Directory.Exists(directory))
+        throw new DirectoryNotFoundException($"Directory '{directory}' does not exist");
+
+    // Normalise input
+    baseName ??= "file";
+
+    string timeStamp  = DateTime.Now.ToString("yyyyMMddHHmmss");
+    string targetName = $"{baseName}_{timeStamp}.{fileType.TrimStart('.')}";
+    return Path.Combine(directory, targetName);
-    …
-    throw new($"An error occurred while running script: Got invalid Directory or empty folder");
}

---

4583-4587: IsAlphaUpper() still undefined – compile-time error

c.IsAlphaUpper() is not a BCL method, nor is an extension defined in this file/namespace.
Compilation will fail unless an extension exists elsewhere.
Replace with the standard char.IsUpper(c) (or define the extension).

---

4990-4999: PushFolder path concatenation is still fragile

var remoteFilePath = remoteFolderPath + Path.GetFileName(filePath);

If remoteFolderPath lacks a trailing “/”, the resulting path is invalid (…/folderfile.txt).
Use a safe join that guarantees exactly one slash:

-var remoteFilePath = remoteFolderPath + Path.GetFileName(filePath);
+var remoteFilePath = $"{remoteFolderPath.TrimEnd('/')}/{Path.GetFileName(filePath)}";

---

4865-4876: 🧹 Nitpick (assertive)

Blocking the driver thread with Thread.Sleep hampers responsiveness

Thread.Sleep(…) in LockForDuration blocks the STA thread, preventing:

• event pumping,
• cancellation requests,
• UI refreshes.

Prefer asynchronous waits or driver-native wait APIs, e.g.:

await Task.Delay(TimeSpan.FromSeconds(seconds), token);

Or, on Android, poll Driver.IsLocked() in a loop with short delays.

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

Ginger/Ginger/Actions/ActionEditPages/ActMobileDeviceEditPage.xaml (3)

95-96: 🧹 Nitpick (assertive)

Inconsistent control widths.

The width of controls in this panel varies between 330px and 335px. Consider standardizing widths for better visual consistency.

-<Actions:UCValueExpression x:Name="xFilePathTextBox" HorizontalAlignment="Left" Margin="0,0,0,0" Width="335"/>
+<Actions:UCValueExpression x:Name="xFilePathTextBox" HorizontalAlignment="Left" Width="330"/>

---

101-101: 🧹 Nitpick (assertive)

Remove redundant empty line.

There's an unnecessary empty line at the end of this section.

---

98-99: 🧹 Nitpick (assertive)

Inconsistent control width and helpful path information added.

The width inconsistency continues here, but the added TextBlock with predefined target folder paths provides valuable information to the user.

-<Actions:UCValueExpression x:Name="xFolderPathTxtBox" Width="335" />
+<Actions:UCValueExpression x:Name="xFolderPathTxtBox" Width="330" />

Ginger/Ginger/Actions/ActionEditPages/ActMobileDeviceEditPage.xaml.cs (6)

84-84: Verify if folder controls are still needed.

Previous review comments questioned whether folder controls are still necessary. Please confirm if this control initialization is still required.

---

358-362: 🛠️ Refactor suggestion

Add validation for iOS-specific typing action.

The TypeUsingIOSkeyboard action is primarily targeted at iOS devices as indicated by the note in the UI. Consider adding a runtime check to validate if the device is iOS before attempting to execute.

case ActMobileDevice.eMobileDeviceAction.TypeUsingIOSkeyboard:
    xInputLabelVE.Content = "Text:";
    xInputTextBlock.Text = "Note: For IOS, Keyboard Should be Open.";
+   // Add warning message for Android devices
+   if (mContext?.RunsetExecutor?.CurrentActivity?.CurrentDriver?.IsDriverTypeAppiumAndroid == true)
+   {
+     xInputTextBlock.Text = "WARNING: This action is optimized for iOS. For Android, consider using standard input methods.";
+   }
    xInputPnl.Visibility = Visibility.Visible;               
    break;

---

372-376: 🛠️ Refactor suggestion

Add input validation for time duration inputs.

For LockForDuration action that require time in seconds, there's no validation to ensure users enter numeric values or reasonable durations. Consider adding validation to prevent runtime errors.

case ActMobileDevice.eMobileDeviceAction.LockForDuration:
    xInputLabelVE.Content = "Set a Time Duration for Lock(Seconds):";
    xInputTextBlock.Text = "";
    xInputPnl.Visibility = Visibility.Visible;
+   // Add validation for numeric input in seconds
+   xInputVE.ValueTextBox.PreviewTextInput += (s, e) => 
+   {
+       e.Handled = !char.IsDigit(e.Text[0]) && !char.IsPunctuation(e.Text[0]);
+   };
    break;

---

381-386: 🧹 Nitpick (assertive)

Consider adding documentation for the new actions.

The new action GrantAppPermission is added without comments explaining its purpose or expected inputs. Adding XML documentation would improve maintainability.

+/// <summary>
+/// Configures UI for granting app permissions
+/// </summary>
case ActMobileDevice.eMobileDeviceAction.GrantAppPermission:
    xAppPnl.Visibility = Visibility.Visible;
    xInputLabelVE.Content = "Permission:";
    xInputTextBlock.Text = ""; 
    xInputPnl.Visibility = Visibility.Visible;
    break;

---

387-392: 🛠️ Refactor suggestion

Add input validation for SendAppToBackground duration.

Similar to LockForDuration, this action requires numeric time in seconds but has no validation to ensure valid input.

case ActMobileDevice.eMobileDeviceAction.SendAppToBackground:
    xInputLabelVE.Content = "Set a Time Duration for Backgroud(Seconds):";
    xInputTextBlock.Text = "";
    xInputPnl.Visibility = Visibility.Visible;
+   // Add validation for numeric input in seconds
+   xInputVE.ValueTextBox.PreviewTextInput += (s, e) => 
+   {
+       e.Handled = !char.IsDigit(e.Text[0]) && !char.IsPunctuation(e.Text[0]);
+   };
    break;

---

393-399: 🧹 Nitpick (assertive)

Add documentation for StartActivity action.

Similar to other new actions, StartActivity would benefit from XML documentation explaining its purpose and required inputs.

+/// <summary>
+/// Configures UI for starting an Android activity
+/// </summary>
case ActMobileDevice.eMobileDeviceAction.StartActivity:
    xAppPnl.Visibility = Visibility.Visible;
    xInputLabelVE.Content = "Activity:";
    xInputTextBlock.Text = "";
    xInputPnl.Visibility = Visibility.Visible;
    break;

Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs (4)

525-528: 🧹 Nitpick (assertive)

Clarify iOS-specific keyboard action in enum description.

The description "Type Using keyboard" doesn't clearly indicate that this is primarily for iOS keyboard simulation (as shown in the implementation and UI note). Consider updating the description to be more specific.

-    [EnumValueDescription("Type Using keyboard")]
+    [EnumValueDescription("Type Using iOS keyboard")]
     TypeUsingIOSkeyboard,

---

553-554: 🧹 Nitpick (assertive)

Correct capitalization in WiFi enum description.

As noted in previous reviews, use consistent capitalization for "WiFi" in the enum description.

-    [EnumValueDescription("Toggle WIFI")] 
+    [EnumValueDescription("Toggle WiFi")] 
     ToggleWifi,

---

525-569: 🧹 Nitpick (assertive)

Consider adding enum value grouping or categorization.

With the significant expansion of mobile device actions (now over 45 operations), it might be beneficial to organize these enum values into logical categories, either through comments, regions, or a nested enum approach. This would improve maintainability as the list continues to grow.

For example, you could add region directives to group related actions:

 public enum eMobileDeviceAction
 {
+    #region Touch Interaction
     [EnumValueDescription("Press XY")]
     PressXY,
     // other touch-related actions...
+    #endregion

+    #region Device Controls
     [EnumValueDescription("Press Back Button")]
     PressBackButton,
     // other device control actions...
+    #endregion

+    #region App Management
     [EnumValueDescription("Open App")]
     OpenApp,
     // other app management actions...
+    #endregion

+    #region Network & Connectivity
     [EnumValueDescription("Toggle WiFi")] 
     ToggleWifi,
     // other network-related actions...
+    #endregion

     // etc.
 }

---

158-169: 💡 Verification agent

🧩 Analysis chain

Remove commented note and verify if property is needed.

The comment "// need to check" suggests uncertainty about this property. Verify if this property is required and remove the comment if it is.

-public ActInputValue LocalFolderPathInput // need to check
+public ActInputValue LocalFolderPathInput

Run this script to check if the property is used elsewhere in the codebase:

---

🏁 Script executed:

#!/bin/bash
# Check for usage of LocalFolderPathInput in the codebase
rg "LocalFolderPathInput" --type cs

Length of output: 579

---

🏁 Script executed:

#!/bin/bash
# List files in the UI/Mobile directory
echo "Files in ActionsLib/UI/Mobile:"
ls Ginger/GingerCoreNET/ActionsLib/UI/Mobile

# Search for LocalFolderPathInput across all file types (including XAML)
echo -e "\nSearching for 'LocalFolderPathInput' in all files:"
rg "LocalFolderPathInput" -n

Length of output: 884

---

Remove the inline “need to check” comment and clean up the unused property

I didn’t find any references to LocalFolderPathInput outside of its own declaration (no usages in code or XAML), so the comment can be dropped—and if this input isn’t actually used by the mobile‐device action, you may want to delete the entire property.

• File to update:

• Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs

• Suggested diff:

-public ActInputValue LocalFolderPathInput // need to check
+public ActInputValue LocalFolderPathInput

If you confirm the property isn’t needed at runtime or in the UI, remove lines 158–169 altogether.

Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (6)

46-56: Unused using directives still present

Cassandra and NUglify.Html namespaces are still imported but never referenced, so the compiler will emit warnings and you pull two heavy transitive dependencies for no benefit.
Please delete these directives or use conditional compilation.

---

4113-4116: throw new($"…") is still invalid C# syntax

The same compile-time error that was flagged earlier remains: throw new($"Failed to get app identifier …");
new must be followed by a concrete exception type.

- throw new($"Failed to get app identifier attribute name: {ex.Message}");
+ throw new InvalidOperationException($"Failed to get app identifier attribute name: {ex.Message}", ex);

---

4377-4383: Device path assembled with Path.Combine breaks on non-Windows devices

Path.Combine(DeviceTargerFolder, Path.GetFileName(filePath)) inserts back-slashes on Windows hosts, yielding an invalid device path (/sdcard\photo.png).
Use / explicitly for the remote end:

- var remoteFilePath = Path.Combine(DeviceTargerFolder, Path.GetFileName(filePath));
+ var remoteFilePath = $"{DeviceTargerFolder.TrimEnd('/')}/{Path.GetFileName(filePath)}";

---

4573-4579: IsAlphaUpper() is not a BCL method – will not compile

char has no IsAlphaUpper() extension in the core libraries.
Use char.IsUpper(c) or add the missing extension.

- if ((c.IsAlphaUpper() && !isShiftActive) || (Char.IsLower(c) && isShiftActive))
+ if ((char.IsUpper(c) && !isShiftActive) || (char.IsLower(c) && isShiftActive))

---

4821-4830: 🧹 Nitpick (assertive)

⚠️ Potential issue

Synchronous Thread.Sleep blocks the driver thread

LockForDuration uses Thread.Sleep to wait while the device is locked.
This blocks the STA thread and the agent cannot receive cancellation or UI events.

Replace with an asynchronous delay:

- Thread.Sleep(1000 * int.Parse(time));
+ await Task.Delay(TimeSpan.FromSeconds(seconds), CancellationToken.None);

---

4121-4131: 🛠️ Refactor suggestion

⚠️ Potential issue

SetFilePath – Incorrect path construction & exception usage

1. Path.GetFileName(FilePath) returns the directory name only when the path ends with a back-slash; otherwise you lose the filename for single-file inputs.
2. The successful branch concatenates "{FilePath}.{FileType}", dropping the original filename and creating c:\dir\.<ext>.
3. Mixing Path.Combine (back-slash) with the device path produced elsewhere could introduce invalid paths on Linux/Android.
4. throw new("…") again omits an exception type.

Suggested fix:

- string timestamp = DateTime.Now.ToString("yyyyMMddHHmmss");
- string fileName = Path.GetFileName(FilePath);
- string targetFile = string.IsNullOrEmpty(fileName) ? Path.Combine(FilePath, $"{FileName}_{timestamp}.{FileType}") : $"{FilePath}.{FileType}";
+ if (!Directory.Exists(FilePath))
+     throw new DirectoryNotFoundException($"Directory '{FilePath}' does not exist");

+ string safeExtension = FileType.TrimStart('.');
+ string baseName      = string.IsNullOrEmpty(FileName) ? "file" : FileName;
+ string targetFile    = Path.Combine(
+     FilePath,
+     $"{baseName}_{DateTime.Now:yyyyMMddHHmmss}.{safeExtension}");

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (1)

19-35: 🧹 Nitpick (assertive)

Duplicate license header – remove redundancy

The file contains the full Apache-2 license block twice.
Keeping both adds unnecessary noise, complicates future merges, and inflates the diff-stat.

-#region License
-/*
-… second, identical license header …
-*/
-#endregion

Please delete the duplicated block and keep a single header at the top of the file.

Ginger/Ginger/Actions/ActionEditPages/ActMobileDeviceEditPage.xaml.cs (5)

357-361: Add validation for iOS-specific typing action.

The TypeUsingIOSkeyboard action is primarily targeted at iOS devices as indicated by the note in the UI. Consider adding a runtime check to validate if the device is iOS before attempting to execute.

---

371-375: Add input validation for time duration inputs.

For LockForDuration action that requires time in seconds, there's no validation to ensure users enter numeric values or reasonable durations. Consider adding validation to prevent runtime errors.

---

380-385: Consider adding documentation for the new actions.

The new action GrantAppPermission is added without comments explaining its purpose or expected inputs. Adding XML documentation would improve maintainability.

---

392-397: Consider adding documentation for the new actions.

The new action StartActivity is added without comments explaining its purpose or expected inputs. Adding XML documentation would improve maintainability.

---

84-84: 🛠️ Refactor suggestion

Consistent naming is needed for folder path textboxes.

The naming scheme between xFolderPathTxtBox and what appears to be a similar control elsewhere (xFolderPathTextBox) is inconsistent and could cause confusion during maintenance.

Consider renaming the controls to follow a more descriptive and consistent naming convention that reflects their purpose, such as:

-xFolderPathTxtBox.Init(Context.GetAsContext(mAct.Context), mAct.FolderPathInput, nameof(ActMobileDevice.Value), true, true, UCValueExpression.eBrowserType.Folder, "*");
+xDeviceFolderPathTextBox.Init(Context.GetAsContext(mAct.Context), mAct.FolderPathInput, nameof(ActMobileDevice.Value), true, true, UCValueExpression.eBrowserType.Folder, "*");

Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (5)

54-54: Unused using directive was already flagged earlier
using NUglify.Html; is still not referenced anywhere and will continue to trigger CS8019.

The previous review comment on this exact line still applies.
Please remove the directive (or use the type) to avoid the warning.

---

4821-4824: Synchronous Thread.Sleep blocks the STA thread

The earlier review pointed this out; Thread.Sleep is still used in LockForDuration.
Replace with an asynchronous wait (await Task.Delay(…) with a CancellationToken) to keep the agent responsive.

---

4573-4577: ⚠️ Potential issue

IsAlphaUpper() causes compile error – use char.IsUpper

char has no IsAlphaUpper() method (unless an extension is defined elsewhere).

-            if ((c.IsAlphaUpper() && !isShiftActive) || (Char.IsLower(c) && isShiftActive))
+            if ((char.IsUpper(c) && !isShiftActive) || (char.IsLower(c) && isShiftActive))

---

4112-4115: ⚠️ Potential issue

throw new without exception type – code will not compile

The C# compiler expects a concrete exception type after new.

-                throw new($"Failed to get app identifier attribute name: {ex.Message}");
+                throw new InvalidOperationException($"Failed to get app identifier attribute name: {ex.Message}", ex);

Search & replace the same pattern throughout the file (throw new($"…")) to unblock the build.

---

4120-4126: 🛠️ Refactor suggestion

SetFilePath still has parameter shadowing & malformed path logic

fileName shadows the FileName parameter and the else-branch appends the extension to the directory ("C:\tmp\logs.txt" ➜ "C:\tmp\.txt").

-            if (!string.IsNullOrEmpty(FilePath) && Directory.Exists(FilePath))
-            {
-                string timestamp = DateTime.Now.ToString("yyyyMMddHHmmss");
-                string fileName = Path.GetFileName(FilePath);
-                string targetFile = string.IsNullOrEmpty(fileName) ? Path.Combine(FilePath, $"{FileName}_{timestamp}.{FileType}") : $"{FilePath}.{FileType}";
-                return targetFile;
-            }
+            if (string.IsNullOrWhiteSpace(FilePath) || !Directory.Exists(FilePath))
+                throw new DirectoryNotFoundException($"Directory '{FilePath}' does not exist");
+
+            string timestamp   = DateTime.Now.ToString("yyyyMMddHHmmss");
+            string baseName    = string.IsNullOrWhiteSpace(FileName) ? "file" : Path.GetFileNameWithoutExtension(FileName);
+            string targetName  = $"{baseName}_{timestamp}.{FileType.TrimStart('.')}";
+            return Path.Combine(FilePath, targetName);

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

Learnt from: prashelke
PR: Ginger-Automation/Ginger#3429
File: Ginger/Ginger/AutomatePageLib/AddActionMenu/WindowExplorer/Common/WindowExplorerPage.xaml.cs:1123-1126
Timestamp: 2024-07-26T22:04:12.930Z
Learning: The user has removed an unnecessary empty line at 1123 in `WindowExplorerPage.xaml.cs` for better code compactness.