Ginger/Ginger/ExternalConfigurations/ApplitoolsExternalConfigurationsPage.xaml.cs (1)

77-81: Consolidate with other hyperlink handlers.

This is a duplicate of the code duplication issue flagged in GingerPlayConfigurationpage.xaml.cs (lines 247-251). Please apply the same refactoring solution across all affected files.

Ginger/Ginger/ExternalConfigurations/SealightsExternalConfigurationsPage.xaml.cs (1)

148-152: Consolidate with other hyperlink handlers.

This is a duplicate of the code duplication issue flagged in GingerPlayConfigurationpage.xaml.cs (lines 247-251). Please apply the same refactoring solution across all affected files.

Ginger/Ginger/ExternalConfigurations/WireMockConfigurationPage.xaml.cs (1)

144-148: Consolidate with other hyperlink handlers.

This is a duplicate of the code duplication issue flagged in GingerPlayConfigurationpage.xaml.cs (lines 247-251). Please apply the same refactoring solution across all affected files.

Ginger/Ginger/ExternalConfigurations/VRTExternalConfigurationsPage.xaml.cs (1)

93-97: Consolidate with other hyperlink handlers.

This is a duplicate of the code duplication issue flagged in GingerPlayConfigurationpage.xaml.cs (lines 247-251). Please apply the same refactoring solution across all affected files.

Ginger/Ginger/ExternalConfigurations/WireMockConfigurationPage.xaml (2)

47-52: Remove unnecessary Grid structure.

Identical to the VRT file, this Grid defines two columns but only uses Grid.Column="0". This pattern is duplicated across multiple external configuration pages.

Apply this diff to simplify the structure:

-<StackPanel Orientation="Horizontal" Grid.Row="2" >
-    <Grid>
-        <Grid.ColumnDefinitions>
-            <ColumnDefinition Width="*" />
-            <ColumnDefinition Width="*" />
-        </Grid.ColumnDefinitions>
-
-        <!-- Left Side -->
-        <Grid Grid.Column="0" >
-            <StackPanel Margin="0,10,0,0" >
+<StackPanel Orientation="Horizontal" Grid.Row="2" Margin="0,10,0,0">

And at the end (lines 75-78):

-                        </StackPanel>
-                    </Grid>
-                </Grid>
-            </StackPanel>
+</StackPanel>

---

57-61: Avoid hardcoded dimensions for responsive design.

Same issue as in VRT file: hardcoded Width="591", Width="581", and Height="250" constrain responsiveness.

Consider using MaxWidth and removing fixed height:

-<Expander x:Name="xAboutWireMock" ExpandDirection="Down" Style="{StaticResource $ExpanderStyle}" IsExpanded="True" Width="591">
+<Expander x:Name="xAboutWireMock" ExpandDirection="Down" Style="{StaticResource $ExpanderStyle}" IsExpanded="True" MaxWidth="591">
     <Expander.Header>
         <Label Content="About WireMock" Style="{StaticResource $HorizontalExpanderLabelStyle}"/>
     </Expander.Header>
-    <TextBlock x:Name="xWireMockInfo" TextWrapping="Wrap" Width="581" Height="250" Padding="10,5,5,5" HorizontalAlignment="Left">
+    <TextBlock x:Name="xWireMockInfo" TextWrapping="Wrap" MaxWidth="581" Padding="10,5,5,5" HorizontalAlignment="Left">

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Ginger/Ginger/ExternalConfigurations/GingerPlayConfigurationpage.xaml.cs (1)

250-254: Centralize hyperlink handler + add URI validation and error handling

This duplicates across pages and lacks URI validation/try-catch. Prefer a single helper in GingerUIPage or a static utility; at minimum, harden this handler.

Apply this hardening inline:

