Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
<RowDefinition Height="1*"/>
<RowDefinition Height="1*"/>
<RowDefinition Height="1*"/>
<RowDefinition Height="1*"/>
</Grid.RowDefinitions>

<Label x:Name="xVRTActionLabel" Grid.Row="0" Grid.Column="0" Margin="0,10,0,0" Style="{StaticResource $LabelStyle}" VerticalAlignment="Center" Content="VRT Action:"/>
Expand Down Expand Up @@ -85,22 +86,26 @@
<StackPanel x:Name="xBaselineImageRadioButtonPnl" Grid.Row="9" Grid.Column="1" Orientation="Horizontal" HorizontalAlignment="Left" VerticalAlignment="Center" Width="550" Margin="-25,10,0,0">
<UserControlsLib:UCRadioButtons x:Name="xBaselineImageRadioButton" VerticalAlignment="Center" HorizontalAlignment="Left"/>
</StackPanel>



<Label x:Name="xBaselineImagePath" Style="{StaticResource $LabelStyle}" Grid.Row="10" Grid.Column="0" Margin="0,10,0,0" VerticalAlignment="Center" Content="Select Image:"/>
<StackPanel x:Name="VRTCurrentBaselineImagePathTxtBoxPnl" Grid.Row="10" Grid.Column="1">
<Actions:UCValueExpression x:Name="VRTCurrentBaselineImagePathTxtBox" Grid.Row="1" Margin="0,10,0,0"/>
</StackPanel>

<Label x:Name="xPreviewImage" Style="{StaticResource $LabelStyle}" Grid.Row="11" Grid.Column="0" Margin="0,10,0,0" VerticalAlignment="Top" Content="Preview Image:"/>
<StackPanel x:Name="VRTBaseImageFramePnl" Grid.Row="11" Grid.Column="1">
<Label x:Name="xFullPageScreenshotLabel" Style="{StaticResource $LabelStyle}" Grid.Row="11" Grid.Column="0" Margin="0,10,0,0" VerticalAlignment="Center" Content="Full Page Screenshot:"/>
<CheckBox x:Name="xFullPageScreenshotCheckbox" Grid.Row="11" Grid.Column="1" VerticalAlignment="Center" Margin="5,10,0,0" Style="{StaticResource @InputCheckBoxStyle}" ToolTip="Capture the full page screenshot of the active window for baseline and checkpoint comparison"/>
Comment on lines +96 to +97
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 | ⚡ Quick win

Add Visibility="Collapsed" as the XAML default for the new controls.

Neither xFullPageScreenshotLabel nor xFullPageScreenshotCheckbox declares an initial Visibility. WPF defaults both to Visible, so they are briefly shown until InitLayout() runs in the constructor and collapses them for the default Track action state. Adding the attribute eliminates any possible visual flicker and makes the intent explicit, consistent with how other conditionally-shown rows behave in this page.

🛡️ Proposed fix
-        <Label x:Name="xFullPageScreenshotLabel" Style="{StaticResource $LabelStyle}" Grid.Row="11" Grid.Column="0" Margin="0,10,0,0" VerticalAlignment="Center" Content="Full Page Screenshot:"/>
-        <CheckBox x:Name="xFullPageScreenshotCheckbox" Grid.Row="11" Grid.Column="1" VerticalAlignment="Center" Margin="5,10,0,0" Style="{StaticResource `@InputCheckBoxStyle`}" ToolTip="Capture the full page screenshot of the active window for baseline and checkpoint comparison"/>
+        <Label x:Name="xFullPageScreenshotLabel" Visibility="Collapsed" Style="{StaticResource $LabelStyle}" Grid.Row="11" Grid.Column="0" Margin="0,10,0,0" VerticalAlignment="Center" Content="Full Page Screenshot:"/>
+        <CheckBox x:Name="xFullPageScreenshotCheckbox" Visibility="Collapsed" Grid.Row="11" Grid.Column="1" VerticalAlignment="Center" Margin="5,10,0,0" Style="{StaticResource `@InputCheckBoxStyle`}" ToolTip="Capture the full page screenshot of the active window for baseline and checkpoint comparison"/>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Ginger/Ginger/Actions/ActionEditPages/VisualTesting/VRTComparePage.xaml`
around lines 96 - 97, The new controls xFullPageScreenshotLabel and
xFullPageScreenshotCheckbox currently lack an explicit Visibility setting and
briefly render visible before InitLayout() collapses them for the default Track
action state; add Visibility="Collapsed" to both control declarations (the Label
named xFullPageScreenshotLabel and the CheckBox named
xFullPageScreenshotCheckbox in VRTComparePage.xaml) so they start hidden and
avoid flicker, matching the pattern used by other conditionally-shown rows
handled by InitLayout().


<Label x:Name="xPreviewImage" Style="{StaticResource $LabelStyle}" Grid.Row="12" Grid.Column="0" Margin="0,10,0,0" VerticalAlignment="Top" Content="Preview Image:"/>
<StackPanel x:Name="VRTBaseImageFramePnl" Grid.Row="12" Grid.Column="1">
<Frame x:Name="VRTBaseImageFrame" Background="White" Height="400" Margin="0,20,5,10"></Frame>
</StackPanel>

<Label x:Name="xPreviewBaselineImage" Style="{StaticResource $LabelStyle}" Grid.Row="12" Grid.Column="0" Margin="0,10,0,0" VerticalAlignment="Top" Content="Preview Baseline Image:"/>
<StackPanel x:Name="VRTPreviewBaselineImageFramePnl" Grid.Row="12" Grid.Column="1">
<Label x:Name="xPreviewBaselineImage" Style="{StaticResource $LabelStyle}" Grid.Row="13" Grid.Column="0" Margin="0,10,0,0" VerticalAlignment="Top" Content="Preview Baseline Image:"/>
<StackPanel x:Name="VRTPreviewBaselineImageFramePnl" Grid.Row="13" Grid.Column="1">
<Frame x:Name="VRTPreviewBaselineImageFrame" Background="White" Height="400" Margin="5,20,5,10"></Frame>
</StackPanel>

<Label x:Name="xVRTNote" Style="{StaticResource $LabelStyle}" Width="auto" Grid.Row="13" Grid.ColumnSpan="2" Margin="0,10,0,0" VerticalAlignment="Bottom" Content="Note - Ensure that VRT details are entered in Configurations -> External Integrations -> VRT configurations."/>
<Label x:Name="xVRTNote" Style="{StaticResource $LabelStyle}" Width="auto" Grid.Row="14" Grid.ColumnSpan="2" Margin="0,10,0,0" VerticalAlignment="Bottom" Content="Note - Ensure that VRT details are entered in Configurations -> External Integrations -> VRT configurations."/>
</Grid>
</Page>
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ public VRTComparePage(GingerCore.Actions.ActVisualTesting mAct)
}
});
GingerCore.GeneralLib.BindingHandler.ObjFieldBinding(xCreateBaselineCheckbox, CheckBox.IsCheckedProperty, mAct, nameof(mAct.CreateBaselineImage));
GingerCore.GeneralLib.BindingHandler.ObjFieldBinding(xFullPageScreenshotCheckbox, CheckBox.IsCheckedProperty, mAct, nameof(mAct.IsFullPageScreenshot));

InitLayout();

Expand Down Expand Up @@ -160,6 +161,8 @@ public void InitLayout()
xCreateBaselineCheckbox.Visibility = Visibility.Collapsed;
xPreviewImage.Visibility = Visibility.Collapsed;
xCreateBaselineNote.Visibility = Visibility.Collapsed;
xFullPageScreenshotLabel.Visibility = Visibility.Collapsed;
xFullPageScreenshotCheckbox.Visibility = Visibility.Collapsed;
xPreviewBaselineImage.Visibility = Visibility.Collapsed;
VRTPreviewBaselineImageFramePnl.Visibility = Visibility.Collapsed;
break;
Expand Down Expand Up @@ -222,6 +225,16 @@ public void InitLayout()
xBaselineImageRadioButtonPnl.Visibility = Visibility.Visible;
VRTPreviewBaselineImageFramePnl.Visibility = Visibility.Collapsed;
xPreviewBaselineImage.Visibility = Visibility.Collapsed;
if (baselineImageBy == VRTAnalyzer.eBaselineImageBy.ActiveWindow && actionBy == VRTAnalyzer.eActionBy.Window)
{
xFullPageScreenshotLabel.Visibility = Visibility.Visible;
xFullPageScreenshotCheckbox.Visibility = Visibility.Visible;
}
else
{
xFullPageScreenshotLabel.Visibility = Visibility.Collapsed;
xFullPageScreenshotCheckbox.Visibility = Visibility.Collapsed;
}
}
else
{
Expand All @@ -231,6 +244,16 @@ public void InitLayout()
VRTBaseImageFramePnl.Visibility = Visibility.Collapsed;
xBaselineImagePath.Visibility = Visibility.Collapsed;
xPreviewImage.Visibility = Visibility.Collapsed;
if (actionBy == VRTAnalyzer.eActionBy.Window)
{
xFullPageScreenshotLabel.Visibility = Visibility.Visible;
xFullPageScreenshotCheckbox.Visibility = Visibility.Visible;
}
else
{
xFullPageScreenshotLabel.Visibility = Visibility.Collapsed;
xFullPageScreenshotCheckbox.Visibility = Visibility.Collapsed;
}
}

break;
Expand Down Expand Up @@ -263,6 +286,8 @@ public void InitLayout()
xCreateBaselineCheckbox.Visibility = Visibility.Collapsed;
xPreviewImage.Visibility = Visibility.Collapsed;
xCreateBaselineNote.Visibility = Visibility.Collapsed;
xFullPageScreenshotLabel.Visibility = Visibility.Collapsed;
xFullPageScreenshotCheckbox.Visibility = Visibility.Collapsed;
xPreviewBaselineImage.Visibility = Visibility.Collapsed;
VRTPreviewBaselineImageFramePnl.Visibility = Visibility.Collapsed;
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ public bool IsFullPageScreenshot
{
get
{
bool value = true;
bool value = false;
bool.TryParse(GetOrCreateInputParam(nameof(IsFullPageScreenshot), value.ToString()).Value, out value);
return value;
Comment on lines +184 to 186
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 | ⚡ Quick win

Silent behavioral regression for pre-existing actions lacking a stored IsFullPageScreenshot param.

Changing the in-memory default from true to false affects any action that was created (and saved) before this property was populated. For those actions, GetOrCreateInputParam will synthesize the value at runtime using "False" as the default — switching them from implicitly-enabled full-page screenshots to implicitly-disabled without any explicit save/migration step.

If the property was newly introduced in an earlier PR and users could have run actions that persisted "True" to their XML, this change is safe. But if any existing saved action XML files lack this param entirely, they will silently regress.

Consider checking whether a migration note or one-time upgrade path is needed, or assert in release notes that the new default is intentionally false.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Ginger/GingerCoreNET/ActionsLib/UI/VisualTesting/ActVisualTesting.cs` around
lines 184 - 186, The change introduced a new in-memory default of false that
causes existing saved actions missing the IsFullPageScreenshot param to silently
flip behavior; update the call that synthesizes the param in ActVisualTesting
(the GetOrCreateInputParam usage around IsFullPageScreenshot) to use true as the
runtime default (i.e., pass true.ToString() or otherwise treat missing param as
true) or implement a one-time migration/upgrade path that detects absent
IsFullPageScreenshot and sets it to "True" for persisted actions so pre-existing
actions keep their original full-page behavior.

}
Expand Down
Loading