Skip to content

Fix enum selection issues #15

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.

Sign up for GitHub

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

Merged
BorisGerretzen merged 1 commit into from
Dec 5, 2025
Merged

Fix enum selection issues #15

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
153 changes: 153 additions & 0 deletions src/SimpleBlazorMultiselect.Tests/EnumTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
using AngleSharp.Html.Dom;
using Bunit;
using FluentAssertions;
using Microsoft.AspNetCore.Components;
using Xunit;

namespace SimpleBlazorMultiselect.Tests;

public class EnumTests : BaseTest
{
[Fact]
public void Component_WithEmptySet_ShouldShowNoChecks()
{
var options = Enum.GetValues<TestEnum>().ToList();
var selectedItems = new HashSet<TestEnum>();

var component = RenderComponent<SimpleMultiselect<TestEnum>>(parameters => parameters
.Add(p => p.Options, options)
.Add(p => p.SelectedOptions, selectedItems)
.Add(p => p.StringSelector, item => item.ToString())
.Add(p => p.DefaultText, "empty")
.Add(p => p.SelectedOptionsChanged, EventCallback.Factory.Create<HashSet<TestEnum>>(this, newSelection => { selectedItems = newSelection; })));

var button = component.Find("button");
button.TextContent.Should().Be("empty");
button.Click();

// Check if all options are unchecked
var dropdownItems = component.FindAll(".dropdown-item");
foreach (var item in dropdownItems)
{
var checkbox = item.QuerySelector("input[type='checkbox']") as IHtmlInputElement;
checkbox.Should().NotBeNull();
checkbox.IsChecked.Should().BeFalse();
}
}

[Fact]
public void Component_WithPrefilledOptions_ShouldShowCorrectCheck()
{
var options = Enum.GetValues<TestEnum>().ToList();
var selectedItems = new HashSet<TestEnum>
{
TestEnum.OptionB
};

var component = RenderComponent<SimpleMultiselect<TestEnum>>(parameters => parameters
.Add(p => p.Options, options)
.Add(p => p.SelectedOptions, selectedItems)
.Add(p => p.StringSelector, item => item.ToString())
.Add(p => p.DefaultText, "empty")
.Add(p => p.SelectedOptionsChanged, EventCallback.Factory.Create<HashSet<TestEnum>>(this, newSelection => { selectedItems = newSelection; })));

var button = component.Find("button");
button.TextContent.Should().Be(nameof(TestEnum.OptionB));
button.Click();

// Check if all options are unchecked except OptionB
var dropdownItems = component.FindAll(".dropdown-item");
foreach (var item in dropdownItems)
{
var checkbox = item.QuerySelector("input[type='checkbox']") as IHtmlInputElement;
checkbox.Should().NotBeNull();

var optionText = item.TextContent.Trim();
if (optionText == nameof(TestEnum.OptionB))
{
checkbox.Should().NotBeNull();

Copilot AI Dec 5, 2025

Copy link

Choose a reason for hiding this comment

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

Redundant null check. The checkbox is already verified to be non-null on line 63. This duplicate assertion on line 68 is unnecessary and should be removed.

Consider removing this line since it's already checked above.

Suggested change
checkbox.Should().NotBeNull();

Copilot uses AI. Check for mistakes.
checkbox.IsChecked.Should().BeTrue();
}
else
{
checkbox.IsChecked.Should().BeFalse();
}
}
}

[Fact]
public void Component_CanUncheckPrefilled()
{
var options = Enum.GetValues<TestEnum>().ToList();
var selectedItems = new HashSet<TestEnum>
{
TestEnum.OptionB
};

var component = RenderComponent<SimpleMultiselect<TestEnum>>(parameters => parameters
.Add(p => p.Options, options)
.Add(p => p.SelectedOptions, selectedItems)
.Add(p => p.StringSelector, item => item.ToString())
.Add(p => p.DefaultText, "empty")
.Add(p => p.SelectedOptionsChanged, EventCallback.Factory.Create<HashSet<TestEnum>>(this, newSelection => { selectedItems = newSelection; })));

var button = component.Find("button");
button.TextContent.Should().Be(nameof(TestEnum.OptionB));
button.Click();

// Check if option B is checked
var dropdownItems = component.FindAll(".dropdown-item");
var optionBItem = dropdownItems.First(item => item.TextContent.Trim() == nameof(TestEnum.OptionB));
var optionBCheckbox = optionBItem.QuerySelector("input[type='checkbox']") as IHtmlInputElement;
optionBCheckbox.Should().NotBeNull();
optionBCheckbox.IsChecked.Should().BeTrue();

// Click to uncheck OptionB
optionBItem.Click();

// After clicking, OptionB should be deselected
selectedItems.Should().BeEmpty();
button = component.Find("button");
button.TextContent.Should().Be("empty");
}

[Fact]
public void Component_CanCheckUncheckedOption()
{
var options = Enum.GetValues<TestEnum>().ToList();
var selectedItems = new HashSet<TestEnum>();

var component = RenderComponent<SimpleMultiselect<TestEnum>>(parameters => parameters
.Add(p => p.Options, options)
.Add(p => p.SelectedOptions, selectedItems)
.Add(p => p.StringSelector, item => item.ToString())
.Add(p => p.DefaultText, "empty")
.Add(p => p.SelectedOptionsChanged, EventCallback.Factory.Create<HashSet<TestEnum>>(this, newSelection => { selectedItems = newSelection; })));

var button = component.Find("button");
button.TextContent.Should().Be("empty");
button.Click();

// Check if all options are unchecked
var dropdownItems = component.FindAll(".dropdown-item");
var optionCItem = dropdownItems.First(item => item.TextContent.Trim() == nameof(TestEnum.OptionC));
var optionCCheckbox = optionCItem.QuerySelector("input[type='checkbox']") as IHtmlInputElement;
optionCCheckbox.Should().NotBeNull();
optionCCheckbox.IsChecked.Should().BeFalse();

// Click to check OptionC
optionCItem.Click();

// After clicking, OptionC should be selected
selectedItems.Should().Contain(TestEnum.OptionC);
button = component.Find("button");
button.TextContent.Should().Be(nameof(TestEnum.OptionC));
}

private enum TestEnum
{
OptionA,
OptionB,
OptionC
}
}
63 changes: 40 additions & 23 deletions src/SimpleBlazorMultiselect/SimpleMultiselect.razor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Microsoft.AspNetCore.Components;
using System.Diagnostics.CodeAnalysis;
using Microsoft.AspNetCore.Components;

namespace SimpleBlazorMultiselect;

Expand Down Expand Up @@ -101,25 +102,30 @@ private bool DefaultFilterPredicate(TItem item, string filterString)
/// </summary>
[CascadingParameter(Name = "Standalone")]
public bool Standalone { get; set; }

/// <summary>
/// If true, the selection of options will be matched by reference instead of by string representation.
/// </summary>
[Parameter]
public bool MatchByReference { get; set; }

private async Task ToggleOption(TItem option)
{
var newSelected = new HashSet<TItem>(SelectedOptions);

var existingSelected = FindSelected(option);
var wasSelected = existingSelected != null && newSelected.Remove(existingSelected);

var wasSelected = false;
if (TryFindSelected(option, out var existing))
{
wasSelected = newSelected.Remove(existing);
}

if (!wasSelected)
{
if (!IsMultiSelect)
{
newSelected.Clear();
}

newSelected.Add(option);
}

Expand All @@ -134,45 +140,56 @@ private async Task ToggleOption(TItem option)

private bool IsOptionSelected(TItem option)
{
return FindSelected(option) != null;
return TryFindSelected(option, out _);
}

/// <summary>
/// Helper function to find the selected option that matches the given options.
/// Options might not be the same reference.
/// </summary>
private TItem? FindSelected(TItem option)

private bool TryFindSelected(TItem option, [NotNullWhen(true)] out TItem existing)
{
if (SelectedOptions.Count == 0)
{
existing = default!;
return false;
}

if (MatchByReference)
{
SelectedOptions.TryGetValue(option, out var existing);
return existing;
return SelectedOptions.TryGetValue(option, out existing!);

Copilot AI Dec 5, 2025

Copy link

Choose a reason for hiding this comment

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

[nitpick] The null-forgiving operator ! is redundant here. Since SelectedOptions.TryGetValue already returns a non-nullable TItem through the out parameter when it returns true, and the [NotNullWhen(true)] attribute on the method signature already indicates that existing will be non-null when the method returns true, the null-forgiving operator is unnecessary.

Consider removing it:

return SelectedOptions.TryGetValue(option, out existing);
Suggested change
return SelectedOptions.TryGetValue(option, out existing!);
return SelectedOptions.TryGetValue(option, out existing);

Copilot uses AI. Check for mistakes.
}

var optionString = StringSelector(option);
return SelectedOptions.FirstOrDefault(selected => StringSelector(selected) == optionString);
foreach (var selected in SelectedOptions)
{
if (StringSelector(selected) == optionString)
{
existing = selected!;

Copilot AI Dec 5, 2025

Copy link

Choose a reason for hiding this comment

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

[nitpick] The null-forgiving operator ! is redundant here. Since the code only reaches this line when a match is found (the string selector values are equal), and we're assigning a value that comes from iterating over SelectedOptions (which contains non-null items), the null-forgiving operator serves no purpose.

Consider removing it:

existing = selected;
Suggested change
existing = selected!;
existing = selected;

Copilot uses AI. Check for mistakes.
return true;
}
}
Comment on lines +160 to +167

Copilot AI Dec 5, 2025

Copy link

Choose a reason for hiding this comment

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

This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.

Copilot uses AI. Check for mistakes.

existing = default!;
return false;
}

private List<TItem>? _filteredOptionsCache;
private List<TItem>? _prevOptions;
private Func<TItem, string, bool>? _prevFilterPredicate;
private string? _prevFilterText;
private bool _prevCanFilter;

private List<TItem> FilteredOptions()
{
if(_prevCanFilter == CanFilter && _prevFilterPredicate == FilterPredicate && _prevFilterText == _filterText && _prevOptions == Options)
if (_prevCanFilter == CanFilter && _prevFilterPredicate == FilterPredicate && _prevFilterText == _filterText && _prevOptions == Options)
{
return _filteredOptionsCache ?? Options;
}

_prevOptions = Options;
_prevCanFilter = CanFilter;
_prevFilterPredicate = FilterPredicate;
_prevFilterText = _filterText;

_filteredOptionsCache = [];
if(!CanFilter || string.IsNullOrWhiteSpace(_filterText))
if (!CanFilter || string.IsNullOrWhiteSpace(_filterText))
{
_filteredOptionsCache.AddRange(Options);
}
Expand All @@ -181,7 +198,7 @@ private List<TItem> FilteredOptions()
var predicate = FilterPredicate ?? DefaultFilterPredicate;
_filteredOptionsCache.AddRange(Options.Where(option => predicate(option, _filterText)));
}

return _filteredOptionsCache;
}
}