Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ucList design changes #3435

Merged
merged 3 commits into from
Jan 10, 2024
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
2 changes: 1 addition & 1 deletion Ginger/Ginger/AutomatePageLib/ExecutionSummaryPage.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:lvc="clr-namespace:LiveCharts.Wpf;assembly=LiveCharts.Wpf"
x:Class="Ginger.BusinessFlowWindows.ExecutionSummaryPage"
Height="800" Width="1000">
Height="600" Width="600">

<Page.Resources>
<x:ArrayExtension Type="{x:Type Brush}" x:Key="brushes">
Expand Down
4 changes: 2 additions & 2 deletions Ginger/Ginger/AutomatePageLib/NewAutomatePage.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@
</DockPanel>
<Grid x:Name="xActivitiesDataGrid" DockPanel.Dock="Top" Visibility="Visible" Background="Transparent">
<Grid.ColumnDefinitions>
<ColumnDefinition Width="650"/>
<ColumnDefinition Width="550"/>
<ColumnDefinition Width="300*"/>
<ColumnDefinition x:Name="xAddActionsColumn" Width="5" MinWidth="5" MaxWidth="1300"/>
</Grid.ColumnDefinitions>
Expand All @@ -327,7 +327,7 @@
<Frame x:Name="xActivitiesListFrame" Content="Activities List Frame" Margin="10,0,10,0" NavigationUIVisibility="Hidden" />
</Border>

<GridSplitter x:Name="xActivityDetailsSectionSpliter" ResizeDirection="Columns" Grid.Column="1" Width="1" HorizontalAlignment="Left" Background="DarkGray" VerticalAlignment="Stretch" IsEnabled="True" Margin="0,0,0,0"/>
<GridSplitter x:Name="xActivityDetailsSectionSpliter" ResizeDirection="Columns" Grid.Column="1" Width="1" HorizontalAlignment="Left" Background="DarkGray" VerticalAlignment="Stretch" IsEnabled="True" Margin="0,0,0,10"/>

<StackPanel x:Name="xCurrentActivityLoadingIconPnl" Grid.Column="1" HorizontalAlignment="Center" Visibility="Collapsed" VerticalAlignment="Center" >
<UserControls:ImageMakerControl x:Name="xCurrentActivityLoadingIcon" ImageType="Processing" HorizontalAlignment="Center" Width="80" Height="80" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1574,7 +1574,7 @@
</Style>

<Style TargetType="ListViewItem" x:Key="$ListViewItemStyle">
<Setter Property="Margin" Value="0 5 0 5"/>
<Setter Property="Margin" Value="0 4 0 4"/>
<Setter Property="Template">
<Setter.Value>
<ControlTemplate TargetType="{x:Type ListViewItem}">
Expand Down
74 changes: 49 additions & 25 deletions Ginger/Ginger/UserControlsLib/UCListView/UcListViewItem.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -93,34 +93,58 @@

<usercontrols:ImageMakerControl x:Name="xItemIcon" Grid.RowSpan="2" Grid.Column="3" ImageToolTip="" ImageType="Null" ImageForeground="{StaticResource $BackgroundColor_DarkGray}" SetAsFontImageWithSize="17" Height="20" Width="20" HorizontalAlignment="Center" Margin="0,0,0,0"/>

<Grid x:Name="xDetailsGrid" Grid.Row="0" Grid.Column="4" >
<Grid x:Name="xDetailsGrid" Grid.Row="0" Grid.Column="4" HorizontalAlignment="Stretch" >
<Grid.ColumnDefinitions>
<ColumnDefinition Width="100*" MinWidth="100"/>
<ColumnDefinition x:Name="xItemNameColumn" Width="100*" MinWidth="100"/>
<ColumnDefinition x:Name="xItemNotificationsClm" Width="0"/>
<ColumnDefinition x:Name="xItemStatusClm" Width="25"/>
</Grid.ColumnDefinitions>
<TextBlock x:Name="xItemNameTxtBlock" Grid.Column="0" Text="Title">
<TextBlock.Style>
<Style TargetType="TextBlock">
<Setter Property="HorizontalAlignment" Value="Stretch"/>
<Setter Property="VerticalAlignment" Value="Center"/>
<Setter Property="FontSize" Value="15"/>
<Setter Property="FontWeight" Value="Normal"/>
<Setter Property="Margin" Value="2,0,0,0"/>
<Setter Property="TextTrimming" Value="CharacterEllipsis"/>
<Style.Triggers>
<DataTrigger Binding="{Binding RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type ListViewItem}}, Path=IsSelected, Mode=OneWay, UpdateSourceTrigger=PropertyChanged}" Value="True">
<Setter Property="FontWeight" Value="Bold"/>
<Setter Property="Foreground" Value="{StaticResource $BackgroundColor_Black}"/>
</DataTrigger>
<DataTrigger Binding="{Binding RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type ListViewItem}}, Path=IsSelected, Mode=OneWay, UpdateSourceTrigger=PropertyChanged}" Value="False">
<Setter Property="FontWeight" Value="Normal"/>
<Setter Property="Foreground" Value="{StaticResource $BackgroundColor_Black}"/>
</DataTrigger>
</Style.Triggers>
</Style>
</TextBlock.Style>
</TextBlock>
<StackPanel Grid.Column="0" Orientation="Horizontal" >
<TextBlock x:Name="xItemNameTxtBlock" Text="Title" >
<TextBlock.Style>
<Style TargetType="TextBlock">
<Setter Property="HorizontalAlignment" Value="Left"/>
<Setter Property="VerticalAlignment" Value="Center"/>
<Setter Property="FontSize" Value="15"/>
<Setter Property="FontWeight" Value="Normal"/>
<Setter Property="Margin" Value="2,0,0,0"/>
<Setter Property="TextTrimming" Value="CharacterEllipsis"/>
<Style.Triggers>
<DataTrigger Binding="{Binding RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type ListViewItem}}, Path=IsSelected, Mode=OneWay, UpdateSourceTrigger=PropertyChanged}" Value="True">
<Setter Property="FontWeight" Value="Bold"/>
<Setter Property="Foreground" Value="{StaticResource $BackgroundColor_Black}"/>
</DataTrigger>
<DataTrigger Binding="{Binding RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type ListViewItem}}, Path=IsSelected, Mode=OneWay, UpdateSourceTrigger=PropertyChanged}" Value="False">
<Setter Property="FontWeight" Value="Normal"/>
<Setter Property="Foreground" Value="{StaticResource $BackgroundColor_Black}"/>
</DataTrigger>
</Style.Triggers>
</Style>
</TextBlock.Style>
</TextBlock>
<TextBlock x:Name="xItemExtraInfoTxtBlock" Text="[ExtraInfo]" MaxWidth="100" >
<TextBlock.Style>
<Style TargetType="TextBlock">
<Setter Property="HorizontalAlignment" Value="Left"/>
<Setter Property="VerticalAlignment" Value="Center"/>
<Setter Property="FontSize" Value="15"/>
<Setter Property="FontWeight" Value="Normal"/>
<Setter Property="Margin" Value="2,0,0,0"/>
<Setter Property="TextTrimming" Value="CharacterEllipsis"/>
<Style.Triggers>
<DataTrigger Binding="{Binding RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type ListViewItem}}, Path=IsSelected, Mode=OneWay, UpdateSourceTrigger=PropertyChanged}" Value="True">
<Setter Property="FontWeight" Value="Bold"/>
<Setter Property="Foreground" Value="{StaticResource $BackgroundColor_Black}"/>
</DataTrigger>
<DataTrigger Binding="{Binding RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type ListViewItem}}, Path=IsSelected, Mode=OneWay, UpdateSourceTrigger=PropertyChanged}" Value="False">
<Setter Property="FontWeight" Value="Normal"/>
<Setter Property="Foreground" Value="{StaticResource $BackgroundColor_Black}"/>
</DataTrigger>
</Style.Triggers>
</Style>
</TextBlock.Style>
</TextBlock>
</StackPanel>
<StackPanel x:Name="xItemNotificationsPnl" Orientation="Horizontal" Grid.Column="1" HorizontalAlignment="Right" Margin="10,0,0,0" />
<uclistview:UcItemExecutionStatus x:Name="xItemStatusImage" StatusViewMode="Image" StatusSize="16" VerticalAlignment="Center" Grid.Column="2"/>
</Grid>
Expand All @@ -132,7 +156,7 @@
<!--to set width per need like for notifications-->
</Grid.ColumnDefinitions>

<TextBlock x:Name="xItemDescriptionTxtBlock" Grid.Column="0" Text="Description" FontSize="11" Margin="2 0 0 0" Foreground="{StaticResource $BackgroundColor_DarkGray}" FontStyle="Normal" FontWeight="Normal" TextTrimming="CharacterEllipsis" TextWrapping="NoWrap" HorizontalAlignment="Stretch" VerticalAlignment="Center" />
<TextBlock x:Name="xItemDescriptionTxtBlock" Grid.Column="0" Text="Description" FontSize="11" Margin="2 0 0 0" Foreground="{StaticResource $BackgroundColor_DarkGray}" FontStyle="Normal" FontWeight="Normal" TextTrimming="CharacterEllipsis" TextWrapping="NoWrap" HorizontalAlignment="left" VerticalAlignment="Center" />

