Skip to content

Extract InfiniteScrollList business logic into InfiniteScrollListViewModel#7240

Draft
jebriede wants to merge 1 commit intodevfrom
dev-jebriede-RefactorInfiniteScrollList
Draft

Extract InfiniteScrollList business logic into InfiniteScrollListViewModel#7240
jebriede wants to merge 1 commit intodevfrom
dev-jebriede-RefactorInfiniteScrollList

Conversation

@jebriede
Copy link
Copy Markdown
Contributor

@jebriede jebriede commented Mar 27, 2026

Bug

Fixes: https://github.com/NuGet/Client.Engineering/issues/3700

Description

Extract InfiniteScrollList business logic into InfiniteScrollListViewModel

Summary

Extracts all business logic from InfiniteScrollList.xaml.cs (860 lines) into a new InfiniteScrollListViewModel (714 lines), reducing the codebehind to ~200 lines of WPF-specific plumbing. The ViewModel has no WPF type references and is fully unit-testable.

This is the first step in addressing https://github.com/NuGet/Client.Engineering/issues/2539 (business logic in codebehind). Further improvements are tracked in https://github.com/NuGet/Client.Engineering/issues/3702

Design decisions

  • ViewModel property naming follows state semantics, not UI semantics: HasUpdatablePackages (not IsUpdateContainerVisible), HasSelectedPackages (not IsUpdateEnabled), SelectionState (not SelectAllChecked), IsUpdateMode (not CheckBoxesEnabled)
  • Action delegates for status bar: LoadingStatusBar.Reset()/SetError()/SetCancelled() replace their own DataContext with new objects — fundamentally imperative, can't be XAML bindings. Delegates are wired in the constructor (not Loaded) because PackageManagerControl calls LoadItemsAsync during initialization
  • TopLevelPackageCount/TransitivePackageCount stay on the codebehind: They use CollectionViewGroup.ItemCount which reflects filtered counts. LINQ-based counts on the ViewModel would show unfiltered totals — wrong when vulnerability filter is active
  • PackageManagerControl accesses _packageList.ViewModel directly: Eliminates passthrough properties on the control. Both types are internal to the same assembly
  • Shared converters: Uses existing BooleanToVisibilityConverter and BooleanToHiddenVisibilityConverter from Resources.xaml instead of defining new ones

Testing

New unit tests in InfiniteScrollListViewModelTests.cs covering:

  • Constructor validation and default state
  • LoadItemsAsync parameter validation and cancellation
  • Loading flow: empty results, populated results, clearing on reload
  • Selection: select all, deselect, count tracking, tri-state checkbox state
  • Filtering: vulnerability filter on/off, loading indicator visibility
  • Grouping: HasTransitiveItems detection
  • ResolveSelectedItem: by ID, not found, null
  • Status bar: ShouldShowStatusBar for cancelled/error/ready/null states
  • Collection management: clear, refresh, append

Pre-existing issue found

Group header package counts (TopLevelPackageCount/TransitivePackageCount) intermittently show 0 when the vulnerability filter is active. This is a pre-existing bug on dev — the plain CLR getters have no change notification mechanism, and CollectionViewGroup may not be fully rebuilt when the binding re-evaluates. Tracked separately.

PR Checklist

  • Meaningful title, helpful description and a linked NuGet/Home issue
  • Added tests
  • Link to an issue or pull request to update docs if this PR changes settings, environment variables, new feature, etc.

…Model, establishing MVVM pattern with XAML bindings and unit tests
@jebriede jebriede requested a review from a team as a code owner March 27, 2026 23:42
@jebriede jebriede marked this pull request as draft March 27, 2026 23:43
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

This PR contains changes to XAML files. Verify that you've completed the following tasks:

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant