Skip to content

Conversation

@Avid29
Copy link
Contributor

@Avid29 Avid29 commented Dec 19, 2025

Fixes #712

PR Type

What kind of change does this PR introduce?

What is the current behavior?

If a ListViewBase does not use a container, for whatever reason, the item's background will not be altered.

What is the new behavior?

If the ItemContainer is not, the item itself will be used if the type is a Control.

PR Checklist

Please check if your PR fulfills the following requirements:

Other information

Copy link

@zubinqayam zubinqayam left a 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

@Avid29 Avid29 changed the title Fix AlternateColor when using ListViewBase without container [Extensions] AlternateColor without item containers Dec 19, 2025
@Avid29 Avid29 mentioned this pull request Dec 29, 2025
11 tasks
@Arlodotexe Arlodotexe self-requested a review January 7, 2026 10:29
@Arlodotexe Arlodotexe added bug Something isn't working components::extensions Extensions for text, framework components, visual transforms, shadows, and more. labels Jan 7, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR extends the AlternateColor functionality for ListViewBase to support scenarios where no item container is used. When an item container is unavailable, the extension now falls back to using the item itself if it's a Control.

Key changes:

  • Added fallback logic to use the item as a Control when ItemContainer is unavailable
  • Refactored code across multiple ListViewExtensions files for consistency and modern C# patterns
  • Consolidated event cleanup logic with uniquely named unload handlers

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
ListViewExtensions.AlternateRows.cs Added fallback logic to use args.Item when args.ItemContainer is null, refactored to use modern C# patterns, renamed methods/variables for clarity
ListViewExtensions.Command.cs Refactored to use expression-bodied members and modern pattern matching for consistency
ListViewExtensions.StretchItemContainer.cs Implemented previously missing event handlers, changed return type to nullable, refactored for consistency

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

var itemContainer = args.ItemContainer as Control;
SetItemContainerBackground(sender, itemContainer, args.ItemIndex);
}
_trackedListViews[listViewBase.Items] = listViewBase;
Copy link

Copilot AI Jan 10, 2026

Choose a reason for hiding this comment

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

The _trackedListViews dictionary is being updated unconditionally on line 67, even when AlternateColor is being removed (set to null). This means the dictionary will contain entries for ListViews that no longer use AlternateColor, which is a memory leak. The entry should only be added when GetAlternateColor(listViewBase) is not null, and should be removed when it is null.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think there's no actual memory leak because the item is removed when the ListViewBase is unloaded, but it doesn't hurt to address it earlier

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working components::extensions Extensions for text, framework components, visual transforms, shadows, and more.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ListViewExtensions.AlternateColor doesn't work with ListView.Items with ListViewItem

4 participants