<DockPanel x:Name="xItemOperationsMainPnl" Grid.Column="1" HorizontalAlignment="Right" Visibility="Collapsed">
<StackPanel x:Name="xItemExecutionOperationsPnl" Orientation="Horizontal" DockPanel.Dock="Right"/>
Expand Down
27 changes: 24 additions & 3 deletions Ginger/Ginger/UserControlsLib/UCListView/UcListViewItem.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,12 @@ private void SetItemFullName()
{
try
{
AlignGridandTextWidth();
xDetailsGrid.SizeChanged -= XDetailsGrid_SizeChanged;
xDetailsGrid.SizeChanged += XDetailsGrid_SizeChanged;
Comment on lines +701 to +702
Copy link
Contributor

Choose a reason for hiding this comment

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

The event handler XDetailsGrid_SizeChanged is being unsubscribed and then immediately resubscribed. This is redundant and can be removed if the event handler is not being dynamically changed elsewhere in the code.

- xDetailsGrid.SizeChanged -= XDetailsGrid_SizeChanged;
- xDetailsGrid.SizeChanged += XDetailsGrid_SizeChanged;

If the intention is to ensure that the event handler is only subscribed once, consider checking if the event is already subscribed before adding it, or ensure that the event is unsubscribed when the control is disposed of to prevent memory leaks.


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.

Suggested change
xDetailsGrid.SizeChanged -= XDetailsGrid_SizeChanged;
xDetailsGrid.SizeChanged += XDetailsGrid_SizeChanged;

xItemNameTxtBlock.Text = string.Empty;
xItemExtraInfoTxtBlock.Text = string.Empty;
xItemExtraInfoTxtBlock.Visibility = Visibility.Collapsed;
string fullname = string.Empty;
if (!string.IsNullOrEmpty(mItemNameField))
{
Comment on lines 697 to 708
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [700-753]

The method SetItemFullName has been modified to include additional logic for setting the visibility and text of xItemExtraInfoTxtBlock. The changes seem to correctly implement the desired functionality, but there are a few points to consider:

  1. The use of direct property access through reflection (Item.GetType().GetProperty(mItemNameField).GetValue(Item)) is not type-safe and may lead to runtime errors if property names change. Consider using a safer approach, such as a method or a dictionary to map property names to delegates.
  2. The visibility of xItemExtraInfoTxtBlock is being set multiple times which could be simplified.
  3. The try-catch block is good for exception handling, but it's important to ensure that the catch block does not swallow exceptions that should be handled or logged at a higher level.

Expand All @@ -709,7 +714,6 @@ private void SetItemFullName()
FontSize = 15,
Text = name.ToString()
});

fullname += name;
}
}
Expand All @@ -718,7 +722,8 @@ private void SetItemFullName()
bool isMandatory = (bool)Item.GetType().GetProperty(mItemMandatoryField).GetValue(Item);
if (isMandatory)
{
xItemNameTxtBlock.Inlines.Add(new System.Windows.Documents.Run
xItemExtraInfoTxtBlock.Visibility = Visibility.Visible;
xItemExtraInfoTxtBlock.Inlines.Add(new System.Windows.Documents.Run
{
FontSize = 18,
Text = "*",
Expand All @@ -733,7 +738,8 @@ private void SetItemFullName()
Object extension = Item.GetType().GetProperty(mItemNameExtentionField).GetValue(Item);
if (extension != null)
{
xItemNameTxtBlock.Inlines.Add(new System.Windows.Documents.Run
xItemExtraInfoTxtBlock.Visibility = Visibility.Visible;
xItemExtraInfoTxtBlock.Inlines.Add(new System.Windows.Documents.Run
{
FontSize = 11,
Text = string.Format(" [{0}]", extension.ToString())
Expand All @@ -744,6 +750,7 @@ private void SetItemFullName()
}

xItemNameTxtBlock.ToolTip = fullname;
xItemExtraInfoTxtBlock.ToolTip = fullname;
}
catch (Exception ex)
{
Expand All @@ -753,6 +760,20 @@ private void SetItemFullName()
});
}

private void AlignGridandTextWidth()
{
if (xItemNameColumn.ActualWidth > 200)
{
xItemNameTxtBlock.MaxWidth = xItemNameColumn.ActualWidth - 100;
xItemDescriptionTxtBlock.MaxWidth = xItemNameTxtBlock.MaxWidth;
}
}

private void XDetailsGrid_SizeChanged(object sender, SizeChangedEventArgs e)
{
AlignGridandTextWidth();
}

private void SetItemDescription()
{
this.Dispatcher.Invoke(() =>
Comment on lines 760 to 779
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [700-770]

The method AlignGridandTextWidth has been added to dynamically adjust the MaxWidth of text blocks based on the actual width of a column. This is a good approach to ensure responsive design. However, the magic number 200 and 100 used in the condition and width calculation should be replaced with named constants for better maintainability and readability.

- if (xItemNameColumn.ActualWidth > 200)
+ const int minWidthThreshold = 200; // This value should be adjusted to the appropriate threshold
+ const int widthOffset = 100; // This value should be adjusted to the appropriate offset
+ if (xItemNameColumn.ActualWidth > minWidthThreshold)
...
- xItemNameTxtBlock.MaxWidth = xItemNameColumn.ActualWidth - 100;
+ xItemNameTxtBlock.MaxWidth = xItemNameColumn.ActualWidth - widthOffset;

Expand Down
Loading