-        private void Hyperlink_RequestNavigate(object sender, System.Windows.Navigation.RequestNavigateEventArgs e)
-        {
-            System.Diagnostics.Process.Start(new System.Diagnostics.ProcessStartInfo(e.Uri.AbsoluteUri) { UseShellExecute = true });
-            e.Handled = true;
-        }
+        private void Hyperlink_RequestNavigate(object sender, System.Windows.Navigation.RequestNavigateEventArgs e)
+        {
+            try
+            {
+                if (e?.Uri == null || !e.Uri.IsAbsoluteUri || (e.Uri.Scheme != Uri.UriSchemeHttp && e.Uri.Scheme != Uri.UriSchemeHttps))
+                {
+                    Reporter.ToLog(eLogLevel.WARN, $"Blocked navigation to invalid URI: {e?.Uri}");
+                    return;
+                }
+                Process.Start(new ProcessStartInfo(e.Uri.AbsoluteUri) { UseShellExecute = true });
+                e.Handled = true;
+            }
+            catch (System.ComponentModel.Win32Exception ex)
+            {
+                Reporter.ToLog(eLogLevel.ERROR, $"Failed to open URL: {e?.Uri}", ex);
+                Reporter.ToUser(eUserMsgKey.StaticErrorMessage, $"Unable to open link: {ex.Message}");
+            }
+            catch (Exception ex)
+            {
+                Reporter.ToLog(eLogLevel.ERROR, $"Unexpected error opening URL: {e?.Uri}", ex);
+            }
+        }

Ginger/Ginger/ExternalConfigurations/ZAPConfigurationPage.xaml (1)

91-93: Ensure handler exists; prefer centralized helper

Confirm ZAPConfigurationPage.xaml.cs defines Hyperlink_RequestNavigate; otherwise build will fail. Also centralize this handler as suggested on other pages.

#!/bin/bash
set -euo pipefail

echo "XAMLs wiring RequestNavigate to Hyperlink_RequestNavigate:"
rg -nP 'RequestNavigate\s*=\s*"Hyperlink_RequestNavigate"' -g '!**/bin/**' -g '!**/obj/**'

echo
echo "Handler definitions in code-behind:"
rg -nP --type=cs '\bvoid\s+Hyperlink_RequestNavigate\s*\(' -C2

echo
echo "Check if GingerUIPage already provides a shared handler:"
rg -nP --type=cs '(class\s+GingerUIPage\b.*?Hyperlink_RequestNavigate\()' -U -C2

Ginger/Ginger/ExternalConfigurations/WireMockConfigurationPage.xaml (1)

83-85: Deduplicate hyperlink navigation across pages

Replace per-page Hyperlink_RequestNavigate with a single base/helper implementation used by all ExternalConfigurations pages.

Ginger/Ginger/ExternalConfigurations/SealightsExternalConfigurationsPage.xaml (1)

138-140: Centralize the hyperlink handler

Use a shared handler (e.g., in GingerUIPage) rather than duplicating Hyperlink_RequestNavigate per page.

Ginger/Ginger/ExternalConfigurations/GingerPlayConfigurationpage.xaml (1)

91-129: Extract the About panel to a reusable UserControl

This pattern repeats across multiple pages; a shared control would reduce duplication and ensure consistent styling/content structure.

Ginger/Ginger/MainWindow.xaml (1)

92-136: Fix promo button sizing and accessibility; avoid overriding the default template

• Border Width=95 vs Button Width=99 will clip; let content size naturally.
• Negative Padding causes rendering issues; use positive padding.
• Custom template strips focus and keyboard cues; keep default for accessibility.
• Add AutomationProperties.Name and use a resource for text to enable localization.

Apply this diff:

-                        <Border Margin="10,0,0,0" Height="28" CornerRadius="6" Width="95">
+                        <Border Margin="10,0,0,0" Height="28" CornerRadius="6">
@@
-                                <Button
-Content="Upgrade"
+                                <Button x:Name="xUpgradeProFeaturesButton"
+Content="{DynamicResource UpgradeProFeatures_Text}"
                                 Foreground="White"
                                 Background="Transparent"
                                 FontWeight="Bold"
                                 FontSize="14"
                                 HorizontalAlignment="Left"
- ToolTip="Unlock Pro Features with Ginger Play"
-Padding="-10,0"
+ ToolTip="Unlock Pro Features with Ginger Play"
+Padding="10,0"
                                 BorderThickness="0"
                                 Cursor="Hand"
-Click="ProFeatureButtonClick" Margin="-10,0,0,0" Width="99" Grid.ColumnSpan="2" Height="28">
-                                    <Button.Style>
-                                        <Style TargetType="Button" BasedOn="{StaticResource {x:Type Button}}">
-                                            <Setter Property="Background" Value="Transparent"/>
-                                            <Setter Property="BorderBrush" Value="Transparent"/>
-                                            <Setter Property="OverridesDefaultStyle" Value="True"/>
-                                            <Setter Property="Template">
-                                                <Setter.Value>
-                                                    <ControlTemplate TargetType="Button">
-                                                        <ContentPresenter HorizontalAlignment="Center" VerticalAlignment="Center"/>
-                                                    </ControlTemplate>
-                                                </Setter.Value>
-                                            </Setter>
-                                        </Style>
-                                    </Button.Style>
-                                </Button>
+Click="ProFeatureButtonClick" Margin="0,0,0,0" Grid.ColumnSpan="2" Height="28"
+AutomationProperties.Name="Unlock Pro Features with Ginger Play"
+IsTabStop="True" Focusable="True"/>

If you need the text resource:

<!-- In resources -->
<string x:Key="UpgradeProFeatures_Text">Upgrade</string>

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Ginger/Ginger/ExternalConfigurations/VRTExternalConfigurationsPage.xaml (6)

142-148: Hyperlink handler verification still needed.

The hyperlinks use RequestNavigate="Hyperlink_RequestNavigate" handler. A previous review requested verification that the handler implementation sets e.Handled = true to prevent default navigation behavior. This verification has not been completed.

---

11-11: Title mismatch remains unresolved.

The page title is still "Execution Logger Configuration" instead of a VRT-specific title. This issue was flagged in a previous review but has not been addressed.

---

25-25: Irrelevant DockPanel.Dock attribute remains.

The DockPanel.Dock="Top" attribute on xSaveButton has no effect since it's inside a Grid. This issue was flagged in a previous review but has not been cleaned up.

---

32-49: Excessive zero-height rows remain.

Multiple RowDefinition entries with Height="0" are still present, which adds unnecessary complexity. This issue was flagged in a previous review but has not been addressed.

---

59-59: Fixed widths on input controls remain.

The Width="400" constraint on UCValueExpression controls prevents responsive layout. This issue was flagged in a previous review but has not been addressed.

Also applies to: 65-65, 77-77, 83-83, 88-88

---

91-91: Negative margin remains.

The Margin="-15,0,0,0" on xEnableSoftAssertPanel can cause layout glitches. This issue was flagged in a previous review but has not been addressed.

Ginger/Ginger/ExternalConfigurations/GingerPlayConfigurationpage.xaml.cs (2)

250-254: Consolidate duplicated hyperlink handler.

This method is already flagged in previous review comments as being duplicated across multiple configuration pages. The duplication violates DRY principles and should be consolidated into a base class or static helper.

---

299-354: Extract duplicated banner window to shared helper.

This comment echoes previous feedback and highlights additional duplication. ShowGingerPlayBannerWindow is duplicated identically in TwoLevelMenuPage.xaml.cs (lines 287-342), and the banner window modal issues flagged in earlier reviews remain unaddressed.

Extract the banner window logic to a static helper and ensure the window is properly modal:

public static class GingerPlayBannerHelper
{
    public static void ShowBannerWindow(Window owner)
    {
        var imageUri = new Uri("pack://application:,,,/Ginger;component/UserControlsLib/ImageMakerLib/Images/GingerPlayDetailsPopUpContent.png", UriKind.Absolute);
        
        var image = new System.Windows.Controls.Image
        {
            Source = new System.Windows.Media.Imaging.BitmapImage(imageUri),
            Stretch = Stretch.Uniform,
            HorizontalAlignment = HorizontalAlignment.Center,
            VerticalAlignment = VerticalAlignment.Center,
            Cursor = System.Windows.Input.Cursors.Hand
        };
        
        var viewbox = new Viewbox
        {
            Stretch = Stretch.Uniform,
            Child = image
        };
        
        var border = new Border
        {
            Child = viewbox,
            Background = Brushes.Transparent
        };
        
        Window bannerWindow = new Window
        {
            Title = "Get to know Ginger Play",
            Width = 900,
            Height = 700,
            WindowStartupLocation = WindowStartupLocation.CenterOwner,
            Owner = owner,
            ResizeMode = ResizeMode.NoResize,
            WindowStyle = WindowStyle.ThreeDBorderWindow,
            ShowInTaskbar = false,
            Content = border,
            SizeToContent = SizeToContent.Manual,
        };
        
        border.MouseLeftButtonUp += (s, args) =>
        {
            try
            {
                Process.Start(new ProcessStartInfo
                {
                    FileName = "https://www.amdocs.com/products-services/quality-engineering-services",
                    UseShellExecute = true
                });
                bannerWindow.Close();
            }
            catch (Exception ex)
            {
                MessageBox.Show("Unable to open the link: " + ex.Message, "Error", MessageBoxButton.OK, MessageBoxImage.Error);
            }
        };
        
        bannerWindow.ShowDialog();
    }
}

Then replace both implementations with:

-        private void ShowGingerPlayBannerWindow()
-        {
-            // ... entire method body
-        }
+        private void ShowGingerPlayBannerWindow()
+        {
+            GingerPlayBannerHelper.ShowBannerWindow(Window.GetWindow(this));
+        }

Ginger/Ginger/ExternalConfigurations/ApplitoolsExternalConfigurationsPage.xaml (1)

54-65: Remove fixed 400 px widths to prevent clipping in split layout.

The new 3*:2* grid leaves ~340 px for column 1 inside xApplitoolsGrid at standard window sizes. The hard-coded Width="400" forces both value-expression controls to extend past the column, overlapping the right “About” panel (and ScrollViewer has horizontal scrolling disabled). Let the grid manage width so the inputs stay visible.

-                            <Activities:UCValueExpression x:Name="xAPIURLTextBox" Grid.Column="1" Grid.Row="0" HorizontalAlignment="Left" VerticalAlignment="Center" ToolTip="Applitools API URL" Margin="10,0,0,0"  Width="400"/>
+                            <Activities:UCValueExpression x:Name="xAPIURLTextBox" Grid.Column="1" Grid.Row="0" HorizontalAlignment="Stretch" VerticalAlignment="Center" ToolTip="Applitools API URL" Margin="10,0,0,0" MinWidth="250"/>
...
-                            <Activities:UCValueExpression x:Name="xAPIKeyTextBox" Grid.Column="1" Grid.Row="2" HorizontalAlignment="Left" VerticalAlignment="Center" ToolTip="API Key" Margin="10,0,0,0"  Width="400"/>
+                            <Activities:UCValueExpression x:Name="xAPIKeyTextBox" Grid.Column="1" Grid.Row="2" HorizontalAlignment="Stretch" VerticalAlignment="Center" ToolTip="API Key" Margin="10,0,0,0" MinWidth="250"/>

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Learnt from: prashelke
PR: Ginger-Automation/Ginger#4254
File: Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs:70-73
Timestamp: 2025-07-16T14:42:32.229Z
Learning: In Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs, prashelke prefers to keep the current `!WorkSpace.Instance.BetaFeatures.ShowPOMForAI` logic for binding and showing AI POM controls, rather than changing it to positive logic.

Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4286
File: Ginger/Ginger/Drivers/DriversConfigsEditPages/WebAgentConfigEditPage.xaml.cs:556-571
Timestamp: 2025-08-28T09:27:18.847Z
Learning: In Ginger's WebAgentConfigEditPage.xaml.cs ZAP configuration, when ZAP Security Testing is enabled, the manual proxy UI should remain enabled/accessible even though ZAP will override the proxy settings in the background. This allows users to see and modify their proxy configurations for transparency and future use when ZAP is disabled. Confirmed by AmanPrasad43 in PR #4286.

Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4285
File: Ginger/Ginger/Actions/ActionEditPages/ActSecurityTestingEditPage.xaml:23-23
Timestamp: 2025-08-26T07:40:08.345Z
Learning: In the Ginger codebase ActSecurityTestingEditPage.xaml file, the large bottom margin of 530 on the "Acceptable Alerts" label (Margin="10,10,0,530") is intentional design choice, not an error.

Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4137
File: Ginger/Ginger/SourceControl/SourceControlProjectsPage.xaml.cs:21-21
Timestamp: 2025-03-20T11:10:33.780Z
Learning: The `Amdocs.Ginger.Common.SourceControlLib` namespace is required in files that reference the `GingerSolution` class for source control operations in the Ginger automation framework.

Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4188
File: Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs:22-23
Timestamp: 2025-04-25T13:29:45.059Z
Learning: The `using Amdocs.Ginger.Repository;` statement in Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs is necessary as it provides access to the `ObservableList<>` class which is used throughout the file for collections of RunSetConfig, AnalyzerItemBase, and ApplicationPOMModel objects.

Ginger/Ginger/ExternalConfigurations/SealightsExternalConfigurationsPage.xaml (2)

100-107: Remove dead layout block.

Empty StackPanel/Grid serve no purpose.

-                    <StackPanel Orientation="Horizontal" Grid.Row="2" >
-                        <Grid>
-                            <Grid.ColumnDefinitions>
-                                <ColumnDefinition Width="*" />
-                                <ColumnDefinition Width="*" />
-                            </Grid.ColumnDefinitions>
-                        </Grid>
-                    </StackPanel>

---

143-145: Split Hyperlink and icon onto separate lines.

Improves readability; matches style used elsewhere.

-                           </Hyperlink>   <usercontrols:ImageMakerControl ImageType="Link" Height="10" Width="10" Margin="0,2,10,0"/>                         </TextBlock>
+                           </Hyperlink>
+                           <usercontrols:ImageMakerControl ImageType="Link" Height="10" Width="10" Margin="0,2,10,0"/>
+                       </TextBlock>

Ginger/Ginger/ExternalConfigurations/WireMockConfigurationPage.xaml (4)

16-19: Add MinWidth to left column for responsiveness.

Prevents crowding on small widths.

 <Grid.ColumnDefinitions>
-    <ColumnDefinition Width="3*"/>
+    <ColumnDefinition Width="3*" MinWidth="400"/>
     <ColumnDefinition Width="2*"/>
 </Grid.ColumnDefinitions>

---

53-60: Use theme resources instead of hard-coded colors.

Swap GhostWhite/#B6E0FE for StaticResource brushes.

---

53-93: Add accessibility metadata (panel/link).

Provide AutomationProperties for screen readers.

-                <Border
+                <Border
+                    AutomationProperties.Name="About WireMock Information Panel"
...
-                        <TextBlock Margin="0,10,0,0" FontSize="14">
-                            <Hyperlink NavigateUri="https://wiremock.org/" RequestNavigate="Hyperlink_RequestNavigate">
+                        <TextBlock Margin="0,10,0,0" FontSize="14">
+                            <Hyperlink NavigateUri="https://wiremock.org/" RequestNavigate="Hyperlink_RequestNavigate"
+                                       AutomationProperties.Name="Learn more about WireMock (opens in browser)">
                                 <Run Text="Learn more about WireMock"/>
                             </Hyperlink>

---

87-90: Split Hyperlink and icon onto separate lines.

Improve readability.

-                           </Hyperlink><usercontrols:ImageMakerControl ImageType="Link" Height="10" Width="10" Margin="0,2,10,0"/></TextBlock>
+                           </Hyperlink>
+                           <usercontrols:ImageMakerControl ImageType="Link" Height="10" Width="10" Margin="0,2,10,0"/>
+                        </TextBlock>

Ginger/Ginger/ExternalConfigurations/VRTExternalConfigurationsPage.xaml (2)

11-11: Page title still incorrect (previously flagged).

The title still says "Execution Logger Configuration" instead of a VRT-specific title. This was flagged in a previous review but remains unresolved, causing potential confusion for users navigating to this page.

Apply this diff:

-	Title="Execution Logger Configuration">
+	Title="Visual Regression Testing (VRT) Configuration">

---

91-93: Negative margin still present despite previous resolution claim.

A previous review flagged the negative left margin (Margin="-15,0,0,0") and marked it as addressed in commit bab1efc. However, the negative margin is still present in the current code. Negative margins can cause layout glitches and should be avoided in favor of proper alignment and padding.

Apply this diff to remove the negative margin:

-<StackPanel x:Name="xEnableSoftAssertPanel" Margin="-15,0,0,0" Orientation="Horizontal" ToolTip="Select 'No' to ignore mismatches on checkpoint actions." VerticalAlignment="Center" HorizontalAlignment="Left" Grid.Row="12" Grid.Column="1">
+<StackPanel x:Name="xEnableSoftAssertPanel" Margin="0,0,0,0" Orientation="Horizontal" ToolTip="Select 'No' to ignore mismatches on checkpoint actions." VerticalAlignment="Center" HorizontalAlignment="Left" Grid.Row="12" Grid.Column="1">

If the intent is to align the radio buttons with controls in other rows, adjust the Grid's ColumnDefinition padding or the parent column's margin instead.

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4285
File: Ginger/Ginger/Actions/ActionEditPages/ActSecurityTestingEditPage.xaml:23-23
Timestamp: 2025-08-26T07:40:08.345Z
Learning: In the Ginger codebase ActSecurityTestingEditPage.xaml file, the large bottom margin of 530 on the "Acceptable Alerts" label (Margin="10,10,0,530") is intentional design choice, not an error.