-
-
Notifications
You must be signed in to change notification settings - Fork 364
feat(MultiSelectGeneric): add MultiSelectGeneric component #6103
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.
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
Conversation
Reviewer's GuideIntroduces a new fully-featured generic multi-select component (MultiSelectGeneric) with virtualization, toolbar and template support; integrates it into the sample page with demo data and localization; and refactors the base class metadata while removing obsolete styling. Sequence Diagram for Item Selection in MultiSelectGenericsequenceDiagram
actor User
participant RazorUI as "MultiSelectGeneric.razor"
participant ComponentLogic as "MultiSelectGeneric.razor.cs"
User->>RazorUI: Clicks on an item (e.g., 'Item A')
RazorUI->>ComponentLogic: ToggleRow(itemA_Instance)
activate ComponentLogic
ComponentLogic->>ComponentLogic: Update SelectedItems list (add/remove itemA_Instance)
ComponentLogic->>ComponentLogic: SetValue() calls CurrentValue = ..., OnSelectedItemsChanged(), StateHasChanged()
ComponentLogic-->>RazorUI: Updated state (selection changed)
deactivate ComponentLogic
RazorUI-->>User: UI reflects new selection (Item A is selected/deselected)
Class Diagram for the new MultiSelectGeneric ComponentclassDiagram
direction LR
class SelectBase_List_TValue_ {
<<BlazorComponent, generic>>
#List~TValue~ CurrentValue
+EventCallback~List~TValue~~ ValueChanged
#OnClearValue() Task
#OnValidate(bool? valid) void
}
class MultiSelectGeneric_TValue_ {
<<BlazorComponent, generic>>
+IEnumerable~SelectedItem~TValue~~ Items
+RenderFragment~List~SelectedItem~TValue~~~ DisplayTemplate
+RenderFragment~SelectedItem~TValue~~ ItemTemplate
+bool ShowToolbar
+bool ShowSearch
+bool ShowCloseButton
+int Max
+int Min
+string SelectAllText
+string ReverseSelectText
+string ClearText
+Func~VirtualizeQueryOption, Task~QueryData~SelectedItem~TValue~~~~ OnQueryAsync
+Func~IEnumerable~SelectedItem~TValue~~, Task~ OnSelectedItemsChanged
-List~SelectedItem~TValue~~ SelectedItems
+ToggleRow(SelectedItem~TValue~ item) Task
+SelectAll() Task
+InvertSelect() Task
+Clear() Task
+TriggerOnSearch(string searchText) Task
}
class SelectedItem_TValue_ {
<<Data, generic>>
+TValue Value
+string Text
+bool IsDisabled
}
MultiSelectGeneric_TValue_ --|> SelectBase_List_TValue_
MultiSelectGeneric_TValue_ "1" o-- "0..*" SelectedItem_TValue_ : selectedItems
MultiSelectGeneric_TValue_ ..> SelectedItem_TValue_ : uses_items_parameter
class VirtualizeQueryOption {
+int StartIndex
+int Count
+string SearchText
}
class QueryData_T_ {
<<generic>>
+IEnumerable~T~ Items
+int TotalCount
}
MultiSelectGeneric_TValue_ ..> VirtualizeQueryOption : uses_for_OnQueryAsync
MultiSelectGeneric_TValue_ ..> QueryData_T_ : uses_for_OnQueryAsync_return
Class Diagram: MultiSelects Page using MultiSelectGenericclassDiagram
class MultiSelectsPage {
<<RazorComponent>>
-FooItems: List~SelectedItem~Foo~~
-_genericValue: List~Foo~
#OnInitialized() void
}
class MultiSelectGeneric_Foo_ {
<<BlazorComponent>>
+IEnumerable~SelectedItem~Foo~~ Items
+List~Foo~ Value
+EventCallback~List~Foo~~ ValueChanged
}
MultiSelectsPage ..> MultiSelectGeneric_Foo_ : uses_and_configures
note for MultiSelectsPage "_genericValue is bound to MultiSelectGeneric_Foo_.Value"
class SelectedItem_Foo_ {
<<Data>>
+Foo Value
+string Text
}
MultiSelectsPage ..> SelectedItem_Foo_ : initializes (FooItems)
MultiSelectGeneric_Foo_ ..> SelectedItem_Foo_ : processes (Items)
class Foo {
+string Name
}
SelectedItem_Foo_ *-- Foo : wraps
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @ArgoZhang - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 4 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 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.
| [ExcludeFromCodeCoverage] | ||
| public partial class MultiSelectGeneric<TValue> | ||
| { | ||
| private List<SelectedItem<TValue>> SelectedItems { get; } = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): SelectedItems is always initialized empty, which may not reflect the initial Value.
If the component uses @bind-Value, ensure SelectedItems is initialized from the initial Value to keep the UI and bound data consistent.
|
|
||
| private List<SelectedItem<TValue>> GetVirtualItems() => [.. FilterBySearchText(GetRowsByItems())]; | ||
|
|
||
| private async ValueTask<ItemsProviderResult<SelectedItem<TValue>>> LoadItems(ItemsProviderRequest request) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: LoadItems does not handle exceptions from OnQueryAsync.
Add try/catch in LoadItems to handle exceptions from OnQueryAsync and ensure the Virtualize component remains stable.
| @<DynamicElement OnClick="() => ToggleRow(item)" TriggerClick="@CheckCanTrigger(item)" class="@GetItemClassString(item)"> | ||
| <div class="multi-select-item"> | ||
| <div class="form-check"> | ||
| <input class="form-check-input" type="checkbox" disabled="@CheckCanSelect(item)" checked="@GetCheckedString(item)" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): The checked attribute is set as a string, which may cause issues.
Blazor checkboxes require a boolean for the 'checked' attribute. Update GetCheckedState to return a boolean and bind it directly to 'checked'.
| { | ||
| @DisplayTemplate(SelectedItems) | ||
| } | ||
| else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Group dividers are rendered both before and after each group.
Rendering dividers both before and after each group can create duplicates. Consider rendering a divider only before each group except the first, or after each group except the last, to avoid unnecessary dividers.
Link issues
fixes #6102
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add the MultiSelectGeneric component with rich multi-selection capabilities and integrate it into the existing demos.
New Features:
Documentation:
Chores: