-
Notifications
You must be signed in to change notification settings - Fork 123
[Extensions] AlternateColor without item containers #763
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
base: main
Are you sure you want to change the base?
Conversation
zubinqayam
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avid29:fix/alt-color6
| private static void OnListViewBaseUnloaded(object sender, RoutedEventArgs e) | ||
| { | ||
| if (sender is ListViewBase listViewBase) | ||
| { | ||
| _itemsForList.Remove(listViewBase.Items); | ||
|
|
||
| listViewBase.ContainerContentChanging -= ItemContainerStretchDirectionChanging; | ||
| listViewBase.ContainerContentChanging -= ItemTemplateContainerContentChanging; | ||
| listViewBase.ContainerContentChanging -= ColorContainerContentChanging; | ||
| listViewBase.Items.VectorChanged -= ColorItemsVectorChanged; | ||
| listViewBase.Unloaded -= OnListViewBaseUnloaded; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you are already organizing code in this PR, this would be a good time to move OnListViewBaseUnloaded into its own file instead of keeping it in ListViewExtensions.AlternateRows.cs. That method handles unsubscribing from several features, so separating it would improve clarity and maintainability.
ListViewExtensions.cs
namespace CommunityToolkit.WinUI;
public static partial class ListViewExtensions
{
private static void OnListViewBaseUnloaded(object sender, RoutedEventArgs e)
{
if (sender is not ListViewBase listViewBase)
return;
_itemsForList.Remove(listViewBase.Items);
listViewBase.ContainerContentChanging -= ItemContainerStretchDirectionChanging;
listViewBase.ContainerContentChanging -= ItemTemplateContainerContentChanging;
listViewBase.ContainerContentChanging -= ColorContainerContentChanging;
listViewBase.Items.VectorChanged -= ColorItemsVectorChanged;
listViewBase.Unloaded -= OnListViewBaseUnloaded;
}
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the reminder. I had noticed this, but hadn't yet written an alternative because I was torn between your solution, and splitting the method for the different attached property types.
ListViewExtensions.AlternateRows.cs
private static void OnListViewBaseUnloaded_AltRow(object sender, RoutedEventArgs e)
{
if (sender is not ListViewBase listViewBase)
return;
// Untrack the list view
_trackedListViews.Remove(listViewBase.Items);
// Unsubscribe from events
listViewBase.ContainerContentChanging -= ItemTemplateContainerContentChanging;
listViewBase.ContainerContentChanging -= ColorContainerContentChanging;
listViewBase.Items.VectorChanged -= ColorItemsVectorChanged;
listViewBase.Unloaded -= OnListViewBaseUnloaded_AltRow;
}ListViewExtensions.StretchItemContainer.cs
private static void OnListViewBaseUnloaded_StretchDirection(object sender, RoutedEventArgs e)
{
if (sender is not ListViewBase listViewBase)
return;
// Unsubscribe from events
listViewBase.ContainerContentChanging -= ItemContainerStretchDirectionChanging;
listViewBase.Unloaded -= OnListViewBaseUnloaded_StretchDirection;
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One vote for your solution then. We can still keep each feature on its own file unless we have other shared logic. 🙂
Fixes #712
PR Type
What kind of change does this PR introduce?
What is the current behavior?
If a
ListViewBasedoes not use a container, for whatever reason, the item's background will not be altered.What is the new behavior?
If the
ItemContaineris not, the item itself will be used if the type is aControl.PR Checklist
Please check if your PR fulfills the following requirements:
Other information