feat(MultiFilter): add StringComparison parameter #6010
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Reviewer's GuideThis pull request implements customizable string comparison in the Sequence Diagram: Filtering Logic in MultiFilter.OnSearchValueChangedsequenceDiagram
actor User
participant MF as "MultiFilter Component"
participant Item as "SelectedItem (from internal list)"
User->>MF: Inputs/modifies search text (searchText)
activate MF
MF->>MF: Calls OnSearchValueChanged(searchText)
alt searchText is not empty
MF->>MF: Iterates over its internal list of items (_source)
loop For each item in _source
MF->>Item: Checks if item.Text.Contains(searchText, StringComparison)
note right of Item: Filtering uses the configured StringComparison property
end
MF->>MF: Updates its display list (_items) with filtered results
else searchText is empty
MF->>MF: Updates its display list (_items) with all original items from _source
end
deactivate MF
Class Diagram: Update to MultiFilter ComponentclassDiagram
class MultiFilter {
+StringComparison StringComparison
+OnSearchValueChanged(string? val) Task
%% Other existing members are not shown for brevity %%
}
note for MultiFilter "Added: StringComparison property (default: StringComparison.OrdinalIgnoreCase).\nOnSearchValueChanged: Method logic updated to use the StringComparison property during text filtering."
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the filtering functionality of the MultiFilter component by adding a customizable StringComparison parameter and refactoring relevant filtering logic, including updating test cases and simplifying filtered item returns.
- Introduces a new StringComparison parameter in MultiFilter with a default value of StringComparison.OrdinalIgnoreCase.
- Updates filtering logic in MultiFilter to use the new parameter.
- Refactors the TablesFilter component and its test to use the C# 12 collection expression (spread operator) for returning filtered items.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/UnitTest/Components/TableFilterTest.cs | Updates attribute ordering in the test for MultiFilter to include the new StringComparison parameter. |
| src/BootstrapBlazor/Components/Filters/MultiFilter.razor.cs | Adds the StringComparison parameter and updates filtering logic to use it. |
| src/BootstrapBlazor.Server/Components/Samples/Table/TablesFilter.razor.cs | Simplifies the return of filtered items via the C# 12 spread operator. |
Comments suppressed due to low confidence (2)
src/BootstrapBlazor/Components/Filters/MultiFilter.razor.cs:235
- Consider verifying that the C# 12 collection expression syntax used here produces a list type compatible with _items in all intended target frameworks.
_items = [.. _source.Where(i => i.Text.Contains(_searchText, StringComparison))];
src/BootstrapBlazor.Server/Components/Samples/Table/TablesFilter.razor.cs:49
- Ensure that the use of the collection expression reliably returns the expected list type across all target environments, matching the method's return type.
return [.. Items.Select(i => new SelectedItem(i.Address!, i.Address!)).DistinctBy(i => i.Value)];
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6010 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 670 670
Lines 30645 30646 +1
Branches 4361 4361
=========================================
+ Hits 30645 30646 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey @ArgoZhang - I've reviewed your changes - here's some feedback:
- Consider extending
StringComparisonparameter support to other relevant filter components for API consistency across the library. - Evaluate if
CurrentCultureIgnoreCasemight be a more suitable default for theStringComparisonparameter in typical UI filtering scenarios, instead ofOrdinalIgnoreCase.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟡 Testing: 1 issue found
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| b.AddAttribute(1, nameof(MultiFilter.ShowSearch), true); | ||
| b.AddAttribute(2, nameof(MultiFilter.OnGetItemsAsync), () => Task.FromResult(new List<SelectedItem>() { new("test1", "test1") })); | ||
| b.AddAttribute(1, nameof(MultiFilter.AlwaysTriggerGetItems), true); | ||
| b.AddAttribute(2, nameof(MultiFilter.StringComparison), StringComparison.OrdinalIgnoreCase); |
Contributor
There was a problem hiding this comment.
suggestion (testing): Consider adding tests to verify different StringComparison behaviors.
Add tests covering each StringComparison mode. For each (e.g. Ordinal, CurrentCulture, OrdinalIgnoreCase), use items with varied casing (e.g., “Apple”, “apple”, “APPLE”), perform a search, and assert that results match the expected case-sensitive or case-insensitive behavior.
Suggested implementation:
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Xunit;
public class TableFilterStringComparisonTests
{
[Fact]
public async Task Test_StringComparison_Ordinal()
{
// Arrange: Ordinal comparison is case-sensitive so only the exact match should be returned.
var filter = new MultiFilter
{
StringComparison = StringComparison.Ordinal
};
var items = new List<SelectedItem>
{
new SelectedItem("Apple", "Apple"),
new SelectedItem("apple", "apple"),
new SelectedItem("APPLE", "APPLE")
};
// Act: Perform search using "Apple" as query.
var result = await filter.GetItemsAsync("Apple", items);
// Assert: Only "Apple" should match.
Assert.Single(result);
Assert.Equal("Apple", result.First().Text);
}
[Fact]
public async Task Test_StringComparison_OrdinalIgnoreCase()
{
// Arrange: OrdinalIgnoreCase comparison returns matches regardless of casing.
var filter = new MultiFilter
{
StringComparison = StringComparison.OrdinalIgnoreCase
};
var items = new List<SelectedItem>
{
new SelectedItem("Apple", "Apple"),
new SelectedItem("apple", "apple"),
new SelectedItem("APPLE", "APPLE")
};
// Act: Perform search using "Apple" as query.
var result = await filter.GetItemsAsync("Apple", items);
// Assert: All three items should match.
Assert.Equal(3, result.Count);
}
[Fact]
public async Task Test_StringComparison_CurrentCulture()
{
// Arrange: CurrentCulture comparison is typically case-sensitive.
var filter = new MultiFilter
{
StringComparison = StringComparison.CurrentCulture
};
var items = new List<SelectedItem>
{
new SelectedItem("Apple", "Apple"),
new SelectedItem("apple", "apple"),
new SelectedItem("APPLE", "APPLE")
};
// Act: Perform search using "Apple" as query.
var result = await filter.GetItemsAsync("Apple", items);
// Assert: Only the item with exact casing "Apple" should be returned.
Assert.Single(result);
Assert.Equal("Apple", result.First().Text);
}
}
// End of fileMake sure that:
- The MultiFilter class has a GetItemsAsync(searchQuery, items) method or a similar method to perform the filtering, otherwise adjust the test to correctly call the filtering functionality.
- The SelectedItem class exposes a property (e.g., Text) that contains the string to be compared.
- The necessary using directives are present at the top of the file if they are not already included in your codebase.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link issues
fixes #6008
Summary By Copilot
This pull request introduces improvements to the filtering functionality in the
MultiFiltercomponent and refactors how filtered items are handled in theTablesFiltercomponent. The most important changes include adding support for customizable string comparison inMultiFilter, updating the filtering logic to use this new feature, and modifying test cases to ensure compatibility.Enhancements to filtering functionality:
src/BootstrapBlazor/Components/Filters/MultiFilter.razor.cs: Added a newStringComparisonparameter to theMultiFiltercomponent, allowing users to specify the string comparison option for filtering operations. The default is set toStringComparison.OrdinalIgnoreCase.src/BootstrapBlazor/Components/Filters/MultiFilter.razor.cs: Updated the filtering logic in theOnSearchValueChangedmethod to use the newStringComparisonparameter when performing string comparisons.Refactoring and test updates:
src/BootstrapBlazor.Server/Components/Samples/Table/TablesFilter.razor.cs: Refactored theOnGetAddressItemsAsyncmethod to use the spread operator ([..]) when returning filtered items, simplifying the code.test/UnitTest/Components/TableFilterTest.cs: Updated theMultiFilter_Oktest to include the newStringComparisonparameter and adjusted the attribute ordering for consistency.Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add customizable string comparison support to MultiFilter component, improving filtering flexibility
New Features:
Enhancements:
Tests: