Skip to content

Feature/uft lab app selection#4472

Merged
ravirk91 merged 6 commits into
masterfrom
Feature/UFTLabAppConfig
Mar 30, 2026
Merged

Feature/uft lab app selection#4472
ravirk91 merged 6 commits into
masterfrom
Feature/UFTLabAppConfig

Conversation

@omri1911

@omri1911 omri1911 commented Mar 24, 2026

Copy link
Copy Markdown
Collaborator

in the uft lab phone selection (in the phone agent config)
added another tab to the phone selection for selecting apps

Summary by CodeRabbit

  • New Features

    • Added app selection alongside device selection in the mobile configuration dialog.
    • New "Apps" tab shows available applications with OS filter, loading states, icons, versions and copyable identifiers.
    • Selected app details are applied to the configuration when confirmed; UI enforces device–app platform compatibility.
  • Improvements

    • Non-matching platform devices/apps are visually de-emphasized.
    • Fetch button label updated to "Select Lab Device/App".

@omri1911
omri1911 requested a review from ravirk91 March 24, 2026 13:55
@coderabbitai

coderabbitai Bot commented Mar 25, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Adds UFT Mobile app discovery and selection to the UFT Credentials Dialog, introduces an Apps tab alongside Devices, and wires selected app details into AppiumDriverEditPage to update Appium capabilities (bundleId / appPackage / appActivity) with platform validation.

Changes

Cohort / File(s) Summary
UFT Credentials Dialog (UI & code)
Ginger/Ginger/Drivers/DriversConfigsEditPages/UFTCredentialsDialog.xaml, Ginger/Ginger/Drivers/DriversConfigsEditPages/UFTCredentialsDialog.xaml.cs
Adds an Apps tab UI and device loading indicators; implements async apps retrieval (/rest/apps/getAplicationsLastVersion), JSON parsing, app models, base64 icon rendering, platform filtering/graying, app selection properties, and platform-consistency validation on Ok. Constructor gains initialAppId and initialPlatform params.
Appium Driver Edit Page (code)
Ginger/Ginger/Drivers/DriversConfigsEditPages/AppiumDriverEditPage.xaml, Ginger/Ginger/Drivers/DriversConfigsEditPages/AppiumDriverEditPage.xaml.cs
Updates Fetch Devices button label; passes resolved currentAppId and currentPlatform into UFTCredentialsDialog; adds ApplySelectedAppDetails(UFTCredentialsDialog) to set or remove appium:bundleId, appium:appPackage, and appium:appActivity based on selected app type and refreshes capabilities grid.

Sequence Diagram

sequenceDiagram
    participant ADE as AppiumDriverEditPage
    participant Dialog as UFTCredentialsDialog
    participant API as UFT Mobile API

    ADE->>Dialog: Open dialog (initialAppId, initialPlatform)
    activate Dialog

    Dialog->>API: GET /rest/devices
    API-->>Dialog: devices list
    Dialog->>Dialog: DisplayDevices() / ApplyDeviceFilters()

    Dialog->>API: GET /rest/apps/getAplicationsLastVersion
    API-->>Dialog: apps payload
    Dialog->>Dialog: ParseApps(json) / DisplayApps() / ApplyAppFilters(initialPlatform)

    note over Dialog: User selects device and app
    Dialog->>Dialog: Validate platform match on Ok
    Dialog-->>ADE: DialogResult = true (with selected device & app)
    deactivate Dialog

    ADE->>ADE: ApplySelectedPhoneDetails(dialog)
    ADE->>ADE: ApplySelectedAppDetails(dialog)
    ADE->>ADE: Update Appium capabilities (bundleId / appPackage / appActivity)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • shahanemahesh
  • Maheshkale447

Poem

🐰
With tabs for phones and tabs for apps,
I hop through lists and tiny maps,
BundleIds hum and packages sing,
Platforms matched — what joy they bring!
Capabilities leap — a merry spring.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description is incomplete and does not follow the required template structure with Type of Change and Checklist sections. Complete the description by filling out the Type of Change checkbox (New feature applies here) and working through all checklist items systematically.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Feature/uft lab app selection' accurately describes the main change: adding app selection functionality to the UFT Lab phone selection dialog.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch Feature/UFTLabAppConfig

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Ginger/Ginger/Drivers/DriversConfigsEditPages/AppiumDriverEditPage.xaml.cs`:
- Around line 867-904: The ApplySelectedAppDetails method can delete
cross-platform capabilities even when the selected platform's identifier is
empty, potentially clearing all app identifiers; update ApplySelectedAppDetails
to guard deletions/update logic: before calling
DeleteCapabilityIfExist("appium:appPackage")/DeleteCapabilityIfExist("appium:appActivity")
for iOS, check if dialog.SelectedAppBundleId is non-empty (or else skip deletion
and show a user warning); likewise for Android, only call
DeleteCapabilityIfExist("appium:bundleId") when dialog.SelectedAppPackage or
SelectedAppActivity is non-empty (or show a warning) — adjust the branches
around
UpdateCapabilityValue("appium:bundleId"/"appium:appPackage"/"appium:appActivity")
and the deletion calls, and use ApplyPlatformFromSelection and
xCapabilitiesGrid.Grid?.Items.Refresh as before to refresh only when actual
changes occur.

In `@Ginger/Ginger/Drivers/DriversConfigsEditPages/UFTCredentialsDialog.xaml`:
- Around line 326-335: The stacked Image and ImageMakerControl currently rely on
layering for fallback but lack an explicit visibility swap when IconSource is
null; update the XAML around Image (binding IconSource) and ImageMakerControl
(binding IconType/SetAsFontImageWithSize) so Image.Visibility is collapsed when
IconSource is null (use TargetNullValue or a converter) and
ImageMakerControl.Visibility is Visible as the fallback (or use a DataTrigger
that toggles their Visibility based on IconSource); also ensure the
ImageMakerControl has a default style/foreground set so its fallback icon
renders consistently.

In `@Ginger/Ginger/Drivers/DriversConfigsEditPages/UFTCredentialsDialog.xaml.cs`:
- Line 509: The code currently reads bool instrumented =
obj.TryGetValue("instrumented", out JToken instToken) &&
instToken.Value<bool>(); which can throw if "instrumented" is a non-boolean
type; change the logic in the method that performs this parse to defensively
handle types: first call obj.TryGetValue("instrumented", out JToken instToken),
then if instToken != null check instToken.Type == JTokenType.Boolean and use
instToken.Value<bool>(); else attempt bool.TryParse(instToken.ToString(), out
var parsed) and use parsed; otherwise set instrumented = false (or a safe
default). Update the parse around the symbols obj, instToken and the
instrumented variable accordingly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: ce396f40-6979-4a41-bf85-22ade6cc185c

📥 Commits

Reviewing files that changed from the base of the PR and between 4921559 and 02498d5.

📒 Files selected for processing (3)
  • Ginger/Ginger/Drivers/DriversConfigsEditPages/AppiumDriverEditPage.xaml.cs
  • Ginger/Ginger/Drivers/DriversConfigsEditPages/UFTCredentialsDialog.xaml
  • Ginger/Ginger/Drivers/DriversConfigsEditPages/UFTCredentialsDialog.xaml.cs

Comment on lines +867 to +904
private void ApplySelectedAppDetails(UFTCredentialsDialog dialog)
{
if (dialog == null || string.IsNullOrWhiteSpace(dialog.SelectedAppId))
{
return;
}

bool wasUpdated = false;

if (string.Equals(dialog.SelectedAppType, "IOS", StringComparison.OrdinalIgnoreCase))
{
if (!string.IsNullOrWhiteSpace(dialog.SelectedAppBundleId))
{
wasUpdated |= UpdateCapabilityValue("appium:bundleId", dialog.SelectedAppBundleId);
}
DeleteCapabilityIfExist("appium:appPackage");
DeleteCapabilityIfExist("appium:appActivity");
}
else
{
if (!string.IsNullOrWhiteSpace(dialog.SelectedAppPackage))
{
wasUpdated |= UpdateCapabilityValue("appium:appPackage", dialog.SelectedAppPackage);
}
if (!string.IsNullOrWhiteSpace(dialog.SelectedAppActivity))
{
wasUpdated |= UpdateCapabilityValue("appium:appActivity", dialog.SelectedAppActivity);
}
DeleteCapabilityIfExist("appium:bundleId");
}

bool platformUpdated = ApplyPlatformFromSelection(dialog.SelectedAppType);

if (wasUpdated || platformUpdated)
{
xCapabilitiesGrid.Grid?.Items.Refresh();
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Edge case: App with missing identifiers may leave capabilities in unclear state.

If an iOS app is selected but SelectedAppBundleId is empty, the Android capabilities (appPackage/appActivity) are deleted without setting any iOS capability. Similarly for Android apps with empty SelectedAppPackage. This could leave the configuration without any app identifier.

Consider either:

  1. Not deleting cross-platform capabilities when the target platform's capability is empty, or
  2. Showing a warning to the user that the selected app has incomplete configuration.
🛡️ Proposed guard to avoid clearing capabilities without replacement
         if (string.Equals(dialog.SelectedAppType, "IOS", StringComparison.OrdinalIgnoreCase))
         {
             if (!string.IsNullOrWhiteSpace(dialog.SelectedAppBundleId))
             {
                 wasUpdated |= UpdateCapabilityValue("appium:bundleId", dialog.SelectedAppBundleId);
+                DeleteCapabilityIfExist("appium:appPackage");
+                DeleteCapabilityIfExist("appium:appActivity");
             }
-            DeleteCapabilityIfExist("appium:appPackage");
-            DeleteCapabilityIfExist("appium:appActivity");
         }
         else
         {
-            if (!string.IsNullOrWhiteSpace(dialog.SelectedAppPackage))
+            bool hasAndroidCap = false;
+            if (!string.IsNullOrWhiteSpace(dialog.SelectedAppPackage))
             {
                 wasUpdated |= UpdateCapabilityValue("appium:appPackage", dialog.SelectedAppPackage);
+                hasAndroidCap = true;
             }
             if (!string.IsNullOrWhiteSpace(dialog.SelectedAppActivity))
             {
                 wasUpdated |= UpdateCapabilityValue("appium:appActivity", dialog.SelectedAppActivity);
+                hasAndroidCap = true;
+            }
+            if (hasAndroidCap)
+            {
+                DeleteCapabilityIfExist("appium:bundleId");
             }
-            DeleteCapabilityIfExist("appium:bundleId");
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private void ApplySelectedAppDetails(UFTCredentialsDialog dialog)
{
if (dialog == null || string.IsNullOrWhiteSpace(dialog.SelectedAppId))
{
return;
}
bool wasUpdated = false;
if (string.Equals(dialog.SelectedAppType, "IOS", StringComparison.OrdinalIgnoreCase))
{
if (!string.IsNullOrWhiteSpace(dialog.SelectedAppBundleId))
{
wasUpdated |= UpdateCapabilityValue("appium:bundleId", dialog.SelectedAppBundleId);
}
DeleteCapabilityIfExist("appium:appPackage");
DeleteCapabilityIfExist("appium:appActivity");
}
else
{
if (!string.IsNullOrWhiteSpace(dialog.SelectedAppPackage))
{
wasUpdated |= UpdateCapabilityValue("appium:appPackage", dialog.SelectedAppPackage);
}
if (!string.IsNullOrWhiteSpace(dialog.SelectedAppActivity))
{
wasUpdated |= UpdateCapabilityValue("appium:appActivity", dialog.SelectedAppActivity);
}
DeleteCapabilityIfExist("appium:bundleId");
}
bool platformUpdated = ApplyPlatformFromSelection(dialog.SelectedAppType);
if (wasUpdated || platformUpdated)
{
xCapabilitiesGrid.Grid?.Items.Refresh();
}
}
private void ApplySelectedAppDetails(UFTCredentialsDialog dialog)
{
if (dialog == null || string.IsNullOrWhiteSpace(dialog.SelectedAppId))
{
return;
}
bool wasUpdated = false;
if (string.Equals(dialog.SelectedAppType, "IOS", StringComparison.OrdinalIgnoreCase))
{
if (!string.IsNullOrWhiteSpace(dialog.SelectedAppBundleId))
{
wasUpdated |= UpdateCapabilityValue("appium:bundleId", dialog.SelectedAppBundleId);
DeleteCapabilityIfExist("appium:appPackage");
DeleteCapabilityIfExist("appium:appActivity");
}
}
else
{
bool hasAndroidCap = false;
if (!string.IsNullOrWhiteSpace(dialog.SelectedAppPackage))
{
wasUpdated |= UpdateCapabilityValue("appium:appPackage", dialog.SelectedAppPackage);
hasAndroidCap = true;
}
if (!string.IsNullOrWhiteSpace(dialog.SelectedAppActivity))
{
wasUpdated |= UpdateCapabilityValue("appium:appActivity", dialog.SelectedAppActivity);
hasAndroidCap = true;
}
if (hasAndroidCap)
{
DeleteCapabilityIfExist("appium:bundleId");
}
}
bool platformUpdated = ApplyPlatformFromSelection(dialog.SelectedAppType);
if (wasUpdated || platformUpdated)
{
xCapabilitiesGrid.Grid?.Items.Refresh();
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Ginger/Ginger/Drivers/DriversConfigsEditPages/AppiumDriverEditPage.xaml.cs`
around lines 867 - 904, The ApplySelectedAppDetails method can delete
cross-platform capabilities even when the selected platform's identifier is
empty, potentially clearing all app identifiers; update ApplySelectedAppDetails
to guard deletions/update logic: before calling
DeleteCapabilityIfExist("appium:appPackage")/DeleteCapabilityIfExist("appium:appActivity")
for iOS, check if dialog.SelectedAppBundleId is non-empty (or else skip deletion
and show a user warning); likewise for Android, only call
DeleteCapabilityIfExist("appium:bundleId") when dialog.SelectedAppPackage or
SelectedAppActivity is non-empty (or show a warning) — adjust the branches
around
UpdateCapabilityValue("appium:bundleId"/"appium:appPackage"/"appium:appActivity")
and the deletion calls, and use ApplyPlatformFromSelection and
xCapabilitiesGrid.Grid?.Items.Refresh as before to refresh only when actual
changes occur.

Comment on lines +326 to +335
<Grid Width="28" Height="28" Margin="0,0,6,0" HorizontalAlignment="Left">
<UserControls:ImageMakerControl Width="28"
Height="28"
ImageType="{Binding IconType}"
SetAsFontImageWithSize="24" />
<Image Source="{Binding IconSource}"
Width="28"
Height="28"
Stretch="Uniform" />
</Grid>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider using a fallback for missing app icons.

The Image and ImageMakerControl are stacked in a Grid, but when IconSource is null, the Image will be invisible and only ImageMakerControl shows. This layering approach works, but ensure ImageMakerControl is styled to be visible as the fallback.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Ginger/Ginger/Drivers/DriversConfigsEditPages/UFTCredentialsDialog.xaml`
around lines 326 - 335, The stacked Image and ImageMakerControl currently rely
on layering for fallback but lack an explicit visibility swap when IconSource is
null; update the XAML around Image (binding IconSource) and ImageMakerControl
(binding IconType/SetAsFontImageWithSize) so Image.Visibility is collapsed when
IconSource is null (use TargetNullValue or a converter) and
ImageMakerControl.Visibility is Visible as the fallback (or use a DataTrigger
that toggles their Visibility based on IconSource); also ensure the
ImageMakerControl has a default style/foreground set so its fallback icon
renders consistently.

string fileName = GetStringValue(obj, "fileName");
string instrumentationStatus = GetStringValue(obj, "instrumentationStatus");
string iconBase64 = GetStringValue(obj, "icon");
bool instrumented = obj.TryGetValue("instrumented", out JToken instToken) && instToken.Value<bool>();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Potential exception when parsing non-boolean instrumented field.

If the API returns instrumented as a string (e.g., "true") or an incompatible type, Value<bool>() could throw an exception or return unexpected results.

🛡️ Proposed defensive parsing
-            bool instrumented = obj.TryGetValue("instrumented", out JToken instToken) && instToken.Value<bool>();
+            bool instrumented = false;
+            if (obj.TryGetValue("instrumented", out JToken instToken))
+            {
+                if (instToken.Type == JTokenType.Boolean)
+                {
+                    instrumented = instToken.Value<bool>();
+                }
+                else if (instToken.Type == JTokenType.String)
+                {
+                    bool.TryParse(instToken.ToString(), out instrumented);
+                }
+            }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
bool instrumented = obj.TryGetValue("instrumented", out JToken instToken) && instToken.Value<bool>();
bool instrumented = false;
if (obj.TryGetValue("instrumented", out JToken instToken))
{
if (instToken.Type == JTokenType.Boolean)
{
instrumented = instToken.Value<bool>();
}
else if (instToken.Type == JTokenType.String)
{
bool.TryParse(instToken.ToString(), out instrumented);
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Ginger/Ginger/Drivers/DriversConfigsEditPages/UFTCredentialsDialog.xaml.cs`
at line 509, The code currently reads bool instrumented =
obj.TryGetValue("instrumented", out JToken instToken) &&
instToken.Value<bool>(); which can throw if "instrumented" is a non-boolean
type; change the logic in the method that performs this parse to defensively
handle types: first call obj.TryGetValue("instrumented", out JToken instToken),
then if instToken != null check instToken.Type == JTokenType.Boolean and use
instToken.Value<bool>(); else attempt bool.TryParse(instToken.ToString(), out
var parsed) and use parsed; otherwise set instrumented = false (or a safe
default). Update the parse around the symbols obj, instToken and the
instrumented variable accordingly.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Ginger/Ginger/Drivers/DriversConfigsEditPages/UFTCredentialsDialog.xaml`:
- Around line 325-340: The ListBox.ItemContainerStyle currently disables
keyboard focus visuals via FocusVisualStyle="{x:Null}", which harms
accessibility; instead restore a subtle focus visual: create a reusable
FocusVisualStyle resource (or a minimal Setter on the same Style) that adds a
non-intrusive border/outline or background change for ListBoxItem when focused,
and reference it from the ListBox.ItemContainerStyle (replace the
FocusVisualStyle="{x:Null}" Setter). Keep other behavior
(PreviewMouseLeftButtonDown handler AppItem_PreviewMouseLeftButtonDown and the
ControlTemplate/ContentPresenter) unchanged.

In `@Ginger/Ginger/Drivers/DriversConfigsEditPages/UFTCredentialsDialog.xaml.cs`:
- Around line 1565-1589: The ConvertBase64ToImage method currently swallows all
exceptions in its catch block; update that catch to capture the Exception (e.g.,
catch (Exception ex)) and write a debug-level log entry including the exception
message/stack and minimal context (such as whether base64 was null/empty or its
length) using the project's logging facility (or System.Diagnostics.Debug/Trace
if no logger exists) before returning null so failed icon conversions are
recorded for troubleshooting.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: b042e858-04bc-4f7e-aa7c-53ca7e58cb29

📥 Commits

Reviewing files that changed from the base of the PR and between 02498d5 and 564dff0.

📒 Files selected for processing (3)
  • Ginger/Ginger/Drivers/DriversConfigsEditPages/AppiumDriverEditPage.xaml
  • Ginger/Ginger/Drivers/DriversConfigsEditPages/UFTCredentialsDialog.xaml
  • Ginger/Ginger/Drivers/DriversConfigsEditPages/UFTCredentialsDialog.xaml.cs

Comment on lines +325 to +340
<ListBox.ItemContainerStyle>
<Style TargetType="ListBoxItem">
<Setter Property="Padding" Value="0" />
<Setter Property="Margin" Value="0" />
<Setter Property="Background" Value="Transparent" />
<Setter Property="BorderThickness" Value="0" />
<Setter Property="FocusVisualStyle" Value="{x:Null}" />
<EventSetter Event="PreviewMouseLeftButtonDown" Handler="AppItem_PreviewMouseLeftButtonDown" />
<Setter Property="Template">
<Setter.Value>
<ControlTemplate TargetType="ListBoxItem">
<ContentPresenter />
</ControlTemplate>
</Setter.Value>
</Setter>
</Style>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Consider preserving keyboard focus visual for accessibility.

Setting FocusVisualStyle="{x:Null}" removes the keyboard focus indicator, which may impact users who navigate using keyboard. This matches the Devices tab pattern, but consider adding a subtle focus visual or border change for accessibility compliance.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Ginger/Ginger/Drivers/DriversConfigsEditPages/UFTCredentialsDialog.xaml`
around lines 325 - 340, The ListBox.ItemContainerStyle currently disables
keyboard focus visuals via FocusVisualStyle="{x:Null}", which harms
accessibility; instead restore a subtle focus visual: create a reusable
FocusVisualStyle resource (or a minimal Setter on the same Style) that adds a
non-intrusive border/outline or background change for ListBoxItem when focused,
and reference it from the ListBox.ItemContainerStyle (replace the
FocusVisualStyle="{x:Null}" Setter). Keep other behavior
(PreviewMouseLeftButtonDown handler AppItem_PreviewMouseLeftButtonDown and the
ControlTemplate/ContentPresenter) unchanged.

Comment on lines +1565 to +1589
private static ImageSource ConvertBase64ToImage(string base64)
{
if (string.IsNullOrWhiteSpace(base64))
{
return null;
}

try
{
byte[] bytes = Convert.FromBase64String(base64);
using var ms = new System.IO.MemoryStream(bytes);
var bitmap = new BitmapImage();
bitmap.BeginInit();
bitmap.CacheOption = BitmapCacheOption.OnLoad;
bitmap.StreamSource = ms;
bitmap.DecodePixelWidth = 56;
bitmap.EndInit();
bitmap.Freeze();
return bitmap;
}
catch
{
return null;
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider logging failed icon conversions for troubleshooting.

The bare catch block silently swallows all exceptions without logging. While the fallback behavior (returning null) is appropriate for UI resilience, consider adding debug-level logging to aid troubleshooting when icons fail to load.

🔧 Suggested improvement
             catch
             {
+                // Optionally log at DEBUG level for troubleshooting
+                // Reporter.ToLog(eLogLevel.DEBUG, $"Failed to decode app icon from base64");
                 return null;
             }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Ginger/Ginger/Drivers/DriversConfigsEditPages/UFTCredentialsDialog.xaml.cs`
around lines 1565 - 1589, The ConvertBase64ToImage method currently swallows all
exceptions in its catch block; update that catch to capture the Exception (e.g.,
catch (Exception ex)) and write a debug-level log entry including the exception
message/stack and minimal context (such as whether base64 was null/empty or its
length) using the project's logging facility (or System.Diagnostics.Debug/Trace
if no logger exists) before returning null so failed icon conversions are
recorded for troubleshooting.

}
catch
{
return null;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

add debug log to catch exception

@ravirk91
ravirk91 merged commit 4a81282 into master Mar 30, 2026
13 of 14 checks passed
@ravirk91
ravirk91 deleted the Feature/UFTLabAppConfig branch March 30, 2026 06:39
@coderabbitai coderabbitai Bot mentioned this pull request Mar 31, 2026
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants