fix(Select): placeholder does not disappear #7581
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 guide (collapsed on small PRs)Reviewer's GuideAdjusts the Select and MultiSelect components’ virtualized loading behavior to always request the provider’s requested count, simplifying pagination logic and resolving the issue where the placeholder did not disappear when items were loaded. Sequence diagram for updated Select LoadItems virtualization flowsequenceDiagram
actor User
participant SelectComponent
participant Virtualize
participant DataProvider
User->>SelectComponent: Open dropdown or type search
SelectComponent->>Virtualize: Initialize virtualized list
Virtualize->>SelectComponent: LoadItems(request)
SelectComponent->>DataProvider: OnQueryAsync(StartIndex, request.Count, SearchText)
DataProvider-->>SelectComponent: QueryData(TotalCount, Items)
SelectComponent->>SelectComponent: Update _totalCount, _itemsCache, _result
SelectComponent-->>Virtualize: ItemsProviderResult
Virtualize-->>User: Render items and hide placeholder
Updated class diagram for Select and MultiSelect LoadItems behaviorclassDiagram
class ItemsProviderRequest {
int StartIndex
int Count
}
class ItemsProviderResult_SelectedItem_ {
List_SelectedItem_ Items
int TotalItemCount
}
class QueryPageOptions {
int StartIndex
int Count
string SearchText
}
class QueryData_SelectedItem_ {
List_SelectedItem_ Items
int TotalCount
}
class Select {
string SearchText
int _totalCount
List_SelectedItem_ _itemsCache
ItemsProviderResult_SelectedItem_ _result
ValueTask~ItemsProviderResult_SelectedItem_~ LoadItems(ItemsProviderRequest request)
Task~QueryData_SelectedItem_~ OnQueryAsync(QueryPageOptions options)
}
class MultiSelect {
string SearchText
int _totalCount
List_SelectedItem_ _itemsCache
ItemsProviderResult_SelectedItem_ _result
ValueTask~ItemsProviderResult_SelectedItem_~ LoadItems(ItemsProviderRequest request)
Task~QueryData_SelectedItem_~ OnQueryAsync(QueryPageOptions options)
}
Select ..> ItemsProviderRequest : uses
Select ..> ItemsProviderResult_SelectedItem_ : returns
Select ..> QueryPageOptions : builds
Select ..> QueryData_SelectedItem_ : receives
MultiSelect ..> ItemsProviderRequest : uses
MultiSelect ..> ItemsProviderResult_SelectedItem_ : returns
MultiSelect ..> QueryPageOptions : builds
MultiSelect ..> QueryData_SelectedItem_ : receives
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 - I've left some high level feedback:
- The
LoadItemsimplementations inSelectandMultiSelectare now identical; consider extracting this shared logic into a common helper or base class method to reduce duplication and keep future behavioral fixes in one place. - By removing
GetCountByTotalyou’re fully trustingItemsProviderRequest.Count; if any data providers return fewer items than requested without correctly settingTotalCount, this could cause unexpected behavior, so it may be worth adding a brief comment explaining that this assumption is intentional and aligned withVirtualize’s contract.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `LoadItems` implementations in `Select` and `MultiSelect` are now identical; consider extracting this shared logic into a common helper or base class method to reduce duplication and keep future behavioral fixes in one place.
- By removing `GetCountByTotal` you’re fully trusting `ItemsProviderRequest.Count`; if any data providers return fewer items than requested without correctly setting `TotalCount`, this could cause unexpected behavior, so it may be worth adding a brief comment explaining that this assumption is intentional and aligned with `Virtualize`’s contract.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 #7581 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 749 749
Lines 32976 32972 -4
Branches 4580 4576 -4
=========================================
- Hits 32976 32972 -4
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 fixes a bug where the placeholder text was incorrectly retained in Select and MultiSelect components when search text was cleared during virtualization. The issue occurred when users typed a search term, then deleted it - the placeholder would remain visible instead of showing the full list of items.
Changes:
- Simplified the LoadItems method in Select and MultiSelect components by removing conditional count logic
- Removed the GetCountByTotal helper method that was causing incorrect item count requests after search text was cleared
- Updated package version from 10.2.3-beta03 to 10.2.3 (release version)
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/BootstrapBlazor/Components/Select/Select.razor.cs | Removed conditional count logic in LoadItems that caused placeholder issues when search was cleared |
| src/BootstrapBlazor/Components/Select/MultiSelect.razor.cs | Applied same fix as Select component to ensure consistent behavior |
| src/BootstrapBlazor/BootstrapBlazor.csproj | Bumped version to 10.2.3 release version |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| var count = !string.IsNullOrEmpty(SearchText) ? request.Count : GetCountByTotal(); | ||
| var data = await OnQueryAsync(new() { StartIndex = request.StartIndex, Count = count, SearchText = SearchText }); | ||
| var data = await OnQueryAsync(new() { StartIndex = request.StartIndex, Count = request.Count, SearchText = SearchText }); |
There was a problem hiding this comment.
The same conditional count logic that was removed here still exists in SelectGeneric.razor.cs (line 318) and MultiSelectGeneric.razor.cs (line 348). If this fix resolves the placeholder issue in Select and MultiSelect, the same fix should be applied to SelectGeneric and MultiSelectGeneric components for consistency and to prevent the same bug from occurring there.
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 #7325
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Bug Fixes: