doc(Select): add DefaultVirtualizeItemText parameter sample code#7294
Merged
doc(Select): add DefaultVirtualizeItemText parameter sample code#7294
Conversation
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates the Select component samples to demonstrate DefaultVirtualizeItemText with ID-based values, fixes localization key typos, adjusts virtualized data mappings, and slightly refines default item selection logic in the core Select component. Updated class diagram for Selects sample component and Select core logicclassDiagram
class Selects {
+IEnumerable~SelectedItem~ VirtualItems
-int? _virtualItem1
-string? VirtualItemText1
-int? _virtualItem2
-string? VirtualItemText2
-List~Foo~ Foos
+Task~QueryData~SelectedItem~~ OnQueryAsync(VirtualizeQueryOption option)
}
class Foo {
+int Id
+string? Name
}
class SelectedItem {
+string? Value
+string? Text
+bool Active
+bool IsDisabled
SelectedItem(string value, string text)
}
class Select {
-List~SelectedItem~ Rows
-string? CurrentValueAsString
+SelectedItem? GetDefaultItem()
+SelectedItem? GetItemWithEnumValue()
}
Selects --> Foo : uses
Selects --> SelectedItem : builds_virtual_items
Selects --> Select : used_in_Razor_sample
Select --> SelectedItem : manages_rows
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- In
Selects.razor.cs,VirtualItemText1andVirtualItemText2are hardcoded toId == 2andId == 3; consider deriving these from_virtualItem1and_virtualItem2respectively so the displayed text always matches the currently bound value. - The new
_virtualItem1/_virtualItem2fields use a naming convention that appears different from the existingVirtualItem*members; consider aligning the naming pattern (fields vs properties) with the rest of the component for consistency and readability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `Selects.razor.cs`, `VirtualItemText1` and `VirtualItemText2` are hardcoded to `Id == 2` and `Id == 3`; consider deriving these from `_virtualItem1` and `_virtualItem2` respectively so the displayed text always matches the currently bound value.
- The new `_virtualItem1`/`_virtualItem2` fields use a naming convention that appears different from the existing `VirtualItem*` members; consider aligning the naming pattern (fields vs properties) with the rest of the component for consistency and readability.
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor.Server/Components/Samples/Selects.razor:455-456` </location>
<code_context>
<div class="row mb-3">
<div class="col-6">
- <Select IsVirtualize="true" OnQueryAsync="OnQueryAsync" @bind-Value="VirtualItem1"
+ <Select IsVirtualize="true" OnQueryAsync="OnQueryAsync" @bind-Value="_virtualItem1"
+ DefaultVirtualizeItemText="@VirtualItemText1"
ShowSearch="_showSearch" IsClearable="_isClearable"></Select>
</div>
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Keep the default virtual item text derived from the current bound value instead of a hard-coded ID.
`DefaultVirtualizeItemText` is currently based on `Foos.FirstOrDefault(i => i.Id == 2)?.Name`, while the bound value is `_virtualItem1`. If `_virtualItem1`’s default changes (e.g., to 5) but the hard-coded `2` is not updated, the displayed text and selected value will diverge. Please derive the text from `_virtualItem1` (e.g., by using its value in the lookup) so they always stay in sync.
Suggested implementation:
```
private string? VirtualItemText1 => _virtualItem1.HasValue
? Foos.FirstOrDefault(i => i.Id == _virtualItem1.Value)?.Name
: null;
```
If the property is currently declared with a non-nullable return type (`string` instead of `string?`), adjust the type accordingly or provide a sensible fallback (e.g., `?? string.Empty`). Also ensure that `_virtualItem1` and `Foos` are in scope in this component's code block.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7294 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 745 745
Lines 32628 32628
Branches 4522 4522
=========================================
Hits 32628 32628
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request adds sample code demonstrating the DefaultVirtualizeItemText parameter for the Select component when using virtualization. The changes update the virtualization demo to properly show how this parameter works with both OnQueryAsync and Items data sources.
Key changes:
- Modified virtualization demo to use integer IDs instead of SelectedItem objects for better type safety
- Added
DefaultVirtualizeItemTextparameter to both virtualization examples - Fixed spelling typos in localization keys (SelectConfirm vs SelectConfifm)
- Updated Chinese documentation to explain the DefaultVirtualizeItemText parameter usage
- Changed Select component logic from
FirstOrDefaulttoFindfor consistency
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Select.razor.cs | Changed method from FirstOrDefault to Find for consistent pattern usage when selecting non-disabled items |
| zh-CN.json | Fixed typo in localization keys (SelectConfirm vs SelectConfifm) and added documentation about DefaultVirtualizeItemText parameter |
| en-US.json | Fixed typo in localization keys (SelectConfirm vs SelectConfifm) |
| Selects.razor.cs | Updated virtualization demo to use integer IDs, added properties for default virtualized item text, and modified data source to use Foo.Id instead of Foo.Name |
| Selects.razor | Added DefaultVirtualizeItemText parameter to both virtualization examples and updated Display components to show integer values |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "SelectsVirtualizeTitle": "虚拟滚动", | ||
| "SelectsVirtualizeIntro": "通过设置 <code>IsVirtualize</code> 参数开启组件虚拟功能特性", | ||
| "SelectsVirtualizeDescription": "组件虚拟滚动支持两种形式通过 <code>Items</code> 或者 <code>OnQueryAsync</code> 回调方法提供数据", | ||
| "SelectsVirtualizeDescription": "组件虚拟滚动支持两种形式通过 <code>Items</code> 或者 <code>OnQueryAsync</code> 回调方法提供数据。如果数据源使用 <code>OnQueryAsync</code> 回调获得时只有当下拉框展开时才会触发，，如果数据源使用 <code>Items</code> 时，由于性能问题（有些开发会把几百万条数据给 Items）内部并没有进行查找选中项，所以需要设置 <code>DefaultVirtualizeItemText</code> 值应对首次加载时不知道如何显示问题", |
There was a problem hiding this comment.
There is a double comma (，，) in the Chinese text. It should be a single comma.
Suggested change
| "SelectsVirtualizeDescription": "组件虚拟滚动支持两种形式通过 <code>Items</code> 或者 <code>OnQueryAsync</code> 回调方法提供数据。如果数据源使用 <code>OnQueryAsync</code> 回调获得时只有当下拉框展开时才会触发，，如果数据源使用 <code>Items</code> 时，由于性能问题（有些开发会把几百万条数据给 Items）内部并没有进行查找选中项，所以需要设置 <code>DefaultVirtualizeItemText</code> 值应对首次加载时不知道如何显示问题", | |
| "SelectsVirtualizeDescription": "组件虚拟滚动支持两种形式通过 <code>Items</code> 或者 <code>OnQueryAsync</code> 回调方法提供数据。如果数据源使用 <code>OnQueryAsync</code> 回调获得时只有当下拉框展开时才会触发，如果数据源使用 <code>Items</code> 时，由于性能问题（有些开发会把几百万条数据给 Items）内部并没有进行查找选中项，所以需要设置 <code>DefaultVirtualizeItemText</code> 值应对首次加载时不知道如何显示问题", |
Comment on lines
+455
to
457
| <Select IsVirtualize="true" OnQueryAsync="OnQueryAsync" @bind-Value="_virtualItem1" | ||
| DefaultVirtualizeItemText="@VirtualItemText1" | ||
| ShowSearch="_showSearch" IsClearable="_isClearable"></Select> |
There was a problem hiding this comment.
The Select component is missing the TValue generic parameter specification. While type inference may work in some cases, it's best practice to explicitly declare TValue for clarity and to avoid potential type inference issues. Consider adding TValue="int?" to match the binding type.
| <div class="row"> | ||
| <div class="col-6"> | ||
| <Select IsVirtualize="true" Items="VirtualItems" @bind-Value="VirtualItem2" | ||
| <Select IsVirtualize="true" Items="VirtualItems" @bind-Value="_virtualItem2" |
There was a problem hiding this comment.
The Select component is missing the TValue generic parameter specification. While type inference may work in some cases, it's best practice to explicitly declare TValue for clarity and to avoid potential type inference issues. Consider adding TValue="int?" to match the binding type.
Suggested change
| <Select IsVirtualize="true" Items="VirtualItems" @bind-Value="_virtualItem2" | |
| <Select TValue="int?" IsVirtualize="true" Items="VirtualItems" @bind-Value="_virtualItem2" |
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
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 #7292
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Document and demonstrate the use of DefaultVirtualizeItemText in virtualized Select samples while aligning value types with Foo IDs and tightening default item selection logic.
New Features:
Bug Fixes:
Enhancements: