Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Aug 25, 2025

The LevelSelected event handler in ProviderBrowser.xaml.cs was calling LevelListBox.SelectedItem.ToString() without null checking, causing a crash when users attempted to deselect the selected level by holding Ctrl and clicking it.

Problem

When a user holds Ctrl and clicks the selected item in the Level ListBox:

  1. The ListBox.SelectedItem becomes null
  2. The LevelSelected event handler tries to call SelectedItem.ToString()
  3. This causes a NullReferenceException

Solution

Added defensive null checking that prevents deselection and maintains the requirement that exactly one level must always be selected:

private void LevelSelected(object sender, SelectionChangedEventArgs e)
{
    // Ensure at least one level is always selected
    if (LevelListBox.SelectedItem == null)
    {
        // If nothing is selected, reselect the previous level or default to "Verbose"
        string levelToSelect = !string.IsNullOrEmpty(m_level) ? m_level : "Verbose";
        LevelListBox.SelectedItem = levelToSelect;
        return;
    }
    
    m_level = LevelListBox.SelectedItem.ToString();
    updateDisplays();
}

This approach:

  • Prevents the NullReferenceException
  • Maintains the invariant that exactly one level is always selected
  • Reselects the user's previous choice when they attempt to deselect
  • Aligns with the maintainer's guidance that "you can't unselect the selected option"

Fixes #983.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: brianrob <6210322+brianrob@users.noreply.github.com>
Copilot AI changed the title [WIP] NullReferenceException in ProviderBrowser.LevelSelected Fix NullReferenceException in ProviderBrowser.LevelSelected when deselecting level Aug 25, 2025
Copilot AI requested a review from brianrob August 25, 2025 22:14
@brianrob
Copy link
Member

Copilot Post-Mortem:

  • Great example of a simple fix that didn't require me to debug to find the code and implement a fix.
  • Simple to test and validate since it's a small, scoped change.

@brianrob brianrob marked this pull request as ready for review August 25, 2025 22:22
Copy link
Collaborator

@marklio marklio left a comment

Choose a reason for hiding this comment

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

This looks like a reasonable fall-back should the state get unselected, but it would seem to indicate the wrong UI paradigm is being used for selection. Consider a follow-up change to the UI to make the selection more natural and UI-enforced (single-selected radio-button? drop-down? spinner? etc.)

@brianrob
Copy link
Member

Yeah, it is kind of funny that it's a multi-select control but only allows one selection. Will consider a follow-up change. Thanks for the review!

@cincuranet cincuranet merged commit 13e54b6 into main Aug 26, 2025
5 checks passed
@brianrob brianrob deleted the copilot/fix-983 branch August 26, 2025 18:07
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.

NullReferenceException in ProviderBrowser.LevelSelected

4 participants